depsfile: Flatten the "hashes" locks to a single set of strings

Although origin registries return specific [filename, hash] pairs, our
various different installation methods can't produce a structured mapping
from platform to hash without breaking changes.

Therefore, as a compromise, we'll continue to do platform-specific checks
against upstream data in the cases where that's possible (installation
from origin registry or network mirror) but we'll treat the lock file as
just a flat set of equally-valid hashes, at least one of which must match
after we've completed whatever checks we've made against the
upstream-provided checksums/signatures.

This includes only the minimal internal/getproviders updates required to
make this compile. A subsequent commit will update that package to
actually support the idea of verifying against multiple hashes.

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(hashes)
-		sort.Strings(slice)
-	}
 
 	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
+	// hashes contains zero or more hashes of packages or package contents
-	// for the package associated with the selected version on each supported
+	// for the package associated with the selected version across all of
-	// architecture.
+	// 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
+// AllHashes returns all of the package hashes that were recorded when this
-// the given platform when this lock was created. If no hashes were recorded
+// lock was created. If no hashes were recorded for that platform, the result
-// for that platform, the result is a zero-length slice.
+// 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
+// PreferredHashes to get only the hashes which the current version
-// of Terraform considers the strongest of the available hashes, which is
+// of Terraform considers the strongest of the available hashing schemes, one
-// the one that must pass for verification to be considered successful.
+// 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 {
+func (l *ProviderLock) AllHashes() []string {
-	return l.hashes[platform]
+	return l.hashes
 }
 
-// PreferredHashForPlatform returns a single hash which must match for a package
+// PreferredHashes returns a filtered version of the AllHashes return value
-// for the given platform to be considered valid, or an empty string if there
+// which includes only the strongest of the availabile hash schemes, in
-// are no acceptable hashes recorded for the given platform.
+// case legacy hash schemes are deprecated over time but still supported for
-func (l *ProviderLock) PreferredHashForPlatform(platform getproviders.Platform) string {
+// upgrade purposes.
-	return getproviders.PreferredHash(l.hashes[platform])
+//
+// 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)
 }

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))
+			hashVals := make([]cty.Value, 0, len(lock.hashes))
-			for platform := range lock.hashes {
+			for _, str := range lock.hashes {
-				platforms = append(platforms, platform)
+				hashVals = append(hashVals, cty.StringVal(str))
-			}
-			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)
 			}
+			// 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
+	content, hclDiags := block.Body.Content(&hcl.BodySchema{
-	// special structural validation to do other than what gohcl will naturally
+		Attributes: []hcl.AttributeSchema{
-	// do for us here.
+			{Name: "version", Required: true},
-	type RawHashes struct {
+			{Name: "constraints"},
-		// We'll consume all of the attributes and process them dynamically.
+			{Name: "hashes"},
-		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)
 	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, content.Attributes["hashes"])
-		hashes, moreDiags := decodeProviderHashesArgument(addr, raw.HashesBlock.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 {
+	constraints, err := getproviders.ParseVersionConstraints(raw)
-		// It's okay to omit this argument.
-		return nil, diags
-	}
-	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) {
+func decodeProviderHashesArgument(provider addrs.Provider, attr *hcl.Attribute) ([]string, tfdiags.Diagnostics) {
-	if len(attrs) == 0 {
-		return nil, nil
-	}
-	ret := make(map[getproviders.Platform][]string, len(attrs))
 	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 {
+	// We'll decode this argument using the HCL static analysis mode, because
-		platform, err := getproviders.ParsePlatform(platformStr)
+	// there's no reason for the hashes list to be dynamic and this way we can
-		if err != nil {
+	// 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 platform",
+			Summary:  "Invalid provider hash set",
-				Detail:   fmt.Sprintf("The string %q is not a valid platform specification: %s.", platformStr, err),
+			Detail:   "The \"hashes\" argument must either be omitted or contain at least one hash value.",
-				Subject:  attr.NameRange.Ptr(),
+			Subject:  expr.Range().Ptr(),
 		})
-			continue
+		return nil, diags
-		}
-		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
 	}
 
-		var hashes []string
+	ret := make([]string, 0, len(hashExprs))
-		hclDiags := gohcl.DecodeExpression(attr.Expr, nil, &hashes)
+	for _, hashExpr := range hashExprs {
+		var raw string
+		hclDiags := gohcl.DecodeExpression(hashExpr, nil, &raw)
 		diags = diags.Append(hclDiags)
 		if hclDiags.HasErrors() {
 			continue
 		}
-
+		// TODO: Validate the hash syntax, but not the actual hash schemes
-		// We don't validate the hashes, because we expect to support different
+		// because we expect to support different hash formats over time and
-		// hash formats over time and so we'll assume any that are in formats
+		// will silently ignore ones that we no longer prefer.
-		// we don't understand are from later Terraform versions, or perhaps
+		ret = append(ret, raw)
-		// from an origin registry that is offering hashes aimed at a later
-		// Terraform version.
-		ret[platform] = hashes
 	}
 
 	return ret, diags

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{
+						wantHashes := []string{
-							{OS: "amigaos", Arch: "m68k"}: {
+							"test:placeholder-hash-1",
-								"placeholder-hash-1",
+							"test:placeholder-hash-2",
-							},
+							"test:placeholder-hash-3",
-							{OS: "tos", Arch: "m68k"}: {
-								"placeholder-hash-2",
-								"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{
+	hashes := []string{
-		{OS: "riscos", Arch: "arm"}: {
+		"test:cccccccccccccccccccccccccccccccccccccccccccccccc",
-			"cccccccccccccccccccccccccccccccccccccccccccccccc",
+		"test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
-			"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+		"test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-			"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      = ["test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "test:cccccccccccccccccccccccccccccccccccccccccccccccc"]
-  hashes {
-    riscos_arm = ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "cccccccccccccccccccccccccccccccccccccccccccccccc"]
-  }
 }
 `
 	if diff := cmp.Diff(wantContent, gotContent); diff != "" {

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

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 {
+  hashes = [
-    amigaos_m68k = [
+    "test:placeholder-hash-1",
-      "placeholder-hash-1",
+    "test:placeholder-hash-2",
-    ]
+    "test:placeholder-hash-3",
-    tos_m68k = [
-      "placeholder-hash-2",
-      "placeholder-hash-3",
   ]
 }
-}

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

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,