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" diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 1a5d8cbc1765..1356503fbdfd 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" @@ -47,20 +49,67 @@ 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 + // dig into the code. + fmt.Println("TF_INMEM_TEST is set: Using test schema for the inmem backend") + + return &Backend{ + Base: backendbase.Base{ + Schema: testSchema(), + }, + } + } + + // 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 testSchema() *configschema.Block { + var newSchemaAttrs = make(map[string]*configschema.Attribute) + maps.Copy(newSchemaAttrs, defaultSchemaAttrs) + + // 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, + Attributes: map[string]*configschema.Attribute{ + "child": { + Type: cty.String, + Optional: true, + Description: "A nested attribute inside the parent attribute `test_nested_attr_single`", }, }, }, } + + return &configschema.Block{ + Attributes: newSchemaAttrs, + } } type Backend struct { diff --git a/internal/command/init.go b/internal/command/init.go index 4ba9448edfdc..6881286d7ba7 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" @@ -1031,10 +1033,29 @@ 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 { + + // Check the value looks like a valid attribute identifier + splitName := strings.Split(name, ".") + 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 contains an invalid identifier that cannot be used for partial configuration: %q.", name, part), + )) + continue + } + } + // 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) + } + targetAttr := schema.AttributeByPath(path) + if targetAttr == nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, "Invalid backend configuration argument", @@ -1042,12 +1063,40 @@ func (c *InitCommand) backendConfigOverrideBody(flags arguments.FlagNameValueSli )) continue } - value, valueDiags := configValueFromCLI(item.String(), rawValue, attrS.Type) - diags = diags.Append(valueDiags) - if valueDiags.HasErrors() { + 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 } - synthVals[name] = value + + 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 + // Nested values need to be added in a way that doesn't affect pre-existing values + 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 + synthVals[name] = value + } } } @@ -1056,6 +1105,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 290e3f274620..000f37a79a92 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -792,6 +792,152 @@ func TestInit_backendConfigKV(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-complex"), td) // test_nested_attr_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_nested_attr_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nested_attr_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_nested_attr_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-kv-complex-empty"), td) // backend block for inmem is empty + 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_nested_attr_single = { + // child = "..." + // } + args := []string{ + "-backend-config=test_nested_attr_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_nested_attr_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-complex"), 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-complex"), 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_nested_attr_single = { + // child = "..." + // } + args := []string{ + "-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()) + } + gotStderr := done(t).Stderr() + wantStderr := ` +Error: Invalid backend configuration argument + +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) + } + }) +} + func TestInit_backendConfigKVReInit(t *testing.T) { // Create a temporary working directory that is empty td := t.TempDir() @@ -3243,3 +3389,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()) + } + }) +} 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-kv-complex/main.tf b/internal/command/testdata/init-backend-config-kv-complex/main.tf new file mode 100644 index 000000000000..8e4d31a0591e --- /dev/null +++ b/internal/command/testdata/init-backend-config-kv-complex/main.tf @@ -0,0 +1,7 @@ +terraform { + backend "inmem" { + test_nested_attr_single = { + child = "" // to be overwritten in test + } + } +}