diff --git a/internal/depsfile/locks.go b/internal/depsfile/locks.go index 17961ae4dd..d676f9cb81 100644 --- a/internal/depsfile/locks.go +++ b/internal/depsfile/locks.go @@ -58,15 +58,13 @@ func (l *Locks) Provider(addr addrs.Provider) *ProviderLock { // non-lockable provider address then this function will panic. Use // function ProviderIsLockable to determine whether a particular provider // should participate in the version locking mechanism. -func (l *Locks) SetProvider(addr addrs.Provider, version getproviders.Version, constraints getproviders.VersionConstraints, hashes map[getproviders.Platform][]string) *ProviderLock { +func (l *Locks) SetProvider(addr addrs.Provider, version getproviders.Version, constraints getproviders.VersionConstraints, hashes []string) *ProviderLock { if !ProviderIsLockable(addr) { panic(fmt.Sprintf("Locks.SetProvider with non-lockable provider %s", addr)) } // Normalize the hash lists into a consistent order. - for _, slice := range hashes { - sort.Strings(slice) - } + sort.Strings(hashes) new := &ProviderLock{ addr: addr, @@ -112,9 +110,9 @@ type ProviderLock struct { version getproviders.Version versionConstraints getproviders.VersionConstraints - // hashes contains one or more hashes of packages or package contents - // for the package associated with the selected version on each supported - // architecture. + // hashes contains zero or more hashes of packages or package contents + // for the package associated with the selected version across all of + // the supported platforms. // // hashes can contain a mixture of hashes in different formats to support // changes over time. The new-style hash format is to have a string @@ -131,7 +129,7 @@ type ProviderLock struct { // when we have the original .zip file exactly; we can't verify a local // directory containing the unpacked contents of that .zip file. // - // We ideally want to populate hashes for all available architectures at + // We ideally want to populate hashes for all available platforms at // once, by referring to the signed checksums file in the upstream // registry. In that ideal case it's possible to later work with the same // configuration on a different platform while still verifying the hashes. @@ -139,7 +137,7 @@ type ProviderLock struct { // means we can only populate the hash for the current platform, and so // it won't be possible to verify a subsequent installation of the same // provider on a different platform. - hashes map[getproviders.Platform][]string + hashes []string } // Provider returns the address of the provider this lock applies to. @@ -164,23 +162,27 @@ func (l *ProviderLock) VersionConstraints() getproviders.VersionConstraints { return l.versionConstraints } -// HashesForPlatform returns all of the package hashes that were recorded for -// the given platform when this lock was created. If no hashes were recorded -// for that platform, the result is a zero-length slice. +// AllHashes returns all of the package hashes that were recorded when this +// lock was created. If no hashes were recorded for that platform, the result +// is a zero-length slice. // // If your intent is to verify a package against the recorded hashes, use -// PreferredHashForPlatform to get a single hash which the current version -// of Terraform considers the strongest of the available hashes, which is -// the one that must pass for verification to be considered successful. +// PreferredHashes to get only the hashes which the current version +// of Terraform considers the strongest of the available hashing schemes, one +// of which must match in order for verification to be considered successful. // // Do not modify the backing array of the returned slice. -func (l *ProviderLock) HashesForPlatform(platform getproviders.Platform) []string { - return l.hashes[platform] +func (l *ProviderLock) AllHashes() []string { + return l.hashes } -// PreferredHashForPlatform returns a single hash which must match for a package -// for the given platform to be considered valid, or an empty string if there -// are no acceptable hashes recorded for the given platform. -func (l *ProviderLock) PreferredHashForPlatform(platform getproviders.Platform) string { - return getproviders.PreferredHash(l.hashes[platform]) +// PreferredHashes returns a filtered version of the AllHashes return value +// which includes only the strongest of the availabile hash schemes, in +// case legacy hash schemes are deprecated over time but still supported for +// upgrade purposes. +// +// At least one of the given hashes must match for a package to be considered +// valud. +func (l *ProviderLock) PreferredHashes() []string { + return getproviders.PreferredHashes(l.hashes) } diff --git a/internal/depsfile/locks_file.go b/internal/depsfile/locks_file.go index ae58dcea5c..dd9fc18403 100644 --- a/internal/depsfile/locks_file.go +++ b/internal/depsfile/locks_file.go @@ -101,29 +101,15 @@ func SaveLocksToFile(locks *Locks, filename string) tfdiags.Diagnostics { body.SetAttributeValue("constraints", cty.StringVal(constraintsStr)) } if len(lock.hashes) != 0 { - platforms := make([]getproviders.Platform, 0, len(lock.hashes)) - for platform := range lock.hashes { - platforms = append(platforms, platform) - } - sort.Slice(platforms, func(i, j int) bool { - return platforms[i].LessThan(platforms[j]) - }) - body.AppendNewline() - hashesBlock := body.AppendNewBlock("hashes", nil) - hashesBody := hashesBlock.Body() - for platform, hashes := range lock.hashes { - vals := make([]cty.Value, len(hashes)) - for i := range hashes { - vals[i] = cty.StringVal(hashes[i]) - } - var hashList cty.Value - if len(vals) > 0 { - hashList = cty.ListVal(vals) - } else { - hashList = cty.ListValEmpty(cty.String) - } - hashesBody.SetAttributeValue(platform.String(), hashList) + hashVals := make([]cty.Value, 0, len(lock.hashes)) + for _, str := range lock.hashes { + hashVals = append(hashVals, cty.StringVal(str)) } + // We're using a set rather than a list here because the order + // isn't significant and SetAttributeValue will automatically + // write the set elements in a consistent lexical order. + hashSet := cty.SetVal(hashVals) + body.SetAttributeValue("hashes", hashSet) } } @@ -276,44 +262,38 @@ func decodeProviderLockFromHCL(block *hcl.Block) (*ProviderLock, tfdiags.Diagnos ret.addr = addr - // We'll decode the block body using gohcl, because we don't have any - // special structural validation to do other than what gohcl will naturally - // do for us here. - type RawHashes struct { - // We'll consume all of the attributes and process them dynamically. - Hashes hcl.Attributes `hcl:",remain"` - } - type Provider struct { - Version hcl.Expression `hcl:"version,attr"` - VersionConstraints hcl.Expression `hcl:"constraints,attr"` - HashesBlock *RawHashes `hcl:"hashes,block"` - } - var raw Provider - hclDiags := gohcl.DecodeBody(block.Body, nil, &raw) + content, hclDiags := block.Body.Content(&hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + {Name: "version", Required: true}, + {Name: "constraints"}, + {Name: "hashes"}, + }, + }) diags = diags.Append(hclDiags) - if hclDiags.HasErrors() { - return ret, diags - } - version, moreDiags := decodeProviderVersionArgument(addr, raw.Version) + version, moreDiags := decodeProviderVersionArgument(addr, content.Attributes["version"]) ret.version = version diags = diags.Append(moreDiags) - constraints, moreDiags := decodeProviderVersionConstraintsArgument(addr, raw.VersionConstraints) + constraints, moreDiags := decodeProviderVersionConstraintsArgument(addr, content.Attributes["constraints"]) ret.versionConstraints = constraints diags = diags.Append(moreDiags) - if raw.HashesBlock != nil { - hashes, moreDiags := decodeProviderHashesArgument(addr, raw.HashesBlock.Hashes) - ret.hashes = hashes - diags = diags.Append(moreDiags) - } + hashes, moreDiags := decodeProviderHashesArgument(addr, content.Attributes["hashes"]) + ret.hashes = hashes + diags = diags.Append(moreDiags) return ret, diags } -func decodeProviderVersionArgument(provider addrs.Provider, expr hcl.Expression) (getproviders.Version, tfdiags.Diagnostics) { +func decodeProviderVersionArgument(provider addrs.Provider, attr *hcl.Attribute) (getproviders.Version, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + if attr == nil { + // It's not okay to omit this argument, but the caller should already + // have generated diagnostics about that. + return getproviders.UnspecifiedVersion, diags + } + expr := attr.Expr var raw *string hclDiags := gohcl.DecodeExpression(expr, nil, &raw) @@ -334,7 +314,7 @@ func decodeProviderVersionArgument(provider addrs.Provider, expr hcl.Expression) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid version number", + Summary: "Invalid provider version number", Detail: fmt.Sprintf("The selected version number for provider %s is invalid: %s.", provider, err), Subject: expr.Range().Ptr(), }) @@ -344,7 +324,7 @@ func decodeProviderVersionArgument(provider addrs.Provider, expr hcl.Expression) // that a file diff will show changes that are entirely cosmetic. diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid version number", + Summary: "Invalid provider version number", Detail: fmt.Sprintf("The selected version number for provider %s must be written in normalized form: %q.", provider, canon), Subject: expr.Range().Ptr(), }) @@ -352,34 +332,35 @@ func decodeProviderVersionArgument(provider addrs.Provider, expr hcl.Expression) return version, diags } -func decodeProviderVersionConstraintsArgument(provider addrs.Provider, expr hcl.Expression) (getproviders.VersionConstraints, tfdiags.Diagnostics) { +func decodeProviderVersionConstraintsArgument(provider addrs.Provider, attr *hcl.Attribute) (getproviders.VersionConstraints, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + if attr == nil { + // It's okay to omit this argument. + return nil, diags + } + expr := attr.Expr - var raw *string + var raw string hclDiags := gohcl.DecodeExpression(expr, nil, &raw) diags = diags.Append(hclDiags) if hclDiags.HasErrors() { return nil, diags } - if raw == nil { - // It's okay to omit this argument. - return nil, diags - } - constraints, err := getproviders.ParseVersionConstraints(*raw) + constraints, err := getproviders.ParseVersionConstraints(raw) if err != nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid version constraints", + Summary: "Invalid provider version constraints", Detail: fmt.Sprintf("The recorded version constraints for provider %s are invalid: %s.", provider, err), Subject: expr.Range().Ptr(), }) } - if canon := getproviders.VersionConstraintsString(constraints); canon != *raw { + if canon := getproviders.VersionConstraintsString(constraints); canon != raw { // Canonical forms are required in the lock file, to reduce the risk // that a file diff will show changes that are entirely cosmetic. diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Invalid version constraints", + Summary: "Invalid provider version constraints", Detail: fmt.Sprintf("The recorded version constraints for provider %s must be written in normalized form: %q.", provider, canon), Subject: expr.Range().Ptr(), }) @@ -388,49 +369,45 @@ func decodeProviderVersionConstraintsArgument(provider addrs.Provider, expr hcl. return constraints, diags } -func decodeProviderHashesArgument(provider addrs.Provider, attrs hcl.Attributes) (map[getproviders.Platform][]string, tfdiags.Diagnostics) { - if len(attrs) == 0 { - return nil, nil - } - ret := make(map[getproviders.Platform][]string, len(attrs)) +func decodeProviderHashesArgument(provider addrs.Provider, attr *hcl.Attribute) ([]string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics + if attr == nil { + // It's okay to omit this argument. + return nil, diags + } + expr := attr.Expr - for platformStr, attr := range attrs { - platform, err := getproviders.ParsePlatform(platformStr) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid provider hash platform", - Detail: fmt.Sprintf("The string %q is not a valid platform specification: %s.", platformStr, err), - Subject: attr.NameRange.Ptr(), - }) - continue - } - if canon := platform.String(); canon != platformStr { - // Canonical forms are required in the lock file, to reduce the risk - // that a file diff will show changes that are entirely cosmetic. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid provider hash platform", - Detail: fmt.Sprintf("The platform specification %q must be written in the normalized form %q.", platformStr, canon), - Subject: attr.NameRange.Ptr(), - }) - continue - } + // We'll decode this argument using the HCL static analysis mode, because + // there's no reason for the hashes list to be dynamic and this way we can + // give more precise feedback on individual elements that are invalid, + // with direct source locations. + hashExprs, hclDiags := hcl.ExprList(expr) + diags = diags.Append(hclDiags) + if hclDiags.HasErrors() { + return nil, diags + } + if len(hashExprs) == 0 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid provider hash set", + Detail: "The \"hashes\" argument must either be omitted or contain at least one hash value.", + Subject: expr.Range().Ptr(), + }) + return nil, diags + } - var hashes []string - hclDiags := gohcl.DecodeExpression(attr.Expr, nil, &hashes) + ret := make([]string, 0, len(hashExprs)) + for _, hashExpr := range hashExprs { + var raw string + hclDiags := gohcl.DecodeExpression(hashExpr, nil, &raw) diags = diags.Append(hclDiags) if hclDiags.HasErrors() { continue } - - // We don't validate the hashes, because we expect to support different - // hash formats over time and so we'll assume any that are in formats - // we don't understand are from later Terraform versions, or perhaps - // from an origin registry that is offering hashes aimed at a later - // Terraform version. - ret[platform] = hashes + // TODO: Validate the hash syntax, but not the actual hash schemes + // because we expect to support different hash formats over time and + // will silently ignore ones that we no longer prefer. + ret = append(ret, raw) } return ret, diags diff --git a/internal/depsfile/locks_file_test.go b/internal/depsfile/locks_file_test.go index a9b8dabf1e..2dd67bb6a8 100644 --- a/internal/depsfile/locks_file_test.go +++ b/internal/depsfile/locks_file_test.go @@ -144,14 +144,10 @@ func TestLoadLocksFromFile(t *testing.T) { if got, want := getproviders.VersionConstraintsString(lock.VersionConstraints()), ">= 3.0.2"; got != want { t.Errorf("wrong version constraints\ngot: %s\nwant: %s", got, want) } - wantHashes := map[getproviders.Platform][]string{ - {OS: "amigaos", Arch: "m68k"}: { - "placeholder-hash-1", - }, - {OS: "tos", Arch: "m68k"}: { - "placeholder-hash-2", - "placeholder-hash-3", - }, + wantHashes := []string{ + "test:placeholder-hash-1", + "test:placeholder-hash-2", + "test:placeholder-hash-3", } if diff := cmp.Diff(wantHashes, lock.hashes); diff != "" { t.Errorf("wrong hashes\n%s", diff) @@ -173,12 +169,10 @@ func TestSaveLocksToFile(t *testing.T) { oneDotTwo := getproviders.MustParseVersion("1.2.0") atLeastOneDotOh := getproviders.MustParseVersionConstraints(">= 1.0.0") pessimisticOneDotOh := getproviders.MustParseVersionConstraints("~> 1") - hashes := map[getproviders.Platform][]string{ - {OS: "riscos", Arch: "arm"}: { - "cccccccccccccccccccccccccccccccccccccccccccccccc", - "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - }, + hashes := []string{ + "test:cccccccccccccccccccccccccccccccccccccccccccccccc", + "test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", } locks.SetProvider(fooProvider, oneDotOh, atLeastOneDotOh, hashes) locks.SetProvider(barProvider, oneDotTwo, pessimisticOneDotOh, nil) @@ -216,10 +210,7 @@ provider "registry.terraform.io/test/baz" { provider "registry.terraform.io/test/foo" { version = "1.0.0" constraints = ">= 1.0.0" - - hashes { - riscos_arm = ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccccccccc"] - } + hashes = ["test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "test:cccccccccccccccccccccccccccccccccccccccccccccccc"] } ` if diff := cmp.Diff(wantContent, gotContent); diff != "" { diff --git a/internal/depsfile/testdata/locks-files/invalid-versions.hcl b/internal/depsfile/testdata/locks-files/invalid-versions.hcl index c2f765fcde..d1e96474f8 100644 --- a/internal/depsfile/testdata/locks-files/invalid-versions.hcl +++ b/internal/depsfile/testdata/locks-files/invalid-versions.hcl @@ -1,25 +1,25 @@ provider "terraform.io/test/foo" { - version = "" # ERROR: Invalid version number + version = "" # ERROR: Invalid provider version number } provider "terraform.io/test/bar" { # The "v" prefix is not expected here - version = "v1.0.0" # ERROR: Invalid version number + version = "v1.0.0" # ERROR: Invalid provider version number } provider "terraform.io/test/baz" { # Must be written in the canonical form, with three parts - version = "1.0" # ERROR: Invalid version number + version = "1.0" # ERROR: Invalid provider version number } provider "terraform.io/test/boop" { # Must be written in the canonical form, with three parts - version = "1" # ERROR: Invalid version number + version = "1" # ERROR: Invalid provider version number } provider "terraform.io/test/blep" { # Mustn't use redundant extra zero padding - version = "1.02" # ERROR: Invalid version number + version = "1.02" # ERROR: Invalid provider version number } provider "terraform.io/test/huzzah" { # ERROR: Missing required argument diff --git a/internal/depsfile/testdata/locks-files/valid-provider-locks.hcl b/internal/depsfile/testdata/locks-files/valid-provider-locks.hcl index 878c8d0c2e..1d19116076 100644 --- a/internal/depsfile/testdata/locks-files/valid-provider-locks.hcl +++ b/internal/depsfile/testdata/locks-files/valid-provider-locks.hcl @@ -12,13 +12,9 @@ provider "terraform.io/test/all-the-things" { version = "3.0.10" constraints = ">= 3.0.2" - hashes { - amigaos_m68k = [ - "placeholder-hash-1", - ] - tos_m68k = [ - "placeholder-hash-2", - "placeholder-hash-3", - ] - } + hashes = [ + "test:placeholder-hash-1", + "test:placeholder-hash-2", + "test:placeholder-hash-3", + ] } diff --git a/internal/getproviders/hash.go b/internal/getproviders/hash.go index 2fbf7f6156..dfbd6c15dd 100644 --- a/internal/getproviders/hash.go +++ b/internal/getproviders/hash.go @@ -57,7 +57,7 @@ func PackageMatchesHash(loc PackageLocation, want string) (bool, error) { } } -// PreferredHash examines all of the given hash strings and returns the one +// PreferredHashes examines all of the given hash strings and returns the one // that the current version of Terraform considers to provide the strongest // verification. // @@ -65,13 +65,14 @@ func PackageMatchesHash(loc PackageLocation, want string) (bool, error) { // format. If PreferredHash returns a non-empty string then it will be one // of the hash strings in "given", and that hash is the one that must pass // verification in order for a package to be considered valid. -func PreferredHash(given []string) string { +func PreferredHashes(given []string) []string { + var ret []string for _, s := range given { if strings.HasPrefix(s, h1Prefix) { - return s + return append(ret, s) } } - return "" + return ret } // PackageHashLegacyZipSHA implements the old provider package hashing scheme diff --git a/internal/getproviders/package_authentication.go b/internal/getproviders/package_authentication.go index 0a0ef2203e..fd38f15b9e 100644 --- a/internal/getproviders/package_authentication.go +++ b/internal/getproviders/package_authentication.go @@ -216,7 +216,12 @@ type packageHashAuthentication struct { // considered by Terraform to be the strongest verification, and authentication // succeeds as long as that chosen hash matches. func NewPackageHashAuthentication(platform Platform, validHashes []string) PackageAuthentication { - requiredHash := PreferredHash(validHashes) + requiredHashes := PreferredHashes(validHashes) + // TODO: Update to support multiple hashes + var requiredHash string + if len(requiredHashes) > 0 { + requiredHash = requiredHashes[0] + } return packageHashAuthentication{ RequiredHash: requiredHash, ValidHashes: validHashes,