github.com Source Shorthand could support ?ref=COMMIT_HASH without double slashes

Current Terraform Version

v1.1.6

The github.com source convenience setup works with subdirectories as modules WITHOUT double slashes. But when you try to pin it to a commit with ?ref=COMMIT_HASH this convenience functionality breaks.

I'm guessing for historical reasons based on how github.com source is defined outside the standard setup (I couldn't find the actual logic): https://github.com/hashicorp/terraform/blob/dc63fda44b67300d5161dabcd803426d0d2f468e/internal/addrs/module_source.go#L295.

I was newish user, was happy that the following just worked:

module "consul" { source = "github.com/hashicorp/example/modules/vpc" }

A week later, when I tried to pin my module, it breaks:

module "consul" { source = "github.com/hashicorp/example/modules/vpc?ref=v1.2.0" }

The solution is figuring out you were supposed to use a double slash all along (mentioned at the very end of the docs with no other subdir examples on the page.

This works:

module "consul" { source = "github.com/hashicorp/example//modules/vpc?ref=v1.2.0" }

I updated the docs with an example, but it seems that the github.com convenience functionality should also support ?ref=COMMIT_HASH

Thanks for reporting this, @dhodun.

Terraform's Git installation behavior lives in an upstream library called go-getter, which includes the concept of "detectors" which try to recognize various shorthands and rewrite them into the canonical form. I believe the "github" detector is the one relevant to the behavior you are describing.

At the time of writing, it has the following implementation:

    parts := strings.Split(src, "/")
    if len(parts) < 3 {
        return "", false, fmt.Errorf(
            "GitHub URLs should be github.com/username/repo")
    }

    urlStr := fmt.Sprintf("https://%s", strings.Join(parts[:3], "/"))
    url, err := url.Parse(urlStr)
    if err != nil {
        return "", true, fmt.Errorf("error parsing GitHub URL: %s", err)
    }

    if !strings.HasSuffix(url.Path, ".git") {
        url.Path += ".git"
    }

    if len(parts) > 3 {
        url.Path += "//" + strings.Join(parts[3:], "/")
    }

    return "git::" + url.String(), true, nil

If we work through that logic with your github.com/hashicorp/example/modules/vpc?ref=v1.2.0 example, I guess it behaves like this:

Split to []string{"github.com", "hashicorp", "example", "modules", "vpc?ref=v1.2.0"} as `parts
Concatenate the first three parts back together again, so that urlStr is "https://github.com/hashicorp/example", and then parse that as a URL to produce url
The path portion of url doesn't have a .git suffix, so add one to produce https://github.com/hashicorp/example.git
The original parts had more than three parts, so treat them as the package subdir part, joining by slashes, making https://github.com/hashicorp/example.git//example/modules/vpc?ref=v1.2.0
Finally, add the git:: prefix which explicitly specifies which get mode this should use (the "git getter"), so the final string is "git::https://github.com/hashicorp/example.git//example/modules/vpc?ref=v1.2.0".

Given this, it isn't really clear to me why this would fail, since the result does seem to be a valid canonical source address string.

Could you add a new comment showing the terraform init output with both the form that worked for you (with the // portion) and the form that didn't? I'm interested to see how go-getter is normalizing both forms; terraform init should mention in its output that it is installing each module, and when doing so it should report the normalized version that resulted from go-getter's "detection" logic, and so we should be able to confirm from that whether my mental execution of the code above was correct, and if not try to understand why it behaved the way it did. (for example, maybe there's some other logic involved that I'm not considering)

With the benefit of hindsight, it seems unfortunate that the git detector hides the separation between the repository URL (the "package URL" from Terraform's perspective) and the subdirectory path here, since as you experience it makes things seem to work in the simple case and thus allows you to avoid learning about the more general form until you get a presumably-confusing error message about it.

However, it is what it is and I'd like to see if there's something we can do to make it work better. We are constrained by backward compatibility and the git detection logic in go-getter has historically been very difficult to evolve in a backward-compatible way, so there is a possibility that the resolution here may be to just mention the special case about the package-subdir boundary being implied in the GitHub section of the docs, and link to the documentation about subdirectories as the necessary context. However, if we can improve the behavior to make this work then I think it would make sense to do so, because it seems like what you tried was intended to work, from reading between the lines in the go-getter implementation.

hashicorp / terraform

github.com Source Shorthand could support ?ref=COMMIT_HASH without double slashes #30593

Current Terraform Version