-
-
Notifications
You must be signed in to change notification settings - Fork 1k
[WIP] feat: Support version constraints for Terraform Registry modules #3926
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
base: main
Are you sure you want to change the base?
Changes from all commits
e6f7cec
b90c34c
6846092
43731b8
c9132ce
0379fac
afde8a2
da6e938
9e2b8cb
ba62eb0
6cffb68
9dd43bd
5962867
9cd52a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# Retrieve a module from the public terraform registry to use with terragrunt | ||
locals { | ||
base_source = "tfr:///terraform-aws-modules/iam/aws" | ||
version = "~>5.0, <5.51.0, !=5.50.0" | ||
} | ||
terraform { | ||
source = "${local.base_source}?version=${local.version}" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,15 @@ import ( | |
"os" | ||
"path" | ||
"path/filepath" | ||
"sort" | ||
"strings" | ||
|
||
"github.com/gruntwork-io/terragrunt/options" | ||
"github.com/gruntwork-io/terragrunt/pkg/log" | ||
"github.com/hashicorp/go-cleanhttp" | ||
"github.com/hashicorp/go-getter" | ||
safetemp "github.com/hashicorp/go-safetemp" | ||
"github.com/hashicorp/go-version" | ||
|
||
"github.com/gruntwork-io/terragrunt/internal/errors" | ||
"github.com/gruntwork-io/terragrunt/util" | ||
|
@@ -40,6 +42,21 @@ type RegistryServicePath struct { | |
ModulesPath string `json:"modules.v1"` | ||
} | ||
|
||
// Modules is a struct for extracting the modules list from a versions endpoint in the Registry. | ||
type Modules struct { | ||
Modules []Module `json:"modules"` | ||
} | ||
|
||
// Module is a struct for extracting the module versions from Modules. | ||
type Module struct { | ||
ModuleVersions []ModuleVersion `json:"versions"` | ||
} | ||
|
||
// ModuleVersion is a struct for extracting the module version from Module. | ||
type ModuleVersion struct { | ||
Version string `json:"version"` | ||
} | ||
|
||
// RegistryGetter is a Getter (from go-getter) implementation that will download from the terraform module | ||
// registry. This supports getter URLs encoded in the following manner: | ||
// | ||
|
@@ -135,18 +152,30 @@ func (tfrGetter *RegistryGetter) Get(dstPath string, srcURL *url.URL) error { | |
return errors.New(MalformedRegistryURLErr{reason: "more than one version query"}) | ||
} | ||
|
||
version := versionList[0] | ||
versionQuery := versionList[0] | ||
|
||
moduleRegistryBasePath, err := GetModuleRegistryURLBasePath(ctx, tfrGetter.TerragruntOptions.Logger, registryDomain) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
moduleURL, err := BuildRequestURL(registryDomain, moduleRegistryBasePath, modulePath, version) | ||
moduleVersionsURL, err := BuildModuleVersionsURL(registryDomain, moduleRegistryBasePath, modulePath) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
targetVersion, err := GetTargetVersion(ctx, tfrGetter.TerragruntOptions.Logger, *moduleVersionsURL, versionQuery) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
moduleURL, err := BuildRequestURL(registryDomain, moduleRegistryBasePath, modulePath, targetVersion) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
tfrGetter.TerragruntOptions.Logger.Infof("Downloading module from %s", moduleURL.String()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure we need this log message at the Info level. It may produce multiple unused lines, so would it make more sense to set it to Debug instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have doubts also. However, since there's another Info log related to this and it includes the query string, I considered useful to have now this second log line clarifying which version is finally being downloaded without having to go some levels up (way far from getter.go) to change that behaviour. That said, I believe this is an intermediate step. The next one should be to implement the version attribute and properly handle the version constraints at an upper level (adding the ability to invalidate tg cache when there's a new version even if the code hasn't changed, among other changes). Then, both log lines should be addressed and merged into an more accurate one. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: Since the scope of the PR has been changed to also support a new version attribute, and this will imply modifying way more than the tf getter, I believe this logs issue could be addressed also at the root, so this log line in getter won't make sense anymore |
||
|
||
terraformGet, err := GetTerraformGetHeader(ctx, tfrGetter.TerragruntOptions.Logger, *moduleURL) | ||
if err != nil { | ||
return err | ||
|
@@ -268,6 +297,39 @@ func GetModuleRegistryURLBasePath(ctx context.Context, logger log.Logger, domain | |
return respJSON.ModulesPath, nil | ||
} | ||
|
||
// GetTargetVersion retrieves the target version of the module based on the version constraint provided. This function | ||
// will return the highest version that satisfies the version constraint. If no version satisfies the constraint, an | ||
// error will be returned. | ||
func GetTargetVersion(ctx context.Context, logger log.Logger, url url.URL, versionQuery string) (string, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears that this function is only used within the tf package. Would it make sense to mark it as private? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to make it private, but then I get an error on the getter_test :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: Since the scope of the PR has been changed to also support a new version attribute, I believe I'll use this function from other places. Let's wait until that refactor is done to evaluate again. |
||
body, _, err := httpGETAndGetResponse(ctx, logger, url) | ||
if err != nil { | ||
return "", errors.New(ModuleVersionsFetchErr{sourceURL: url.String()}) | ||
} | ||
|
||
var responseJSON Modules | ||
if err := json.Unmarshal(body, &responseJSON); err != nil { | ||
return "", errors.New(ModuleVersionsFetchErr{sourceURL: url.String()}) | ||
} | ||
|
||
if len(responseJSON.Modules) == 0 || len(responseJSON.Modules[0].ModuleVersions) == 0 { | ||
return "", errors.New(ModuleVersionsFetchErr{sourceURL: url.String()}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears that we're reusing the same ModuleVersionsFetchErr error for multiple scenarios. This could be confusing to users, as the same error is thrown when the GET request fails, when the JSON response cannot be parsed, or when no module versions are found. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I agree with you, many of those errors are a consequence of the same problem (network issue or lacking proper authentication). As an example, both JSON response parsing error and no ModuleVersions in the response are a consequence of authentication lack OR module not existing, and for certain registries you can't know in advance. As an example, this is a non existing org/module but there's still a partial answer, which is exactly the same as when the org/module exists but the auth is missing or wrong. On the other hand, TFE seems to better implement this for their private registry: at least they return an error. However, since the answers differ from one implementation to another, I consider it's useful to condense these errors into a more abstract one (FetchErr). What I could do for sure is to create separate errors for the GET request and these other tricky cases, and try to refine as much as possible. Let me know if you have any ideas here, please. |
||
} | ||
|
||
// Filter the available module versions based on the version constraint to get the compatible versions | ||
compatibleVersions, err := getCompatibleVersions(responseJSON.Modules[0].ModuleVersions, versionQuery) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
// Get the highest version from the compatible versions | ||
targetVersion := getHighestVersion(compatibleVersions) | ||
if targetVersion == "" { | ||
return "", errors.New(ModuleVersionConstraintErr{versionConstraint: versionQuery}) | ||
} | ||
|
||
return targetVersion, nil | ||
} | ||
|
||
// GetTerraformGetHeader makes an http GET call to the given registry URL and return the contents of location json | ||
// body or the header X-Terraform-Get. This function will return an error if the response does not contain the header. | ||
func GetTerraformGetHeader(ctx context.Context, logger log.Logger, url url.URL) (string, error) { | ||
|
@@ -361,6 +423,26 @@ func httpGETAndGetResponse(ctx context.Context, logger log.Logger, getURL url.UR | |
return bodyData, &resp.Header, errors.New(err) | ||
} | ||
|
||
// BuildModuleVersionsURL - create url to fetch module versions using moduleRegistryBasePath. | ||
func BuildModuleVersionsURL(registryDomain string, moduleRegistryBasePath string, modulePath string) (*url.URL, error) { | ||
moduleRegistryBasePath = strings.TrimSuffix(moduleRegistryBasePath, "/") | ||
modulePath = strings.TrimSuffix(modulePath, "/") | ||
modulePath = strings.TrimPrefix(modulePath, "/") | ||
|
||
moduleVersionsPath := fmt.Sprintf("%s/%s/versions", moduleRegistryBasePath, modulePath) | ||
|
||
moduleVersionsURL, err := url.Parse(moduleVersionsPath) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if moduleVersionsURL.Scheme != "" { | ||
return moduleVersionsURL, nil | ||
} | ||
|
||
return &url.URL{Scheme: "https", Host: registryDomain, Path: moduleVersionsPath}, nil | ||
} | ||
|
||
// BuildRequestURL - create url to download module using moduleRegistryBasePath | ||
func BuildRequestURL(registryDomain string, moduleRegistryBasePath string, modulePath string, version string) (*url.URL, error) { | ||
moduleRegistryBasePath = strings.TrimSuffix(moduleRegistryBasePath, "/") | ||
|
@@ -380,3 +462,38 @@ func BuildRequestURL(registryDomain string, moduleRegistryBasePath string, modul | |
|
||
return &url.URL{Scheme: "https", Host: registryDomain, Path: moduleFullPath}, nil | ||
} | ||
|
||
// getHighestVersion returns the highest version from the list of versions, or an empty string if the list is empty. | ||
func getHighestVersion(availableVersions []*version.Version) string { | ||
if len(availableVersions) == 0 { | ||
return "" | ||
} | ||
|
||
sort.Sort(version.Collection(availableVersions)) | ||
|
||
return availableVersions[len(availableVersions)-1].String() | ||
} | ||
|
||
// getCompatibleVersions returns the list of versions within availableVersions that satisfy the version | ||
// constraint defined by versionQuery. | ||
func getCompatibleVersions(availableVersions []ModuleVersion, versionQuery string) ([]*version.Version, error) { | ||
var compatibleVersions []*version.Version | ||
|
||
versionConstraint, err := version.NewConstraint(versionQuery) | ||
if err != nil { | ||
return nil, errors.New(ModuleVersionConstraintMalformedErr{versionConstraint: versionQuery}) | ||
} | ||
|
||
for _, availableVersion := range availableVersions { | ||
availableVersionParsed, err := version.NewVersion(availableVersion.Version) | ||
if err != nil { | ||
return nil, errors.New(ModuleVersionMalformedErr{version: availableVersion.Version}) | ||
} | ||
|
||
if versionConstraint.Check(availableVersionParsed) { | ||
compatibleVersions = append(compatibleVersions, availableVersionParsed) | ||
} | ||
} | ||
|
||
return compatibleVersions, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need a test for the error case too, current tests only cover happy path and won't catch regression in error handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'm completely open to it. Do you have any idea or clarification about the path I should follow? In fact I tried to make the test more accurate by reading the stdout and trying to read the info log-line where the version picked was shown (to check the picked version), but I discovered logs are at a different level and this was not possible. Any help here is very welcome!