Skip to content

Commit

Permalink
Merge branch 'main' into b/s3-object-lock-file
Browse files Browse the repository at this point in the history
  • Loading branch information
bschaatsbergen authored Dec 2, 2024
2 parents 9db9647 + b2930f3 commit 86ca532
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 67 deletions.
47 changes: 16 additions & 31 deletions internal/backend/local/backend_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,33 +259,26 @@ func (b *Local) opApply(
// same parsing logic from the plan to generate the diagnostics.
undeclaredVariables := map[string]backendrun.UnparsedVariableValue{}

for varName, rawV := range op.Variables {
parsedVars, _ := backendrun.ParseVariableValues(op.Variables, lr.Config.Module.Variables)

for varName := range op.Variables {
parsedVar, parsed := parsedVars[varName]

decl, ok := lr.Config.Module.Variables[varName]
if !ok {
if !ok || !parsed {
// We'll try to parse this and handle diagnostics for missing
// variables with ParseUndeclaredVariableValues after.
undeclaredVariables[varName] = rawV
continue
}

// We're "parsing" only to get the resulting value's SourceType,
// so we'll use configs.VariableParseLiteral just because it's
// the most liberal interpretation and so least likely to
// fail with an unrelated error.
v, _ := rawV.ParseVariableValue(configs.VariableParseLiteral)
if v == nil {
// We'll ignore any that don't parse at all, because
// they'll fail elsewhere in this process anyway.
undeclaredVariables[varName] = op.Variables[varName]
continue
}

var rng *hcl.Range
if v.HasSourceRange() {
rng = v.SourceRange.ToHCL().Ptr()
if parsedVar.HasSourceRange() {
rng = parsedVar.SourceRange.ToHCL().Ptr()
}

// If the var is declared as ephemeral in config, go ahead and handle it
if ok && decl.Ephemeral {
if decl.Ephemeral {
// Determine whether this is an apply-time variable, i.e. an
// ephemeral variable that was set (non-null) during the
// planning phase.
Expand All @@ -311,17 +304,9 @@ func (b *Local) opApply(
continue
}

// Get the value of the variable, because we'll need it for
// the next two steps.
val, valDiags := rawV.ParseVariableValue(decl.ParsingMode)
diags = diags.Append(valDiags)
if valDiags.HasErrors() {
continue
}

// If this is an apply-time variable, the user must supply a
// value during apply: it can't be null.
if applyTimeVar && val.Value.IsNull() {
if applyTimeVar && parsedVar.Value.IsNull() {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Ephemeral variable must be set for apply",
Expand All @@ -336,17 +321,17 @@ func (b *Local) opApply(
// If we get here, we are in possession of a non-null
// ephemeral apply-time input variable, and need only pass
// its value on to the ApplyOpts.
applyTimeValues[varName] = val
applyTimeValues[varName] = parsedVar
} else {
// If a non-ephemeral variable is set differently between plan and apply, we should emit a diagnostic.
plannedVariableValue, ok := plan.VariableValues[varName]
if !ok {
// We'll catch this with ParseUndeclaredVariableValues after
undeclaredVariables[varName] = rawV
undeclaredVariables[varName] = op.Variables[varName]
continue
}

val, err := plannedVariableValue.Decode(cty.DynamicPseudoType)
plannedVar, err := plannedVariableValue.Decode(cty.DynamicPseudoType)
if err != nil {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Expand All @@ -355,11 +340,11 @@ func (b *Local) opApply(
Subject: rng,
})
} else {
if v.Value.Equals(val).False() {
if parsedVar.Value.Equals(plannedVar).False() {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Can't change variable when applying a saved plan",
Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, viewsjson.CompactValueStr(v.Value), viewsjson.CompactValueStr(val), v.SourceType.DiagnosticLabel()),
Detail: fmt.Sprintf("The variable %s cannot be set using the -var and -var-file options when applying a saved plan file, because a saved plan includes the variable values that were set when it was created. The saved plan specifies %s as the value whereas during apply the value %s was %s. To declare an ephemeral variable which is not saved in the plan file, use ephemeral = true.", varName, viewsjson.CompactValueStr(parsedVar.Value), viewsjson.CompactValueStr(plannedVar), parsedVar.SourceType.DiagnosticLabel()),
Subject: rng,
})
}
Expand Down
17 changes: 5 additions & 12 deletions internal/command/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -964,16 +964,10 @@ func TestApply_planWithVarFileChangingVariableValue(t *testing.T) {
}
}

func TestApply_planVars(t *testing.T) {
// This test ensures that it isn't allowed to set non-ephemeral input
// variables when applying from a saved plan file, since in that case the
// variable values come from the saved plan file.
//
// This situation was originally checked by the apply command itself,
// and that's what this test was originally exercising. This rule
// is now enforced by the "local" backend instead, but this test
// is still valid since the command instance delegates to the
// local backend.
func TestApply_planUndeclaredVars(t *testing.T) {
// This test ensures that it isn't allowed to set undeclared input variables
// when applying from a saved plan file, since in that case the variable
// values come from the saved plan file.

planPath := applyFixturePlanFile(t)
statePath := testTempFile(t)
Expand Down Expand Up @@ -1076,7 +1070,6 @@ foo = "bar"

"with planfile passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) {
t.Setenv("TF_VAR_foo", "bar")
defer t.Setenv("TF_VAR_foo", "")

args := []string{
"-state", statePath,
Expand Down Expand Up @@ -1168,7 +1161,7 @@ foo = "bar"

"without planfile passing ephemeral variable through environment variable": func(t *testing.T, c *ApplyCommand, statePath, planPath string, done func(*testing.T) *terminal.TestOutput) {
t.Setenv("TF_VAR_foo", "bar")
defer t.Setenv("TF_VAR_foo", "")
t.Setenv("TF_VAR_unused", `{key:"val"}`)

args := []string{
"-state", statePath,
Expand Down
5 changes: 5 additions & 0 deletions internal/command/testdata/apply-ephemeral-variable/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ variable "bar" {
ephemeral = true
}

variable "unused" {
type = map(string)
default = null
}

resource "test_instance" "foo" {
ami = "bar"
}
3 changes: 2 additions & 1 deletion internal/lang/funcs/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,11 @@ func MakeTemplateFileFunc(baseDir string, funcsCb func() (funcs map[string]funct
if err != nil {
return cty.DynamicPseudoType, err
}
vars, _ := args[1].UnmarkDeep()

// This is safe even if args[1] contains unknowns because the HCL
// template renderer itself knows how to short-circuit those.
val, err := renderTmpl(expr, args[1])
val, err := renderTmpl(expr, vars)
return val.Type(), err
},
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/lang/funcs/filesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ func TestTemplateFile(t *testing.T) {
cty.StringVal("a"),
cty.StringVal("b").Mark("var"),
cty.StringVal("c"),
}),
}).Mark("vars"),
}),
cty.StringVal("- a\n- b\n- c\n").Mark("path").Mark("var"),
cty.StringVal("- a\n- b\n- c\n").Mark("path").Mark("var").Mark("vars"),
``,
},
{
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/context_apply_deferred_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2915,7 +2915,7 @@ import {
wantDeferred: make(map[string]ExpectedDeferred),
wantDiagnostic: func(diags tfdiags.Diagnostics) bool {
for _, diag := range diags {
if diag.Description().Summary == "Use of import for_each in an invalid context" {
if diag.Description().Summary == "Resource has no configuration" {
return true
}
}
Expand Down
13 changes: 0 additions & 13 deletions internal/terraform/node_resource_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,20 +572,7 @@ func (n *NodeValidatableResource) validateImportTargets(ctx EvalContext) tfdiags
return diags
}

// Resource config might be nil here since we are also validating config generation.
expanded := n.Config != nil && (n.Config.ForEach != nil || n.Config.Count != nil)

if imp.Config.ForEach != nil {
if !expanded {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Use of import for_each in an invalid context",
Detail: "Use of for_each in import requires a resource using count or for_each.",
// FIXME: minor issue, but this points to the for_each expression rather than for_each itself.
Subject: imp.Config.ForEach.Range().Ptr(),
})
}

forEachData, _, forEachDiags := newForEachEvaluator(imp.Config.ForEach, ctx, true).ImportValues()
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
Expand Down
15 changes: 9 additions & 6 deletions website/docs/language/backend/s3.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ Using the example above, the state for the workspace `development` would be stor
When not using [workspaces](/terraform/language/state/workspaces)(or when only using the `default` workspace), Terraform will need the following AWS IAM permissions on the target backend bucket:

* `s3:ListBucket` on `arn:aws:s3:::mybucket`. At a minimum, this must be able to list the path where the state is stored.
* `s3:GetObject` on `arn:aws:s3:::mybucket/path/to/my/key`
* `s3:PutObject` on `arn:aws:s3:::mybucket/path/to/my/key`
* `s3:GetObject` on `arn:aws:s3:::mybucket/path/to/my/key` and `arn:aws:s3:::mybucket/path/to/my/key.tflock`
* `s3:PutObject` on `arn:aws:s3:::mybucket/path/to/my/key` and `arn:aws:s3:::mybucket/path/to/my/key.tflock`

Note: `s3:DeleteObject` is not needed, as Terraform will not delete the state storage.

Expand All @@ -66,7 +66,10 @@ This is seen in the following AWS IAM Statement:
{
"Effect": "Allow",
"Action": ["s3:GetObject", "s3:PutObject"],
"Resource": "arn:aws:s3:::mybucket/path/to/my/key"
"Resource": [
"arn:aws:s3:::mybucket/path/to/my/key",
"arn:aws:s3:::mybucket/path/to/my/key.tflock"
]
}
]
}
Expand All @@ -75,9 +78,9 @@ This is seen in the following AWS IAM Statement:
When using [workspaces](/terraform/language/state/workspaces), Terraform will also need permissions to create, list, read, update, and delete the workspace state storage:

* `s3:ListBucket` on `arn:aws:s3:::mybucket`. At a minumum, this must be able to list the path where the `default` workspace is stored as well as the other workspaces.
* `s3:GetObject` on `arn:aws:s3:::mybucket/path/to/my/key` and `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key`
* `s3:PutObject` on `arn:aws:s3:::mybucket/path/to/my/key` and `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key`
* `s3:DeleteObject` on `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key`
* `s3:GetObject` on `arn:aws:s3:::mybucket/path/to/my/key`, `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key` and `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key.tflock`
* `s3:PutObject` on `arn:aws:s3:::mybucket/path/to/my/key`, `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key` and `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key.tflock`
* `s3:DeleteObject` on `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key` and `arn:aws:s3:::mybucket/<workspace_key_prefix>/*/path/to/my/key.tflock`

-> **Note:** AWS can control access to S3 buckets with either IAM policies
attached to users/groups/roles (like the example above) or resource policies
Expand Down
2 changes: 1 addition & 1 deletion website/docs/language/expressions/conditionals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ A common use of conditional expressions is to define defaults to replace
invalid values:

```hcl
var.a != "" ? var.a : "default-a"
var.a == "" ? "default-a" : var.a
```

If `var.a` is an empty string then the result is `"default-a"`, but otherwise
Expand Down

0 comments on commit 86ca532

Please sign in to comment.