From 7a27366b39cdba9d8d2004485a8b7c2af8dcdfae Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Mon, 14 Jul 2025 13:52:27 +0100 Subject: [PATCH] PSS: Add ability to create hashes of the `state_store` block and of its nested `provider` block (#37278) * Add the ability to make a hash of state store config * Add test demonstrating that the provider block doesn't impact the hash of a state_store block * Make sure test asserts what would happen if the schema DID include the provider block * Update the Hash method to return diagnostics, ignore nested provider blocks, and validate incoming schema and config * Update tests to use more representative config, fix code under test as a result * Update Hash method to return hashes for both the state_store and provider blocks * Add test cases to cover how required fields are tolerated when making the hash This is because ENVs may supply those values. * Fix inaccurate comments * Add test to show that hashes are consistent and exclude the provider block * Update backend state file to contain hash of provider block's config * Fix test to expect a hash for the provider config block. * Fix bug in DeepCopy method, update test to have better error messages when diffs are detected * Update test to explicitly check hash values * Try make test intention clearer * Improve user feedback when state store schema contains the protected word "provider" * Update tests * Update test to test the Hash method in a more true-to-life way Copy of 04a1201878cd1f6f117c43c43c1ee9d0fc17cec1 by Radek Simko * Update test to use new approach * Fix `TestInit_stateStoreBlockIsExperimental` test failure --- .../command/workdir/backend_state_test.go | 27 +- .../workdir/statestore_config_state.go | 4 +- internal/command/workdir/testing.go | 2 + internal/configs/backend.go | 2 +- internal/configs/parser_config.go | 3 + internal/configs/state_store.go | 90 +++++- internal/configs/state_store_test.go | 280 ++++++++++++++++++ 7 files changed, 395 insertions(+), 13 deletions(-) create mode 100644 internal/configs/state_store_test.go diff --git a/internal/command/workdir/backend_state_test.go b/internal/command/workdir/backend_state_test.go index e752729de3..52e5c6a461 100644 --- a/internal/command/workdir/backend_state_test.go +++ b/internal/command/workdir/backend_state_test.go @@ -5,7 +5,6 @@ package workdir import ( "encoding/json" - "reflect" "strings" "testing" @@ -57,7 +56,8 @@ func TestParseBackendStateFile(t *testing.T) { "terraform_version": "0.8.0", "backend": { "type": "treasure_chest_buried_on_a_remote_island", - "config": {} + "config": {}, + "hash" : 12345 } }`, Want: &BackendStateFile{ @@ -66,6 +66,7 @@ func TestParseBackendStateFile(t *testing.T) { Backend: &BackendConfigState{ Type: "treasure_chest_buried_on_a_remote_island", ConfigRaw: json.RawMessage("{}"), + Hash: 12345, }, }, }, @@ -84,8 +85,10 @@ func TestParseBackendStateFile(t *testing.T) { "source": "registry.terraform.io/my-org/foobar", "config": { "credentials": "./creds.json" - } - } + }, + "hash" : 12345 + }, + "hash" : 12345 } }`, Want: &BackendStateFile{ @@ -101,6 +104,7 @@ func TestParseBackendStateFile(t *testing.T) { "bucket": "my-bucket", "region": "saturn" }`), + Hash: 12345, }, }, }, @@ -110,7 +114,8 @@ func TestParseBackendStateFile(t *testing.T) { "terraform_version": "9.9.9", "backend": { "type": "treasure_chest_buried_on_a_remote_island", - "config": {} + "config": {}, + "hash" : 12345 }, "state_store": { "type": "foobar_baz", @@ -120,8 +125,10 @@ func TestParseBackendStateFile(t *testing.T) { }, "provider": { "version": "1.2.3", - "source": "registry.terraform.io/my-org/foobar" - } + "source": "registry.terraform.io/my-org/foobar", + "hash" : 12345 + }, + "hash" : 12345 } }`, WantErr: `encountered a malformed backend state file that contains state for both a 'backend' and a 'state_store' block`, @@ -168,7 +175,7 @@ func TestEncodeBackendStateFile(t *testing.T) { Hash: 123, }, }, - Want: []byte("{\n \"version\": 3,\n \"terraform_version\": \"" + tfVersion + "\",\n \"state_store\": {\n \"type\": \"foobar_baz\",\n \"provider\": {\n \"version\": \"1.2.3\",\n \"source\": \"registry.terraform.io/my-org/foobar\",\n \"config\": {\n \"foo\": \"bar\"\n }\n },\n \"config\": {\n \"foo\": \"bar\"\n },\n \"hash\": 123\n }\n}"), + Want: []byte("{\n \"version\": 3,\n \"terraform_version\": \"" + tfVersion + "\",\n \"state_store\": {\n \"type\": \"foobar_baz\",\n \"provider\": {\n \"version\": \"1.2.3\",\n \"source\": \"registry.terraform.io/my-org/foobar\",\n \"config\": {\n \"foo\": \"bar\"\n },\n \"hash\": 12345\n },\n \"config\": {\n \"foo\": \"bar\"\n },\n \"hash\": 123\n }\n}"), }, "it returns an error when neither backend nor state_store config state are present": { Input: &BackendStateFile{}, @@ -286,8 +293,8 @@ func TestBackendStateFile_DeepCopy(t *testing.T) { t.Run(tn, func(t *testing.T) { copy := tc.file.DeepCopy() - if !reflect.DeepEqual(copy, tc.file) { - t.Fatalf("unexpected difference in backend state data:\n got %#v, want %#v", copy, tc.file) + if diff := cmp.Diff(copy, tc.file); diff != "" { + t.Fatalf("unexpected difference in backend state data:\n %s", diff) } }) } diff --git a/internal/command/workdir/statestore_config_state.go b/internal/command/workdir/statestore_config_state.go index fb7dbab1f5..366a334abf 100644 --- a/internal/command/workdir/statestore_config_state.go +++ b/internal/command/workdir/statestore_config_state.go @@ -25,7 +25,7 @@ type StateStoreConfigState struct { Type string `json:"type"` // State store type name Provider *ProviderConfigState `json:"provider"` // Details about the state-storage provider ConfigRaw json.RawMessage `json:"config"` // state_store block raw config, barring provider details - Hash uint64 `json:"hash"` // Hash of portion of configuration from config files + Hash uint64 `json:"hash"` // Hash of the state_store block's configuration, excluding the provider block and any values supplied via methods other than config } // Empty returns true if there is no active state store. @@ -125,6 +125,7 @@ func (s *StateStoreConfigState) DeepCopy() *StateStoreConfigState { provider := &ProviderConfigState{ Version: s.Provider.Version, Source: s.Provider.Source, + Hash: s.Provider.Hash, } if s.Provider.ConfigRaw != nil { provider.ConfigRaw = make([]byte, len(s.Provider.ConfigRaw)) @@ -153,6 +154,7 @@ type ProviderConfigState struct { Version *version.Version `json:"version"` // The specific provider version used for the state store. Should be set using a getproviders.Version, etc. Source *tfaddr.Provider `json:"source"` // The FQN/fully-qualified name of the provider. ConfigRaw json.RawMessage `json:"config"` // state_store block raw config, barring provider details + Hash uint64 `json:"hash"` // Hash of the nested provider block's configuration, excluding any values supplied via methods other than config } // Empty returns true if there is no provider config state data. diff --git a/internal/command/workdir/testing.go b/internal/command/workdir/testing.go index 2c6b312283..15c0d8126a 100644 --- a/internal/command/workdir/testing.go +++ b/internal/command/workdir/testing.go @@ -13,6 +13,7 @@ import ( // getTestProviderState is a test helper that returns a state representation // of a provider used for managing state via pluggable state storage. +// The Hash is always hardcoded at 12345. func getTestProviderState(t *testing.T, semVer, hostname, namespace, typeName, config string) *ProviderConfigState { t.Helper() @@ -29,5 +30,6 @@ func getTestProviderState(t *testing.T, semVer, hostname, namespace, typeName, c Type: typeName, }, ConfigRaw: []byte(config), + Hash: 12345, } } diff --git a/internal/configs/backend.go b/internal/configs/backend.go index e580f4b6c8..8e2381a3f0 100644 --- a/internal/configs/backend.go +++ b/internal/configs/backend.go @@ -29,7 +29,7 @@ func decodeBackendBlock(block *hcl.Block) (*Backend, hcl.Diagnostics) { }, nil } -// Hash produces a hash value for the reciever that covers the type and the +// Hash produces a hash value for the receiver that covers the type and the // portions of the config that conform to the given schema. // // If the config does not conform to the schema then the result is not diff --git a/internal/configs/parser_config.go b/internal/configs/parser_config.go index 8af6b7cc7d..fa72ba7f9c 100644 --- a/internal/configs/parser_config.go +++ b/internal/configs/parser_config.go @@ -394,6 +394,9 @@ var terraformBlockSchema = &hcl.BodySchema{ { Type: "required_providers", }, + // NOTE: An entry for state_store is not present here + // because we conditionally add it in the calling code + // depending on whether experiments are enabled or not. { Type: "provider_meta", LabelNames: []string{"provider"}, diff --git a/internal/configs/state_store.go b/internal/configs/state_store.go index 4fe4c0a896..a5b2fe93bc 100644 --- a/internal/configs/state_store.go +++ b/internal/configs/state_store.go @@ -5,12 +5,20 @@ package configs import ( "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" + "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // StateStore represents a "state_store" block inside a "terraform" block // in a module or file. type StateStore struct { - Type string + Type string + + // Config is the full configuration of the state_store block, including the + // nested provider block. The nested provider block config is accessible + // in isolation via (StateStore).Provider.Config Config hcl.Body Provider *Provider @@ -78,3 +86,83 @@ var StateStorageBlockSchema = &hcl.BodySchema{ }, }, } + +// Hash produces a hash value for the receiver that covers the type and the +// portions of the config that conform to the state_store schema. The provider +// block that is nested inside state_store is ignored. +// +// If the config does not conform to the schema then the result is not +// meaningful for comparison since it will be based on an incomplete result. +// +// As an exception, required attributes in the schema are treated as optional +// for the purpose of hashing, so that an incomplete configuration can still +// be hashed. Other errors, such as extraneous attributes, have no such special +// case. +func (b *StateStore) Hash(stateStoreSchema *configschema.Block, providerSchema *configschema.Block) (stateStoreHash, providerHash int, diags tfdiags.Diagnostics) { + + // 1. Prepare the state_store hash + + // The state store schema should not include a provider block or attr + if _, exists := stateStoreSchema.Attributes["provider"]; exists { + return 0, 0, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Protected argument name \"provider\" in state store schema", + Detail: "Schemas for state stores cannot contain attributes or blocks called \"provider\", to avoid confusion with the provider block nested inside the state_store block. This is a bug in the provider used for state storage, which should be reported in the provider's own issue tracker.", + Context: &b.Provider.DeclRange, + }) + } + if _, exists := stateStoreSchema.BlockTypes["provider"]; exists { + return 0, 0, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Protected block name \"provider\" in state store schema", + Detail: "Schemas for state stores cannot contain attributes or blocks called \"provider\", to avoid confusion with the provider block nested inside the state_store block. This is a bug in the provider used for state storage, which should be reported in the provider's own issue tracker.", + Context: &b.Provider.DeclRange, + }) + } + + // Don't fail if required attributes are not set. Instead, we'll just + // hash them as nulls. + schema := stateStoreSchema.NoneRequired() + spec := schema.DecoderSpec() + + // The value `b.Config` will include data about the provider block nested inside state_store + // so we need to ignore it. Decode will return errors about 'extra' attrs and blocks. We can ignore + // the diagnostic reporting the unexpected provider block, but we need to handle all other diagnostics. + // but we need to check that's the only thing being ignored. + ssVal, decodeDiags := hcldec.Decode(b.Config, spec, nil) + if decodeDiags.HasErrors() { + for _, diag := range decodeDiags { + diags = diags.Append(diag) + } + if diags.HasErrors() { + return 0, 0, diags + } + } + + // We're on the happy path, so continue to get the hash + if ssVal == cty.NilVal { + ssVal = cty.UnknownVal(schema.ImpliedType()) + } + ssToHash := cty.TupleVal([]cty.Value{ + cty.StringVal(b.Type), + ssVal, + }) + + // 2. Prepare the provider hash + schema = providerSchema.NoneRequired() + spec = schema.DecoderSpec() + pVal, decodeDiags := hcldec.Decode(b.Provider.Config, spec, nil) + if decodeDiags.HasErrors() { + diags = diags.Append(decodeDiags) + return 0, 0, diags + } + if pVal == cty.NilVal { + pVal = cty.UnknownVal(schema.ImpliedType()) + } + pToHash := cty.TupleVal([]cty.Value{ + cty.StringVal(b.Type), + pVal, + }) + + return ssToHash.Hash(), pToHash.Hash(), diags +} diff --git a/internal/configs/state_store_test.go b/internal/configs/state_store_test.go new file mode 100644 index 0000000000..75f1ef38dc --- /dev/null +++ b/internal/configs/state_store_test.go @@ -0,0 +1,280 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package configs + +import ( + "strings" + "testing" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/zclconf/go-cty/cty" +) + +// The Hash method assumes that the state_store schema doesn't include a provider block, +// and it requires calling code to remove the nested provider block from state_store config data. +func TestStateStore_Hash(t *testing.T) { + + // This test assumes a configuration like this, + // where the "fs" state store is implemented in + // the "foobar" provider: + // + // terraform { + // required_providers = { + // # entries would be here + // } + // state_store "foobar_fs" { + // # Nested provider block + // provider "foobar" { + // foobar = "foobar" + // } + + // # Attributes for configuring the state store + // path = "mystate.tfstate" + // workspace_dir = "foobar" + // } + // } + + // Normally these schemas would come from a provider's GetProviderSchema data + stateStoreSchema := &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "path": { + Type: cty.String, + Required: true, + }, + "workspace_dir": { + Type: cty.String, + Optional: true, + }, + }, + } + providerSchema := &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foobar": { + Type: cty.String, + Required: true, + }, + }, + } + + cases := map[string]struct { + config hcl.Body + providerConfig hcl.Body + schema *configschema.Block + wantErrorString string + wantProviderHash int + wantStateStoreHash int + }{ + "ignores the provider block in config data, as long as the schema doesn't include it": { + schema: stateStoreSchema, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + providerConfig: configBodyForTest(t, `foobar = "foobar"`), + wantProviderHash: 2672365208, + wantStateStoreHash: 3037430836, + }, + "tolerates empty config block for the provider even when schema has Required field(s)": { + schema: stateStoreSchema, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + # required field "foobar" is missing + } + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + providerConfig: hcl.EmptyBody(), + wantProviderHash: 2911589008, + wantStateStoreHash: 3037430836, + }, + "tolerates missing Required field(s) in state_store config": { + schema: stateStoreSchema, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + + # required field "path" is missing + workspace_dir = "foobar" + }`), + providerConfig: configBodyForTest(t, `foobar = "foobar"`), + wantProviderHash: 2672365208, + wantStateStoreHash: 3453024478, + }, + "returns errors when the config contains non-provider things that aren't in the schema": { + schema: stateStoreSchema, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + unexpected_block { + foobar = "foobar" + } + unexpected_attr = "foobar" + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + providerConfig: configBodyForTest(t, `foobar = "foobar"`), + wantErrorString: "Unsupported argument", + }, + "returns an error if the schema includes a provider block": { + schema: &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "provider": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": { + Type: cty.String, + Optional: true, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, + }, + }, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + providerConfig: configBodyForTest(t, `foobar = "foobar"`), + wantErrorString: `Protected block name "provider" in state store schema`, + }, + "returns an error if the schema includes a provider attribute": { + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "provider": { + Type: cty.String, + Optional: true, + }, + }, + }, + config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + providerConfig: configBodyForTest(t, `foobar = "foobar"`), + wantErrorString: `Protected argument name "provider" in state store schema`, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + content, _, cfgDiags := tc.config.PartialContent(terraformBlockSchema) + if len(cfgDiags) > 0 { + t.Fatalf("unexpected diagnostics: %s", cfgDiags) + } + var ssDiags hcl.Diagnostics + s, ssDiags := decodeStateStoreBlock(content.Blocks.OfType("state_store")[0]) + if len(ssDiags) > 0 { + t.Fatalf("unexpected diagnostics: %s", ssDiags) + } + + ssHash, pHash, diags := s.Hash(tc.schema, providerSchema) + if diags.HasErrors() { + if tc.wantErrorString == "" { + t.Fatalf("unexpected error: %s", diags.Err()) + } + if !strings.Contains(diags.Err().Error(), tc.wantErrorString) { + t.Fatalf("expected %q to be in the returned error string but it's missing: %q", tc.wantErrorString, diags.Err()) + } + + return // early return if testing an error case + } + + if !diags.HasErrors() && tc.wantErrorString != "" { + t.Fatal("expected an error when generating a hash, but got none") + } + + if ssHash != tc.wantStateStoreHash { + t.Fatalf("expected hash for state_store to be %d, but got %d", tc.wantStateStoreHash, ssHash) + } + if pHash != tc.wantProviderHash { + t.Fatalf("expected hash for provider to be %d, but got %d", tc.wantProviderHash, pHash) + } + }) + } +} + +func TestStateStore_checkStateStoreHashUnaffectedByProviderBlock(t *testing.T) { + + // Normally these schemas would come from a provider's GetProviderSchema data + stateStoreSchema := &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "path": { + Type: cty.String, + Required: true, + }, + "workspace_dir": { + Type: cty.String, + Optional: true, + }, + }, + } + providerSchema := &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foobar": { + Type: cty.String, + Required: true, + }, + }, + } + providerConfig := configBodyForTest(t, `foobar = "foobar"`) + + // Make two StateStores: + // 1) Has provider block in the main Config value, as well as matching data in Provider.Config + // 2) Doesn't have provider block in the main Config value, has config in Provider.Config + + s1 := StateStore{ + Config: configBodyForTest(t, `state_store "foo" { + provider "foobar" { + foobar = "foobar" + } + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + Provider: &Provider{ + Config: providerConfig, + }, + } + s2 := StateStore{ + Config: configBodyForTest(t, `state_store "foo" { + # No provider block here + + path = "mystate.tfstate" + workspace_dir = "foobar" + }`), + Provider: &Provider{ + Config: providerConfig, + }, + } + + s1StoreHash, _, _ := s1.Hash(stateStoreSchema, providerSchema) + s2StoreHash, _, _ := s2.Hash(stateStoreSchema, providerSchema) + + if s1StoreHash != s2StoreHash { + t.Fatalf("expected state_store block hashes to match, as hashing logic should ignore presence of provider block. Got s1 %d, s2 %d", s1StoreHash, s2StoreHash) + } + +} + +func configBodyForTest(t *testing.T, config string) hcl.Body { + t.Helper() + f, diags := hclsyntax.ParseConfig([]byte(config), "", hcl.Pos{Line: 1, Column: 1}) + if diags.HasErrors() { + t.Fatalf("failure creating hcl.Body during test setup") + } + return f.Body +}