-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Fix bug where -backend-config
could not override attributes that weren't at the top level in the backend schema
#36919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ff54257
7e53ef6
7bbdbc4
ff081f2
14cc7d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is using |
||
|
||
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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1031,23 +1031,87 @@ 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 as s3 is the only | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we somehow bake this assumption in the code validation? An input like |
||
// backend that contains nested fields. | ||
|
||
parentName := splitName[0] | ||
nestedName := splitName[1] | ||
parentAttr := schema.Attributes[parentName] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a path to panicking here, if the schema does not have e.g |
||
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 | ||
} | ||
|
||
// 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 | ||
} | ||
value, valueDiags := configValueFromCLI(item.String(), rawValue, attrS.Type) | ||
diags = diags.Append(valueDiags) | ||
if valueDiags.HasErrors() { | ||
continue | ||
} | ||
synthVals[name] = value | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
terraform { | ||
backend "inmem" { | ||
test_nesting_single = { | ||
child = "" // to be overwritten in test | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A choice that I needed to consider here was either to:
inmem
backend have test-specific schemas.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 theinmem
backend unavailable to end users and only available during tests is a breaking change. We could do that in future, but for now I think making parts of the inmem backend test-only makes sense, instead of creating a new backend that uses 99% of the same code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be great to have this comment somewhere in this code for our future selves.