From ff54257a067ceef67c31f6045f2a8d0d9781dbdc Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 23 Apr 2025 15:21:23 +0100 Subject: [PATCH 01/12] Add initial changes to allow overriding a nested attribute --- internal/command/init.go | 77 +++++++++++++++++++++++++++++++++++----- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/internal/command/init.go b/internal/command/init.go index 4ba9448edfdc..0701e9007bf5 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -1031,10 +1031,77 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli flushVals() // deal with any accumulated individual values first mergeBody(newBody) } else { + // The flag value is setting a single attribute's value name := item.Value[:eq] rawValue := item.Value[eq+1:] - attrS := schema.Attributes[name] - if attrS == nil { + + splitName := strings.Split(name, ".") + isNested := len(splitName) > 1 + + var value cty.Value + var valueDiags tfdiags.Diagnostics + switch { + case !isNested: + // The flag item is overriding a top-level attribute + attrS := schema.Attributes[name] + if attrS == nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid backend configuration argument", + fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), + )) + continue + } + + value, valueDiags = configValueFromCLI(item.String(), rawValue, attrS.Type) + diags = diags.Append(valueDiags) + if valueDiags.HasErrors() { + continue + } + + // Synthetic values are collected as we parse each flag item + synthVals[name] = value + case isNested: + // The flag item is overriding a nested attribute + // e.g. assume_role.role_arn in the s3 backend + // We assume a max nesting-depth of 1 + + parentName := splitName[0] + nestedName := splitName[1] + parentAttr := schema.Attributes[parentName] + nestedAttr := parentAttr.NestedType.Attributes[nestedName] + if nestedAttr == nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid backend configuration argument", + fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), + )) + continue + } + + value, valueDiags = configValueFromCLI(item.String(), rawValue, nestedAttr.Type) + diags = diags.Append(valueDiags) + if valueDiags.HasErrors() { + continue + } + + // Collected synthetic values need to accounting for attribute nesting + synthParent, found := synthVals[parentName] + if !found { + synthVals[parentName] = cty.ObjectVal(map[string]cty.Value{ + nestedName: value, + }) + } + if found { + // append the new attribute override to any existing attributes + // also nested under the parent + nestedMap := synthParent.AsValueMap() + nestedMap[nestedName] = value + synthVals[parentName] = cty.ObjectVal(nestedMap) + } + + default: + // Should not reach here diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Invalid backend configuration argument", @@ -1042,12 +1109,6 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli )) continue } - value, valueDiags := configValueFromCLI(item.String(), rawValue, attrS.Type) - diags = diags.Append(valueDiags) - if valueDiags.HasErrors() { - continue - } - synthVals[name] = value } } From 7e53ef6e83aa485357ddd902462162904b50cf1b Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 23 Apr 2025 16:19:20 +0100 Subject: [PATCH 02/12] Update the inmem backend to expose a more complex schema only when `TF_INMEM_TEST` is set --- .../backend/remote-state/inmem/backend.go | 52 ++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 1a5d8cbc1765..e07313c3a208 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -6,6 +6,8 @@ package inmem import ( "errors" "fmt" + "maps" + "os" "sort" "sync" "time" @@ -48,19 +50,57 @@ func Reset() { // New creates a new backend for Inmem remote state. func New() backend.Backend { + if os.Getenv("TF_INMEM_TEST") != "" { + // We use a different schema for testing. This isn't user facing unless they + // dig into the code. + fmt.Sprintln("TF_INMEM_TEST is set: Using test schema for the inmem backend") + + return &Backend{ + Base: backendbase.Base{ + Schema: &configschema.Block{ + Attributes: testSchemaAttrs(), + }, + }, + } + } + + // Default schema that's user-facing return &Backend{ Base: backendbase.Base{ Schema: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "lock_id": { - Type: cty.String, - Optional: true, - Description: "initializes the state in a locked configuration", - }, + Attributes: defaultSchemaAttrs, + }, + }, + } +} + +var defaultSchemaAttrs = map[string]*configschema.Attribute{ + "lock_id": { + Type: cty.String, + Optional: true, + Description: "initializes the state in a locked configuration", + }, +} + +func testSchemaAttrs() map[string]*configschema.Attribute { + var newSchema = make(map[string]*configschema.Attribute) + maps.Copy(newSchema, defaultSchemaAttrs) + + // Append test-specific parts of schema + newSchema["test_nesting_single"] = &configschema.Attribute{ + Description: "An attribute that contains nested attributes, where nesting mode is NestingSingle", + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "child": { + Type: cty.String, + Optional: true, + Description: "A nested attribute inside the parent attribute `test_nesting_single`", }, }, }, } + return newSchema } type Backend struct { From 7bbdbc4e225c96c600911ba07efb515e6d9e74e7 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 23 Apr 2025 16:21:10 +0100 Subject: [PATCH 03/12] Add a test asserting that nested fields can be overridden by `-backend-config` --- internal/command/init_test.go | 35 +++++++++++++++++++ .../init-backend-config-kv-nested/main.tf | 7 ++++ 2 files changed, 42 insertions(+) create mode 100644 internal/command/testdata/init-backend-config-kv-nested/main.tf diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 290e3f274620..066388429041 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -792,6 +792,41 @@ func TestInit_backendConfigKV(t *testing.T) { } } +func TestInit_backendConfigKVNested(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + defer testChdir(t, td)() + t.Setenv("TF_INMEM_TEST", "1") // Allows use of inmem backend with a more complex schema + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + // overridden field is nested: + // test_nesting_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nesting_single.child=foobar", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", done(t).Stderr()) + } + + // Read our saved backend config and verify we have our settings + state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { + t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) + } +} + func TestInit_backendConfigKVReInit(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() diff --git a/internal/command/testdata/init-backend-config-kv-nested/main.tf b/internal/command/testdata/init-backend-config-kv-nested/main.tf new file mode 100644 index 000000000000..20a134b19466 --- /dev/null +++ b/internal/command/testdata/init-backend-config-kv-nested/main.tf @@ -0,0 +1,7 @@ +terraform { + backend "inmem" { + test_nesting_single = { + child = "" // to be overwritten in test + } + } +} From ff081f22aa030d200131828a4cde7b2cf8a9e0fc Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 23 Apr 2025 16:42:08 +0100 Subject: [PATCH 04/12] Add changelog entry --- .changes/v1.13/BUG FIXES-20250423-164150.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/v1.13/BUG FIXES-20250423-164150.yaml diff --git a/.changes/v1.13/BUG FIXES-20250423-164150.yaml b/.changes/v1.13/BUG FIXES-20250423-164150.yaml new file mode 100644 index 000000000000..35c36963b859 --- /dev/null +++ b/.changes/v1.13/BUG FIXES-20250423-164150.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'backends: Nested attrbiutes can now be overridden during `init` using the `-backend-config` flag' +time: 2025-04-23T16:41:50.904809+01:00 +custom: + Issue: "36911" From 14cc7d37dd108a9af9743a4b64f29c23bcaccd58 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 23 Apr 2025 16:52:05 +0100 Subject: [PATCH 05/12] Update code comments --- internal/command/init.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/command/init.go b/internal/command/init.go index 0701e9007bf5..f9a6366d9f22 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -1064,7 +1064,8 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli case isNested: // The flag item is overriding a nested attribute // e.g. assume_role.role_arn in the s3 backend - // We assume a max nesting-depth of 1 + // We assume a max nesting-depth of 1 as s3 is the only + // backend that contains nested fields. parentName := splitName[0] nestedName := splitName[1] @@ -1085,7 +1086,9 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli continue } - // Collected synthetic values need to accounting for attribute nesting + // Synthetic values are collected as we parse each flag item + // When doing this we need to account for attribute nesting + // and multiple nested fields being overridden. synthParent, found := synthVals[parentName] if !found { synthVals[parentName] = cty.ObjectVal(map[string]cty.Value{ @@ -1093,7 +1096,7 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli }) } if found { - // append the new attribute override to any existing attributes + // add the new attribute override to any existing attributes // also nested under the parent nestedMap := synthParent.AsValueMap() nestedMap[nestedName] = value From f354a5c09f0cef7c103d5911d9bd6ec35770422c Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 14:03:39 +0100 Subject: [PATCH 06/12] Update code to accommodate any level of nesting. Update how attribute paths are validated. --- internal/command/init.go | 119 ++++++++++++++++++---------------- internal/command/init_test.go | 118 +++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 56 deletions(-) diff --git a/internal/command/init.go b/internal/command/init.go index f9a6366d9f22..54a4809c88be 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -8,11 +8,13 @@ import ( "errors" "fmt" "log" + "maps" "reflect" "sort" "strings" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" svchost "github.com/hashicorp/terraform-svchost" "github.com/posener/complete" "github.com/zclconf/go-cty/cty" @@ -1035,82 +1037,57 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli name := item.Value[:eq] rawValue := item.Value[eq+1:] + // Check the value looks like a valid attribute identifier splitName := strings.Split(name, ".") - isNested := len(splitName) > 1 - - var value cty.Value - var valueDiags tfdiags.Diagnostics - switch { - case !isNested: - // The flag item is overriding a top-level attribute - attrS := schema.Attributes[name] - if attrS == nil { + for _, part := range splitName { + if !hclsyntax.ValidIdentifier(part) { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Invalid backend configuration argument", - fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), + fmt.Sprintf("The backend configuration argument %q given on the command line contains an invalid identifier that cannot be used for partial configuration: %q.", name, part), )) continue } + } + // Check the attribute exists in the backend's schema + path := cty.Path{} + for _, name := range splitName { + path = path.GetAttr(name) + } + targetAttr := schema.AttributeByPath(path) + if targetAttr == nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid backend configuration argument", + fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), + )) + continue + } - value, valueDiags = configValueFromCLI(item.String(), rawValue, attrS.Type) + if len(splitName) > 1 { + // The flag item is overriding a nested attribute (name contains multiple identifiers) + // e.g. assume_role.role_arn in the s3 backend + value, valueDiags := configValueFromCLI(item.String(), rawValue, targetAttr.Type) diags = diags.Append(valueDiags) if valueDiags.HasErrors() { continue } // Synthetic values are collected as we parse each flag item - synthVals[name] = value - case isNested: - // The flag item is overriding a nested attribute - // e.g. assume_role.role_arn in the s3 backend - // We assume a max nesting-depth of 1 as s3 is the only - // backend that contains nested fields. - - parentName := splitName[0] - nestedName := splitName[1] - parentAttr := schema.Attributes[parentName] - nestedAttr := parentAttr.NestedType.Attributes[nestedName] - if nestedAttr == nil { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid backend configuration argument", - fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), - )) - continue - } - - value, valueDiags = configValueFromCLI(item.String(), rawValue, nestedAttr.Type) + // Nested values need to be added in a way that doesn't affect pre-existing values + var synthCopy map[string]cty.Value + maps.Copy(synthCopy, synthVals) + synthVals = addNestedAttrsToCtyValueMap(synthCopy, splitName, value) + } else { + // The flag item is overriding a non-nested, top-level attribute + value, valueDiags := configValueFromCLI(item.String(), rawValue, targetAttr.Type) diags = diags.Append(valueDiags) if valueDiags.HasErrors() { continue } // Synthetic values are collected as we parse each flag item - // When doing this we need to account for attribute nesting - // and multiple nested fields being overridden. - synthParent, found := synthVals[parentName] - if !found { - synthVals[parentName] = cty.ObjectVal(map[string]cty.Value{ - nestedName: value, - }) - } - if found { - // add the new attribute override to any existing attributes - // also nested under the parent - nestedMap := synthParent.AsValueMap() - nestedMap[nestedName] = value - synthVals[parentName] = cty.ObjectVal(nestedMap) - } - - default: - // Should not reach here - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid backend configuration argument", - fmt.Sprintf("The backend configuration argument %q given on the command line is not expected for the selected backend type.", name), - )) - continue + synthVals[name] = value } } } @@ -1120,6 +1097,36 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli return ret, diags } +// addNestedAttrsToCtyValueMap is used to assemble a map of cty Values that is used to create a cty.Object. +// This function should be used to set nested values in the map[string]cty.Value, and cannot be used to set +// top-level attributes (this will result in other top-level attributes being lost). +func addNestedAttrsToCtyValueMap(accumulator map[string]cty.Value, names []string, attrValue cty.Value) map[string]cty.Value { + if len(names) == 1 { + // Base case - we are setting the attribute with the provided value + return map[string]cty.Value{ + names[0]: attrValue, + } + } + + // Below we are navigating the path from the final, nested attribute we want to set a value for. + // During this process we need to ensure that any pre-existing values in the map are not + // accidentally removed + val, ok := accumulator[names[0]] + if !ok { + accumulator[names[0]] = cty.ObjectVal(addNestedAttrsToCtyValueMap(accumulator, names[1:], attrValue)) + } else { + values := val.AsValueMap() + newValues := addNestedAttrsToCtyValueMap(accumulator, names[1:], attrValue) + + // We copy new values into the map of pre-existing values + maps.Copy(values, newValues) + + accumulator[names[0]] = cty.ObjectVal(values) + } + + return accumulator +} + func (c *InitCommand) AutocompleteArgs() complete.Predictor { return complete.PredictDirs("") } diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 066388429041..377e1b913a12 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -3278,3 +3278,121 @@ func expectedPackageInstallPath(name, version string, exe bool) string { baseDir, fmt.Sprintf("registry.terraform.io/hashicorp/%s/%s/%s", name, version, platform), )) } + +func Test_addNestedAttrsToCtyValueMap(t *testing.T) { + + t.Run("adds nested attributes to depth of 2 into an empty map", func(t *testing.T) { + obj := map[string]cty.Value{} + name := "a.b" + splitName := strings.Split(name, ".") + attrVal := cty.StringVal("foobar") + ret := addNestedAttrsToCtyValueMap(obj, splitName, attrVal) + + a, ok := ret["a"] + if !ok { + t.Fatalf("expected a top-level attribute \"a\" but it's missing") + } + + b, ok := a.AsValueMap()["b"] + if !ok { + t.Fatalf("expected an attribute \"a.b\" but it's missing") + } + if b.Equals(attrVal) == cty.False { + t.Fatalf("expected attribute \"a.b\" to equal %q but got %q", attrVal.AsString(), b.AsString()) + } + }) + + t.Run("adds nested attributes to depth of 5 into an empty map", func(t *testing.T) { + obj := map[string]cty.Value{} + name := "a.b.c.d.e" + splitName := strings.Split(name, ".") + attrVal := cty.StringVal("foobar") + ret := addNestedAttrsToCtyValueMap(obj, splitName, attrVal) + + a, ok := ret["a"] + if !ok { + t.Fatalf("expected a top-level attribute \"a\" but it's missing") + } + b, ok := a.AsValueMap()["b"] + if !ok { + t.Fatal() + } + c, ok := b.AsValueMap()["c"] + if !ok { + t.Fatal() + } + d, ok := c.AsValueMap()["d"] + if !ok { + t.Fatal() + } + e, ok := d.AsValueMap()["e"] + if !ok { + t.Fatal() + } + if e.Equals(attrVal) == cty.False { + t.Fatalf("expected attribute %q to equal %q but got %q", name, attrVal.AsString(), e.AsString()) + } + }) + + t.Run("adds nested attributes to a populated map without removing existing values in a separate path", func(t *testing.T) { + obj := map[string]cty.Value{ + "pre-existing-val": cty.StringVal("I should not be deleted!"), + } + name := "a.b" + // Both of the above have separate paths; do not share the first step of paths + + splitName := strings.Split(name, ".") + attrVal := cty.StringVal("foobar") + ret := addNestedAttrsToCtyValueMap(obj, splitName, attrVal) + + _, ok := ret["pre-existing-val"] + if !ok { + t.Fatalf("expected a top-level attribute \"pre-existing-val\" but it's missing") + } + + a, ok := ret["a"] + if !ok { + t.Fatalf("expected a top-level attribute \"a\" but it's missing") + } + + b, ok := a.AsValueMap()["b"] + if !ok { + t.Fatal() + } + if b.Equals(attrVal) == cty.False { + t.Fatal() + } + }) + + t.Run("adds nested attributes to a populated map without removing existing values in the same path", func(t *testing.T) { + obj := map[string]cty.Value{ + "a": cty.ObjectVal(map[string]cty.Value{ + "pre-existing-val": cty.StringVal("I should not be deleted!"), + }), + } + name := "a.b" + // Both of the above include 'a' as first step in path + + splitName := strings.Split(name, ".") + attrVal := cty.StringVal("baz") + ret := addNestedAttrsToCtyValueMap(obj, splitName, attrVal) + + a, ok := ret["a"] + if !ok { + t.Fatalf("expected a top-level attribute \"a\" but it's missing") + } + + _, ok = a.AsValueMap()["pre-existing-val"] + if !ok { + t.Fatalf("expected an attribute \"a.pre-existing-val\" but it's missing") + } + + b, ok := a.AsValueMap()["b"] + if !ok { + t.Fatalf("expected an attribute \"a.b\" but it's missing") + } + if b.Equals(attrVal) == cty.False { + t.Fatalf("expected attribute %q to equal %q but got %q", name, attrVal.AsString(), b.AsString()) + } + }) +} From 6dc264908ecf569f8d5494a338512586477fc9ad Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 14:05:54 +0100 Subject: [PATCH 07/12] Add validation to check that user is specifying an attribute that takes primitive type values/doesn't contain nested attributes itself --- internal/command/init.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/command/init.go b/internal/command/init.go index 54a4809c88be..6881286d7ba7 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -1049,7 +1049,7 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli continue } } - // Check the attribute exists in the backend's schema + // Check the attribute exists in the backend's schema and is a 'leaf' attribute path := cty.Path{} for _, name := range splitName { path = path.GetAttr(name) @@ -1063,6 +1063,14 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli )) continue } + if targetAttr.NestedType != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid backend configuration argument", + fmt.Sprintf("The backend configuration argument %q given on the command line specifies an attribute that contains nested attributes. Instead, use separate flags for each nested attribute inside.", name), + )) + continue + } if len(splitName) > 1 { // The flag item is overriding a nested attribute (name contains multiple identifiers) @@ -1075,7 +1083,7 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli // Synthetic values are collected as we parse each flag item // Nested values need to be added in a way that doesn't affect pre-existing values - var synthCopy map[string]cty.Value + synthCopy := map[string]cty.Value{} maps.Copy(synthCopy, synthVals) synthVals = addNestedAttrsToCtyValueMap(synthCopy, splitName, value) } else { From 0a7a8b646be9c9555cc4024f32436a82328b5234 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 14:06:55 +0100 Subject: [PATCH 08/12] Fix println --- internal/backend/remote-state/inmem/backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index e07313c3a208..4e91b0712f0c 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -53,7 +53,7 @@ func New() backend.Backend { if os.Getenv("TF_INMEM_TEST") != "" { // We use a different schema for testing. This isn't user facing unless they // dig into the code. - fmt.Sprintln("TF_INMEM_TEST is set: Using test schema for the inmem backend") + fmt.Println("TF_INMEM_TEST is set: Using test schema for the inmem backend") return &Backend{ Base: backendbase.Base{ From 7a93fdc0276ba9ac5f7169afb6ad91bec03765a4 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 14:11:51 +0100 Subject: [PATCH 09/12] Add code comment about new test-specific behavior of the inmem backend --- internal/backend/remote-state/inmem/backend.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 4e91b0712f0c..63f1e4943e90 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -49,6 +49,14 @@ func Reset() { } // New creates a new backend for Inmem remote state. +// +// Currently the inmem backend is available for end users to use if they know it exists (it is not in any docs), but it was intended as a test resource. +// Making the inmem backend unavailable to end users and only available during tests is a breaking change. +// As a compromise for now, the inmem backend includes test-specific code that is enabled by setting the TF_INMEM_TEST environment variable. +// Test-specific behaviors include: +// * A more complex schema for testing code related to backend configurations +// +// Note: The alternative choice would be to add a duplicate of inmem in the codebase that's user-inaccessible, and this would be harder to maintain. func New() backend.Backend { if os.Getenv("TF_INMEM_TEST") != "" { // We use a different schema for testing. This isn't user facing unless they From 4fe67f85168416b239c927fb40863481f4ea3a3a Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 15:14:48 +0100 Subject: [PATCH 10/12] Add test cases for when overriding a non-existent attribute, and overriding a non-leaf attribute --- internal/command/init_test.go | 133 +++++++++++++++++++++++++++------- 1 file changed, 105 insertions(+), 28 deletions(-) diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 377e1b913a12..de4cad24ae12 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -793,38 +793,115 @@ func TestInit_backendConfigKV(t *testing.T) { } func TestInit_backendConfigKVNested(t *testing.T) { - // Create a temporary working directory that is empty - td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) - defer testChdir(t, td)() t.Setenv("TF_INMEM_TEST", "1") // Allows use of inmem backend with a more complex schema - ui := new(cli.MockUi) - view, done := testView(t) - c := &InitCommand{ - Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), - Ui: ui, - View: view, - }, - } + t.Run("the -backend-config flag can overwrite a nested attribute", func(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + defer testChdir(t, td)() - // overridden field is nested: - // test_nesting_single = { - // child = "..." - // } - args := []string{ - "-backend-config=test_nesting_single.child=foobar", - } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: \n%s", done(t).Stderr()) - } + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } - // Read our saved backend config and verify we have our settings - state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) - if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { - t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) - } + // overridden field is nested: + // test_nesting_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nesting_single.child=foobar", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", done(t).Stderr()) + } + + // Read our saved backend config and verify we have our settings + state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { + t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) + } + }) + + t.Run("an error is returned when when the parent attribute doesn't exist", func(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + // overridden field doesn't exist + args := []string{ + "-backend-config=does_not_exist.child=foobar", + } + if code := c.Run(args); code != 1 { + t.Fatalf("expected code 1, got: %d \n%s", code, done(t).Stderr()) + } + gotStderr := done(t).Stderr() + wantStderr := ` +Error: Invalid backend configuration argument + +The backend configuration argument "does_not_exist.child" given on the +command line is not expected for the selected backend type. +` + if diff := cmp.Diff(wantStderr, gotStderr); diff != "" { + t.Errorf("wrong error output\n%s", diff) + } + }) + + t.Run("an error is returned when trying to set an attribute that contains nested attributes", func(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + // overridden field is a parent field: + // test_nesting_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nesting_single=foobar", + } + if code := c.Run(args); code != 1 { + t.Fatalf("expected code 1, got: %d \n%s", code, done(t).Stderr()) + } + gotStderr := done(t).Stderr() + wantStderr := ` +Error: Invalid backend configuration argument + +The backend configuration argument "test_nesting_single" given on the command +line specifies an attribute that contains nested attributes. Instead, use +separate flags for each nested attribute inside. +` + if diff := cmp.Diff(wantStderr, gotStderr); diff != "" { + t.Errorf("wrong error output\n%s", diff) + } + }) } func TestInit_backendConfigKVReInit(t *testing.T) { From a4d21a9a3f495a9f4967dc3796a712468869f776 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 25 Apr 2025 18:17:52 +0100 Subject: [PATCH 11/12] Add a test case showing that the field can be missing from the configuration being overridden --- internal/command/init_test.go | 38 ++++++++++++++++++- .../init-backend-config-empty/main.tf | 7 ++++ .../init-backend-config-kv-nested/main.tf | 4 +- 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 internal/command/testdata/init-backend-config-empty/main.tf diff --git a/internal/command/init_test.go b/internal/command/init_test.go index de4cad24ae12..708c981ee01a 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -795,10 +795,44 @@ func TestInit_backendConfigKV(t *testing.T) { func TestInit_backendConfigKVNested(t *testing.T) { t.Setenv("TF_INMEM_TEST", "1") // Allows use of inmem backend with a more complex schema - t.Run("the -backend-config flag can overwrite a nested attribute", func(t *testing.T) { + t.Run("the -backend-config flag can overwrite a nested attribute present in config", func(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) // test_nesting_single.child is set in this config + defer testChdir(t, td)() + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + // overridden field is nested: + // test_nesting_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nesting_single.child=foobar", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", done(t).Stderr()) + } + + // Read our saved backend config and verify we have our settings + state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { + t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) + } + }) + + t.Run("the -backend-config flag can overwrite a nested attribute that's not in the config", func(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-backend-config-empty"), td) // backend block for inmem is empty defer testChdir(t, td)() ui := new(cli.MockUi) diff --git a/internal/command/testdata/init-backend-config-empty/main.tf b/internal/command/testdata/init-backend-config-empty/main.tf new file mode 100644 index 000000000000..20a134b19466 --- /dev/null +++ b/internal/command/testdata/init-backend-config-empty/main.tf @@ -0,0 +1,7 @@ +terraform { + backend "inmem" { + test_nesting_single = { + child = "" // to be overwritten in test + } + } +} diff --git a/internal/command/testdata/init-backend-config-kv-nested/main.tf b/internal/command/testdata/init-backend-config-kv-nested/main.tf index 20a134b19466..b103ceed4c36 100644 --- a/internal/command/testdata/init-backend-config-kv-nested/main.tf +++ b/internal/command/testdata/init-backend-config-kv-nested/main.tf @@ -1,7 +1,5 @@ terraform { backend "inmem" { - test_nesting_single = { - child = "" // to be overwritten in test - } + // empty - for testing overwriting of empty config via CLI } } From 299c1dabcd57f49a01c2055871575790147576d2 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Mon, 28 Apr 2025 10:58:46 +0100 Subject: [PATCH 12/12] Tweak test fixtures, update some naming, refactor test-specific inmem schema creation These diffs are due to some work I started investigating supporting using the -backend-config flag to override blocks . I decided to back out of that, but these remaining changes are still for the better. --- .../backend/remote-state/inmem/backend.go | 21 ++++++------ internal/command/init_test.go | 32 +++++++++---------- .../main.tf | 5 +++ .../main.tf | 2 +- .../init-backend-config-kv-nested/main.tf | 5 --- 5 files changed, 33 insertions(+), 32 deletions(-) create mode 100644 internal/command/testdata/init-backend-config-kv-complex-empty/main.tf rename internal/command/testdata/{init-backend-config-empty => init-backend-config-kv-complex}/main.tf (73%) delete mode 100644 internal/command/testdata/init-backend-config-kv-nested/main.tf diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 63f1e4943e90..1356503fbdfd 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -65,9 +65,7 @@ func New() backend.Backend { return &Backend{ Base: backendbase.Base{ - Schema: &configschema.Block{ - Attributes: testSchemaAttrs(), - }, + Schema: testSchema(), }, } } @@ -90,12 +88,12 @@ var defaultSchemaAttrs = map[string]*configschema.Attribute{ }, } -func testSchemaAttrs() map[string]*configschema.Attribute { - var newSchema = make(map[string]*configschema.Attribute) - maps.Copy(newSchema, defaultSchemaAttrs) +func testSchema() *configschema.Block { + var newSchemaAttrs = make(map[string]*configschema.Attribute) + maps.Copy(newSchemaAttrs, defaultSchemaAttrs) - // Append test-specific parts of schema - newSchema["test_nesting_single"] = &configschema.Attribute{ + // Append test-specific attributes to the default attributes + newSchemaAttrs["test_nested_attr_single"] = &configschema.Attribute{ Description: "An attribute that contains nested attributes, where nesting mode is NestingSingle", NestedType: &configschema.Object{ Nesting: configschema.NestingSingle, @@ -103,12 +101,15 @@ func testSchemaAttrs() map[string]*configschema.Attribute { "child": { Type: cty.String, Optional: true, - Description: "A nested attribute inside the parent attribute `test_nesting_single`", + Description: "A nested attribute inside the parent attribute `test_nested_attr_single`", }, }, }, } - return newSchema + + return &configschema.Block{ + Attributes: newSchemaAttrs, + } } type Backend struct { diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 708c981ee01a..000f37a79a92 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -792,13 +792,13 @@ func TestInit_backendConfigKV(t *testing.T) { } } -func TestInit_backendConfigKVNested(t *testing.T) { +func TestInit_backendConfigKV_nestedAttributes(t *testing.T) { t.Setenv("TF_INMEM_TEST", "1") // Allows use of inmem backend with a more complex schema t.Run("the -backend-config flag can overwrite a nested attribute present in config", func(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) // test_nesting_single.child is set in this config + testCopyDir(t, testFixturePath("init-backend-config-kv-complex"), td) // test_nested_attr_single.child is set in this config defer testChdir(t, td)() ui := new(cli.MockUi) @@ -812,11 +812,11 @@ func TestInit_backendConfigKVNested(t *testing.T) { } // overridden field is nested: - // test_nesting_single = { + // test_nested_attr_single = { // child = "..." // } args := []string{ - "-backend-config=test_nesting_single.child=foobar", + "-backend-config=test_nested_attr_single.child=foobar", } if code := c.Run(args); code != 0 { t.Fatalf("bad: \n%s", done(t).Stderr()) @@ -824,7 +824,7 @@ func TestInit_backendConfigKVNested(t *testing.T) { // Read our saved backend config and verify we have our settings state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) - if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nested_attr_single":{"child":"foobar"}}`; got != want { t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) } }) @@ -832,7 +832,7 @@ func TestInit_backendConfigKVNested(t *testing.T) { t.Run("the -backend-config flag can overwrite a nested attribute that's not in the config", func(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-empty"), td) // backend block for inmem is empty + testCopyDir(t, testFixturePath("init-backend-config-kv-complex-empty"), td) // backend block for inmem is empty defer testChdir(t, td)() ui := new(cli.MockUi) @@ -846,11 +846,11 @@ func TestInit_backendConfigKVNested(t *testing.T) { } // overridden field is nested: - // test_nesting_single = { + // test_nested_attr_single = { // child = "..." // } args := []string{ - "-backend-config=test_nesting_single.child=foobar", + "-backend-config=test_nested_attr_single.child=foobar", } if code := c.Run(args); code != 0 { t.Fatalf("bad: \n%s", done(t).Stderr()) @@ -858,7 +858,7 @@ func TestInit_backendConfigKVNested(t *testing.T) { // Read our saved backend config and verify we have our settings state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) - if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nesting_single":{"child":"foobar"}}`; got != want { + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"lock_id":null,"test_nested_attr_single":{"child":"foobar"}}`; got != want { t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) } }) @@ -866,7 +866,7 @@ func TestInit_backendConfigKVNested(t *testing.T) { t.Run("an error is returned when when the parent attribute doesn't exist", func(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + testCopyDir(t, testFixturePath("init-backend-config-kv-complex"), td) defer testChdir(t, td)() ui := new(cli.MockUi) @@ -901,7 +901,7 @@ command line is not expected for the selected backend type. t.Run("an error is returned when trying to set an attribute that contains nested attributes", func(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() - testCopyDir(t, testFixturePath("init-backend-config-kv-nested"), td) + testCopyDir(t, testFixturePath("init-backend-config-kv-complex"), td) defer testChdir(t, td)() ui := new(cli.MockUi) @@ -915,11 +915,11 @@ command line is not expected for the selected backend type. } // overridden field is a parent field: - // test_nesting_single = { + // test_nested_attr_single = { // child = "..." // } args := []string{ - "-backend-config=test_nesting_single=foobar", + "-backend-config=test_nested_attr_single=foobar", } if code := c.Run(args); code != 1 { t.Fatalf("expected code 1, got: %d \n%s", code, done(t).Stderr()) @@ -928,9 +928,9 @@ command line is not expected for the selected backend type. wantStderr := ` Error: Invalid backend configuration argument -The backend configuration argument "test_nesting_single" given on the command -line specifies an attribute that contains nested attributes. Instead, use -separate flags for each nested attribute inside. +The backend configuration argument "test_nested_attr_single" given on the +command line specifies an attribute that contains nested attributes. Instead, +use separate flags for each nested attribute inside. ` if diff := cmp.Diff(wantStderr, gotStderr); diff != "" { t.Errorf("wrong error output\n%s", diff) diff --git a/internal/command/testdata/init-backend-config-kv-complex-empty/main.tf b/internal/command/testdata/init-backend-config-kv-complex-empty/main.tf new file mode 100644 index 000000000000..51ba0dbf2d9a --- /dev/null +++ b/internal/command/testdata/init-backend-config-kv-complex-empty/main.tf @@ -0,0 +1,5 @@ +terraform { + backend "inmem" { + # empty - for testing overwriting of empty config via CLI + } +} diff --git a/internal/command/testdata/init-backend-config-empty/main.tf b/internal/command/testdata/init-backend-config-kv-complex/main.tf similarity index 73% rename from internal/command/testdata/init-backend-config-empty/main.tf rename to internal/command/testdata/init-backend-config-kv-complex/main.tf index 20a134b19466..8e4d31a0591e 100644 --- a/internal/command/testdata/init-backend-config-empty/main.tf +++ b/internal/command/testdata/init-backend-config-kv-complex/main.tf @@ -1,6 +1,6 @@ terraform { backend "inmem" { - test_nesting_single = { + test_nested_attr_single = { child = "" // to be overwritten in test } } diff --git a/internal/command/testdata/init-backend-config-kv-nested/main.tf b/internal/command/testdata/init-backend-config-kv-nested/main.tf deleted file mode 100644 index b103ceed4c36..000000000000 --- a/internal/command/testdata/init-backend-config-kv-nested/main.tf +++ /dev/null @@ -1,5 +0,0 @@ -terraform { - backend "inmem" { - // empty - for testing overwriting of empty config via CLI - } -}