Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In 0.12, modules can no longer be installed from local git repositories at relative paths #21107

Open
robrankin opened this issue Apr 25, 2019 · 18 comments · May be fixed by #26056
Open

In 0.12, modules can no longer be installed from local git repositories at relative paths #21107

robrankin opened this issue Apr 25, 2019 · 18 comments · May be fixed by #26056
Labels
bug upstream v0.12 Issues (primarily bugs) reported against v0.12 releases

Comments

@robrankin
Copy link
Contributor

Terraform Version

Have reproduced this with beta1, beta2, and a dev compiled version from master today.

Terraform v0.12.0-beta1
Terraform v0.12.0-beta2
Terraform v0.12.0-dev

Terraform Configuration Files

module "modx" {
  source                      = "git::../../../repo//path/to/module?ref=621"

Debug Output

Doesn't appear to be extremely useful, except perhaps this line:

2019/04/25 10:33:15 [TRACE] go-getter detectors rewrote "git::../../../repo?ref=621" to "git::file:///repo?ref=621"

Expected Behavior

terraform init should succeed

Actual Behavior

terraform init fails as follows:

0.12.0-beta1

Error: Failed to download module

Error attempting to download module "modx" (mod_x.tf:6)
source code from "git::../../../repo?ref=621": error
downloading 'file:///repo?ref=621': /usr/bin/git exited with
128: Cloning into '.terraform/modules/modx'...
fatal: '/repo' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

0.12.0-beta2 and 0.12.0-dev

Error: Failed to download module

Error attempting to download module "modx" (mod_x.tf:6)
source code from "git::./../../../repo?ref=621": <nil>

Steps to Reproduce

  1. Use a local filesystem Git repo and reference it as specified above
  2. terraform init

Additional Context

These local filesystem git references work in 0.11.x

@apparentlymart
Copy link
Contributor

Thanks for reporting this, @robrankin!

I don't think this was intentionally supported in v0.11 and prior and was probably just a side-effect of an implementation detail that has changed now, but we'll see what we can do to restore the behavior nonetheless, since I can see that it would be useful.

@apparentlymart apparentlymart changed the title TF 12 Local Filesystem Git Repo In 0.12, modules can no longer be installed from local git repositories at relative paths Apr 25, 2019
@salimane
Copy link

salimane commented May 10, 2019

I am not sure if it is related but I am getting the same error with tf 0.12.0-rc1

module "my_module" {
  source  = "git::ssh://[email protected]/private/repo-modules.git?ref=tag_v0.0.1//folder/sub_folder"
}
Error: Failed to download module

Error attempting to download module "my_module"
(file.tf:2) source code from
"git::ssh://[email protected]/private/repo-modules.git?ref=tag_v0.0.1//folder/sub_folder":
<nil>

am I missing something @apparentlymart ?

@kipkoan
Copy link

kipkoan commented Jun 25, 2019

@apparentlymart - We just ran into this same issue while working on upgrading our terraform repos. We use pinning of local repo hashes to support environment promotion. Do you think a fix for this might be coming soon? Thanks!

@kipkoan
Copy link

kipkoan commented Jun 25, 2019

$ terraform init -upgrade
Upgrading modules...
Downloading git::../..?ref=5067df412b6 for testing...

Error: Failed to download module

Could not download module "testing" (main.tf:22) source code from
"git::../..?ref=5067df412b6": error downloading
'file:///.terraform/modules/..?ref=5067df412b6': /usr/bin/git exited with 128:
Cloning into '.terraform/modules/testing'...
fatal: '/.terraform/modules/..' does not appear to be a git repository

@mnikhil-git
Copy link

I noticed similar issue. I have been using the module sources from git paths but when I switch the module source to local filesystem path, same module does not seem to work because of use of path.module functions.

Also, when module source is git and terraform12 is initialized, modules get checked out and initialized in .terraform/modules/<module_name>/<full_path_to_module_directory_in_git>

whereas for the localfilesystem, modules are simply symlinked under .terraform/modules/<module_name> --> module_directory_local_filesystem.

Is this intentional? why is not there a copy of the module in the initialized directory instead?

@hashibot hashibot added the v0.12 Issues (primarily bugs) reported against v0.12 releases label Aug 22, 2019
@salimane
Copy link

salimane commented Sep 16, 2019

Error: Failed to download module

Error attempting to download module "my_module"
(file.tf:2) source code from
"git::ssh://[email protected]/private/repo-modules.git?ref=tag_v0.0.1//folder/sub_folder":

I have found the solution to the above issue. here is the correct way of sourcing private repositories in terraform assuming you have the right permissions to clone the repository

module "my_module" {
  source  = "git::ssh://[email protected]:private/repo-modules.git//folder/sub_folder?ref=tag_v0.0.1"
}

@hunkjun
Copy link

hunkjun commented Oct 8, 2019

source = "git::ssh://[email protected]:sre/vvv/legacy.git//aws_userdata?ref=master"

@maxmanders
Copy link

For what it's worth, I've had success with

module "my_module" {
  source  = "git::ssh://[email protected]/organisation/repo.git//path/to/module?ref=tag_v0.0.1"
}

Note the / rather than : after [email protected]. I saw errors when using the previous suggestion but that may be specific to my set up.

@kipkoan
Copy link

kipkoan commented Nov 13, 2019

@salimane , @hunkjun , @maxmanders -- you guys are misunderstanding the problem that @robrankin posted, and I confirmed. Installing modules from remote git repos has always worked. Installing modules from local git repos, using relative paths, used to work prior to 0.12.

This worked in 0.11:

source = "git::../../../repo//path/to/module?ref=621"

But no longer does in 0.12.

It's super useful for local development without having to push commits to the remote before running init/plan/apply.

@maxmanders
Copy link

Copy that, apologies for fuelling the confusion!

@Erokos
Copy link

Erokos commented Mar 31, 2020

Any updates regarding this issue? It's fairly tedious having to commit and push commits that don't work so I can find out that they don't work :)

smjmoj added a commit to ministryofjustice/hmpps-delius-network-terraform that referenced this issue Jun 25, 2020
Basically changing things around and removing double slashes.
AS per hashicorp/terraform#21107 (comment)
@salewski
Copy link
Contributor

I just left a comment over in issue #25488 that I implemented a (forthcoming) fix for supporting git:: forced local paths (both relative and absolute). After rummaging around in the code, though, I'm a little surprised that previously worked (though I did not look back in time very far); if I find time, I might try a little archaeology on it. In any event, I'll post a PR with my proposed changes in the next day or so to gather feedback.

@salewski
Copy link
Contributor

...I'll post a PR with my proposed changes in the next day or so to gather feedback.

There's an issue write-up in hashicorp/go-getter#268, as that's the library that provides the Git repo cloning support for Terraform. The string value of a Terraform module call's source parameter is parsed and handled by the go-getter lib.

The draft PR for it is hashicorp/go-getter#269

Assuming that PR (or something like it) gets merged, then getting the support for this feature in Terraform will require updating the dependency on go-getter to a version that includes the change.

@apparentlymart
Copy link
Contributor

Thanks for capturing that context over in the go-getter repository, @salewski!

I want to add a little more context here that is Terraform-specific:

Terraform handles relative paths to modules already on disk (without the git:: prefix, etc) directly itself, rather than with go-getter, because the rule is that a relative path is interpreted against the directory of the module that called it, rather than against the current working directory.

Implementing the handling of relative paths for git inside go-getter would, unfortunately, make this behavior inconsistent: go-getter has no awareness of the idea of Terraform modules, and so the relative path would end up being interpreted relative to the current working directory rather than to the calling module. As well as just being inconsistent, that would also make it impossible to reliably use git:: relative paths in any shared module, because it would not be able to predict its own path relative to the current working directory for similar reasons that today modules can't predict their own absolute paths to find their neighbor local git modules.

I'm not sure at this time what the best way forward is on this. In order to properly meet the use-cases described in that go-getter issue I think we would need to somehow make a git:: relative path be interpreted relative to the current module directory, but it's not clear to me how to do that within the constraints of the existing go-getter API. The GitDetector type could potentially have an optional field to allow overriding its sense of "current working directory" for the purposes of resolving relative paths, but that would require Terraform to construct those instances dynamically on a per-request basis (not super difficult, but quite different than the current design) and it doesn't really fit in with the current proposed PR in hashicorp/go-getter#269 where the lookup is handled directly inside the Detect function, rather than inside GitDetector.

With all of that said, it seems like this might require some more thought to figure out what the appropriate separation of concerns is. Because go-getter is a shared library, we must also consider what impacts this might potentially have on other go-getter callers which may have other ideas about what "current working directory" means, or whether reading objects from the local filesystem is even desirable.

@salewski
Copy link
Contributor

[...]
I want to add a little more context here that is Terraform-specific:
[...]
...because the rule is that a relative path is interpreted against the directory of the module that called it, rather than against the current working directory.

@apparentlymart: Thanks for that additional context; that's quite helpful. I see what you mean about the module's context vs. $PWD -- my local testing setup was too simplistic to have that be an issue.

In the work thus far on hashicorp/go-getter#269, there was definitely some friction against the current Detector API. In fact, my comment that starts on line 55 of detect_git.go is something of an apology for introducing a function outside of the Detector interface. I don't love the leaked abstraction into the calling Detect(...) dispatch function, but I held my nose in order to avoid breaking the API compatibility. The tasklist item "document limitations and/or possible future work" in the PR was there to shine a light on that wart and open a discussion on how to maybe clean it up. But it won't fly, anyway, without a better approach to the $PWD thing, like you said.

I def. agree that more thought is required.

Thanks for the great feedback!

@salewski
Copy link
Contributor

[...]
...or whether reading objects from the local filesystem is even desirable.

@apparentlymart, Could I ask you to elaborate on that notion a little?

The current Detect(...) dispatcher function does give the caller some control over which specific Detector implementations are used, so conceivably could be used to avoid applying particular detectors that have unwanted behaviors for a given scenario. But right now the implementations are "all or nothing" by technology (Git, File) or service (GCS, S3, GitHub, BitBucket).

@apparentlymart
Copy link
Contributor

Hi @salewski,

Regarding the "is local filesystem access desirable?" question: I don't know if this actually matters in practice, but previously it seems like a non-Terraform go-getter caller could specifically have omitted detectors/getters that access the local filesystem and keep only the ones that interact with network services. For example, HashiCorp Nomad also uses go-getter in the context of a clustered network service, where accessing the local disk of a particular node might be conceptually weird -- though I will qualify that I'm only talking hypothetically here; I don't have any specific Nomad knowledge to confirm that.

If this did turn out to be true then one way we could potentially address it is to make the local file support opt-in, by whether this hypothetical new "base directory" field we've been talking about is set at all. Of course, whether that makes sense would depend on how the rest of that plays out, given the still-open question of whether this functionality can be the responsibility of the GitDetector rather than just `Detector.

(On re-read I also see that you've suggested here that absolute local filesystem paths to git repositories have been working, in which case this would be less of a concern because it doesn't introduce any new access that wasn't present before. I didn't quite catch that on first read, so I was assuming that local directories were not possible at all before.)

@salewski
Copy link
Contributor

salewski commented Aug 28, 2020

...previously it seems like a non-Terraform go-getter caller could specifically have omitted detectors/getters that access the local filesystem and keep only the ones that interact with network services.
[...]
If this did turn out to be true then one way we could potentially address it is to make the local file support opt-in

Thanks @apparentlymart. Makes sense; I'll keep it in mind.

(On re-read I also see that you've suggested here that absolute local filesystem paths to git repositories have been working, in which case this would be less of a concern because it doesn't introduce any new access that wasn't present before. I didn't quite catch that on first read, so I was assuming that local directories were not possible at all before.)

Oh, in my comment above, I did not mean to imply that I knew absolute paths had been working previously.

I did not find this GH issue until after I had done most of the work behind the current state of hashicorp/go-getter#269. My experience was that neither absolute nor relative filepaths worked with git:: forced strings. And after having been in the code to make it work[0], I was surprised to read the reports from others who have commented here that git:: with local file paths of any sort ever worked.

[0] only partially, as it turns out, at this point

salewski added a commit to salewski/go-getter that referenced this issue Aug 31, 2020
…nd relative

This series of changesets introduces a feature that allows the 'git::'
forcing token to be used on local file system paths to reference Git
repositories. Both absolute paths and relative paths are supported. For
example:
    git::./some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::../../some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::/some/absolute/path/to/a/git-repo//some-subdir?ref=v4.5.6

Only filepaths that are prefixed with the 'git::' forcing token are
considered for processing.

Internally, go-getter transforms the provided string into a 'file://'
URI with an absolute filepath, with query string params and subdirectory
retained.

The rationale for using a 'file://' URI internally is that the Git clone
operation can already work with 'file://' URIs, and using them for this
feature allows us to leverage the existing go-getter URI-handling
machinery. That gets us support for query params (to clone a specific
git ref (tag, commit hash, ...)) "for free".

The rationale for using an absolute filepath (even when the provided
string is a relative filepath) is that (per RFC 1738 and RFC 8089) only
absolute filepaths are legitimate in 'file://' URIs. But more
importantly here, the Git clone operation only supports 'file://' URIs
with absolute paths.

Q: Why support this functionality at all?

   Why not just require that a source location use an absolute path in a
   'file://' URI explicitly if that's what is needed?

A: The primary reason is to allow support for relative filepaths to Git
   repos.

   There are use cases in which the absolute path cannot be known in
   advance, but a relative path to a Git repo is known.

   For example, when a Terraform project (or any Git-based project) uses
   Git submodules, it will know the relative location of the Git
   submodule repos, but cannot know the absolute path in advance because
   it will vary based on where the "superproject" repo is
   cloned. Nevertheless, those relative paths should be usable as
   clonable Git repos, and this mechanism would allow for that.

   Support for filepaths that are already absolute is provided mainly
   for symmetry. It would be surprising for the feature to work with
   relative file paths, but not for absolute filepaths.

For projects using Terraform, in particular, this feature (along with a
small change in the Terraform code to leverage it) enables the
non-fragile use of relative paths in a module "call" block, when
combined with Git submodules:

    module "my_module" {
        source = "git::../git-submodules/tf-modules/some-tf-module?ref=v0.1.0"
        // ...
    }

In the above example "superproject" Git repo (the one "calling" the
terraform module) knows the relative path to its own Git submodules
because they are embedded in a subdirectory beneath the top-level of the
"superproject" repo.

Two downstream Terraform issues that would require go-getter support for
this feature (or something like it) are at [0] and [1].

This first changeset in the series updates the README.md documentation
to note the new feature and provide examples.

[0] "Unable to use relative path to local Git module"
    hashicorp/terraform#25488

[1] "In 0.12, modules can no longer be installed from local git repositories at relative paths"
    hashicorp/terraform#21107

Design Notes
------------
In order for this feature to work, additional contextual information is
needed by the Git detector than can be provided using the existing
Detector API.

Internally, the Detector's Detect method does not pass along to the
Detector implementations all of the contextual information that it has
available. In particular, the forcing token and go-getter subdir
component are stripped out of the source string before invoking the
implementation's Detect method. In the particular case of the Git
detector, that means it cannot know that a 'git::' forcing token was
provided on an input string that otherwise looks like a file system
path. And /that/ means that it is not correct or safe for it to identify
any filepath string value as a Git repository.

Externally, callers (such as Terraform) already provide a value for the
'pwd' parameter of Detect, but it is not (necessarily) the location from
which a relative path in a 'git::' string should be resolved. In a
Terraform module (which may be in an arbitrary subdirectory from the
process current working directory), module "source" references that
contain relative paths must be interpreted relative to the location of
the module source file. Terraform has that information available, but in
the existing Detect API there is no way to convey it to go-getter.

Constraints
-----------
Additional Detector methods cannot be added without burdening all
existing detectors (both internal and in the wild) with the need to
support them.

Additional Detect method params cannot be added without breaking all
existing Detector implementations (internal, wild).

Additional parameters cannot be added to the Detect dispatching function
without affecting all callers.

Approach
--------
The goal is to provide the feature in a way that is as minimally
invasive as possible. But above all else it needs to avoid breaking
backward compatibility in any way.

Given that, the approach taken by this changeset series is to introduce
the concept of a "Contextual Detector". It is structured in the same way
as the current Detector framework, but works through a new CtxDetector
interface that is not constrained by the existing API.

The only callers affected by this change would be those that wish to take
advantage of the additional capabilities. And for those, the migration path
straight-forward because the new API is structured like the existing one.

In particular, this changeset series introduces four new elements:

    1. CtxDetector interface

    2. CtxDetect dispatching function

    3. CtxDetect method on the CtxDetector interface

    4. Full suite of CtxDetector implementations that are analogues of
       the existing detectors (most of which (currently) just delegate
       to the existing Detector implementations).

There is also a global 'ContextualDetectors' list that serves a function
analogous to the existing 'Detectors' list.
salewski added a commit to salewski/terraform that referenced this issue Aug 31, 2020
The go-getters library now has support for local file system paths to
Git repositories, specified with the 'git::' forcing token. The feature
works for both absolute and relative filepaths, and supports all the
usual go-getter goodies including '//' delimited subdirs and URI-style
query parameters.[0][1]

We incorporate that capability into Terraform, which allows users to
specify paths to locally present Git repositories from which to clone
other Terrform modules on which they are dependent. When coupled with
Git submodules, this creates a powerful way to manage Terraform modules
at specific versions without requiring those modules to be available on
the network (e.g., on GitHub):

    module "my_module" {
        source = "git::../git-submodules/tf-modules/some-tf-module?ref=v0.1.0"
        // ...
    }

From the perspective of Terraform, such Git repositories are "remote" in
the same way that repositories on GitHub are.

Note that within a Terraform module "call" block, the filepaths
specified are relative to the directory in which the *.tf file lives,
not relative to the current working directory of the Terraform
process. In order to support this feature, Terraform needs to supply
that contextual information to go-getter to allow relative filepath
resolution to work. In order to do so, we needed to switch over to using
go-getter's new "Contextual Detector" API. It works in the same basic
way as the traditional Detector API, but allows us to provide this
additional information.

In keeping with the "keep things simple" comment in the commit message
of 2b2ac1f, we are here maintaining our custom go-getter detectors
in two places. Only now each is called FooCtxDetector rather than
FooDetector. Nevertheless, all except the GitCtxDetector do little more
than "pass through" delegation to its analogous FooDetector counterpart.

Fixes hashicorp#25488
Fixes hashicorp#21107

[0] hashicorp/go-getter#268
[1] hashicorp/go-getter#269
@salewski salewski linked a pull request Aug 31, 2020 that will close this issue
salewski added a commit to salewski/go-getter that referenced this issue Jun 5, 2023
…nd relative

This series of changesets introduces a feature that allows the 'git::'
forcing token to be used on local file system paths to reference Git
repositories. Both absolute paths and relative paths are supported. For
example:
    git::./some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::../../some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::/some/absolute/path/to/a/git-repo//some-subdir?ref=v4.5.6

Only filepaths that are prefixed with the 'git::' forcing token are
considered for processing.

Internally, go-getter transforms the provided string into a 'file://'
URI with an absolute filepath, with query string params and subdirectory
retained.

The rationale for using a 'file://' URI internally is that the Git clone
operation can already work with 'file://' URIs, and using them for this
feature allows us to leverage the existing go-getter URI-handling
machinery. That gets us support for query params (to clone a specific
git ref (tag, commit hash, ...)) "for free".

The rationale for using an absolute filepath (even when the provided
string is a relative filepath) is that (per RFC 1738 and RFC 8089) only
absolute filepaths are legitimate in 'file://' URIs. But more
importantly here, the Git clone operation only supports 'file://' URIs
with absolute paths.

Q: Why support this functionality at all?

   Why not just require that a source location use an absolute path in a
   'file://' URI explicitly if that's what is needed?

A: The primary reason is to allow support for relative filepaths to Git
   repos.

   There are use cases in which the absolute path cannot be known in
   advance, but a relative path to a Git repo is known.

   For example, when a Terraform project (or any Git-based project) uses
   Git submodules, it will know the relative location of the Git
   submodule repos, but cannot know the absolute path in advance because
   it will vary based on where the "superproject" repo is
   cloned. Nevertheless, those relative paths should be usable as
   clonable Git repos, and this mechanism would allow for that.

   Support for filepaths that are already absolute is provided mainly
   for symmetry. It would be surprising for the feature to work with
   relative file paths, but not for absolute filepaths.

For projects using Terraform, in particular, this feature (along with a
small change in the Terraform code to leverage it) enables the
non-fragile use of relative paths in a module "call" block, when
combined with Git submodules:

    module "my_module" {
        source = "git::../git-submodules/tf-modules/some-tf-module?ref=v0.1.0"
        // ...
    }

In the above example "superproject" Git repo (the one "calling" the
terraform module) knows the relative path to its own Git submodules
because they are embedded in a subdirectory beneath the top-level of the
"superproject" repo.

Two downstream Terraform issues that would require go-getter support for
this feature (or something like it) are at [0] and [1].

This first changeset in the series updates the README.md documentation
to note the new feature and provide examples.

[0] "Unable to use relative path to local Git module"
    hashicorp/terraform#25488

[1] "In 0.12, modules can no longer be installed from local git repositories at relative paths"
    hashicorp/terraform#21107

Design Notes
------------
In order for this feature to work, additional contextual information is
needed by the Git detector than can be provided using the existing
Detector API.

Internally, the Detector's Detect method does not pass along to the
Detector implementations all of the contextual information that it has
available. In particular, the forcing token and go-getter subdir
component are stripped out of the source string before invoking the
implementation's Detect method. In the particular case of the Git
detector, that means it cannot know that a 'git::' forcing token was
provided on an input string that otherwise looks like a file system
path. And /that/ means that it is not correct or safe for it to identify
any filepath string value as a Git repository.

Externally, callers (such as Terraform) already provide a value for the
'pwd' parameter of Detect, but it is not (necessarily) the location from
which a relative path in a 'git::' string should be resolved. In a
Terraform module (which may be in an arbitrary subdirectory from the
process current working directory), module "source" references that
contain relative paths must be interpreted relative to the location of
the module source file. Terraform has that information available, but in
the existing Detect API there is no way to convey it to go-getter.

Constraints
-----------
Additional Detector methods cannot be added without burdening all
existing detectors (both internal and in the wild) with the need to
support them.

Additional Detect method params cannot be added without breaking all
existing Detector implementations (internal, wild).

Additional parameters cannot be added to the Detect dispatching function
without affecting all callers.

Approach
--------
The goal is to provide the feature in a way that is as minimally
invasive as possible. But above all else it needs to avoid breaking
backward compatibility in any way.

Given that, the approach taken by this changeset series is to introduce
the concept of a "Contextual Detector". It is structured in the same way
as the current Detector framework, but works through a new CtxDetector
interface that is not constrained by the existing API.

The only callers affected by this change would be those that wish to take
advantage of the additional capabilities. And for those, the migration path
straight-forward because the new API is structured like the existing one.

In particular, this changeset series introduces four new elements:

    1. CtxDetector interface

    2. CtxDetect dispatching function

    3. CtxDetect method on the CtxDetector interface

    4. Full suite of CtxDetector implementations that are analogues of
       the existing detectors (most of which (currently) just delegate
       to the existing Detector implementations).

There is also a global 'ContextualDetectors' list that serves a function
analogous to the existing 'Detectors' list.
salewski added a commit to salewski/go-getter that referenced this issue Jun 6, 2023
…nd relative

This series of changesets introduces a feature that allows the 'git::'
forcing token to be used on local file system paths to reference Git
repositories. Both absolute paths and relative paths are supported. For
example:
    git::./some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::../../some/relative/path/to/a/git-repo//some-subdir?ref=v1.2.3
or:
    git::/some/absolute/path/to/a/git-repo//some-subdir?ref=v4.5.6

Only filepaths that are prefixed with the 'git::' forcing token are
considered for processing.

Internally, go-getter transforms the provided string into a 'file://'
URI with an absolute filepath, with query string params and subdirectory
retained.

The rationale for using a 'file://' URI internally is that the Git clone
operation can already work with 'file://' URIs, and using them for this
feature allows us to leverage the existing go-getter URI-handling
machinery. That gets us support for query params (to clone a specific
git ref (tag, commit hash, ...)) "for free".

The rationale for using an absolute filepath (even when the provided
string is a relative filepath) is that (per RFC 1738 and RFC 8089) only
absolute filepaths are legitimate in 'file://' URIs. But more
importantly here, the Git clone operation only supports 'file://' URIs
with absolute paths.

Q: Why support this functionality at all?

   Why not just require that a source location use an absolute path in a
   'file://' URI explicitly if that's what is needed?

A: The primary reason is to allow support for relative filepaths to Git
   repos.

   There are use cases in which the absolute path cannot be known in
   advance, but a relative path to a Git repo is known.

   For example, when a Terraform project (or any Git-based project) uses
   Git submodules, it will know the relative location of the Git
   submodule repos, but cannot know the absolute path in advance because
   it will vary based on where the "superproject" repo is
   cloned. Nevertheless, those relative paths should be usable as
   clonable Git repos, and this mechanism would allow for that.

   Support for filepaths that are already absolute is provided mainly
   for symmetry. It would be surprising for the feature to work with
   relative file paths, but not for absolute filepaths.

For projects using Terraform, in particular, this feature (along with a
small change in the Terraform code to leverage it) enables the
non-fragile use of relative paths in a module "call" block, when
combined with Git submodules:

    module "my_module" {
        source = "git::../git-submodules/tf-modules/some-tf-module?ref=v0.1.0"
        // ...
    }

In the above example "superproject" Git repo (the one "calling" the
terraform module) knows the relative path to its own Git submodules
because they are embedded in a subdirectory beneath the top-level of the
"superproject" repo.

Two downstream Terraform issues that would require go-getter support for
this feature (or something like it) are at [0] and [1].

This first changeset in the series updates the README.md documentation
to note the new feature and provide examples.

[0] "Unable to use relative path to local Git module"
    hashicorp/terraform#25488

[1] "In 0.12, modules can no longer be installed from local git repositories at relative paths"
    hashicorp/terraform#21107

Design Notes
------------
In order for this feature to work, additional contextual information is
needed by the Git detector than can be provided using the existing
Detector API.

Internally, the Detector's Detect method does not pass along to the
Detector implementations all of the contextual information that it has
available. In particular, the forcing token and go-getter subdir
component are stripped out of the source string before invoking the
implementation's Detect method. In the particular case of the Git
detector, that means it cannot know that a 'git::' forcing token was
provided on an input string that otherwise looks like a file system
path. And /that/ means that it is not correct or safe for it to identify
any filepath string value as a Git repository.

Externally, callers (such as Terraform) already provide a value for the
'pwd' parameter of Detect, but it is not (necessarily) the location from
which a relative path in a 'git::' string should be resolved. In a
Terraform module (which may be in an arbitrary subdirectory from the
process current working directory), module "source" references that
contain relative paths must be interpreted relative to the location of
the module source file. Terraform has that information available, but in
the existing Detect API there is no way to convey it to go-getter.

Constraints
-----------
Additional Detector methods cannot be added without burdening all
existing detectors (both internal and in the wild) with the need to
support them.

Additional Detect method params cannot be added without breaking all
existing Detector implementations (internal, wild).

Additional parameters cannot be added to the Detect dispatching function
without affecting all callers.

Approach
--------
The goal is to provide the feature in a way that is as minimally
invasive as possible. But above all else it needs to avoid breaking
backward compatibility in any way.

Given that, the approach taken by this changeset series is to introduce
the concept of a "Contextual Detector". It is structured in the same way
as the current Detector framework, but works through a new CtxDetector
interface that is not constrained by the existing API.

The only callers affected by this change would be those that wish to take
advantage of the additional capabilities. And for those, the migration path
straight-forward because the new API is structured like the existing one.

In particular, this changeset series introduces four new elements:

    1. CtxDetector interface

    2. CtxDetect dispatching function

    3. CtxDetect method on the CtxDetector interface

    4. Full suite of CtxDetector implementations that are analogues of
       the existing detectors (most of which (currently) just delegate
       to the existing Detector implementations).

There is also a global 'ContextualDetectors' list that serves a function
analogous to the existing 'Detectors' list.

Signed-off-by: Alan D. Salewski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug upstream v0.12 Issues (primarily bugs) reported against v0.12 releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.