diff --git a/CHANGELOG.md b/CHANGELOG.md index 9198819b4d..f4a6bb2beb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ENHANCEMENTS: * `tofu show` now supports `-config` and `-module=DIR` options, to be used in conjunction with `-json` to produce a machine-readable summary of either the whole configuration or a single module without first creating a plan. ([#2820](https://github.com/opentofu/opentofu/pull/2820), [#3003](https://github.com/opentofu/opentofu/pull/3003)) * [The JSON representation of configuration](https://opentofu.org/docs/internals/json-format/#configuration-representation) returned by `tofu show` in `-json` mode now includes type constraint information for input variables and whether each input variable is required, in addition to the existing properties related to input variables. ([#3013](https://github.com/opentofu/opentofu/pull/3013)) * Multiline string updates in arrays are now diffed line-by-line, rather than as a single element, making it easier to see changes in the plan output. ([#3030](https://github.com/opentofu/opentofu/pull/3030)) +* Add full support for -var, -var-file, and TF_VARS during `tofu apply` to support plan encryption ([#1998](https://github.com/opentofu/opentofu/pull/1998)) BUG FIXES: diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 73793703c4..f3e14d8da5 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -12,7 +12,6 @@ import ( "sort" "strings" - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" "github.com/opentofu/opentofu/internal/backend" @@ -268,31 +267,7 @@ func (b *Local) localRunForPlanFile(ctx context.Context, op *backend.Operation, // we need to apply the plan. run.Plan = plan - subCall := op.RootCall.WithVariables(func(variable *configs.Variable) (cty.Value, hcl.Diagnostics) { - var diags hcl.Diagnostics - - name := variable.Name - v, ok := plan.VariableValues[name] - if !ok { - if variable.Required() { - // This should not happen... - return cty.DynamicVal, diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Missing plan variable " + variable.Name, - }) - } - return variable.Default, nil - } - - parsed, parsedErr := v.Decode(cty.DynamicPseudoType) - if parsedErr != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: parsedErr.Error(), - }) - } - return parsed, diags - }) + subCall := op.RootCall.WithVariables(plan.VariableMapper()) loader := configload.NewLoaderFromSnapshot(snap) config, configDiags := loader.LoadConfig(ctx, snap.Modules[""].Dir, subCall) @@ -302,6 +277,36 @@ func (b *Local) localRunForPlanFile(ctx context.Context, op *backend.Operation, } run.Config = config + // Check that all provided variables are in the configuration + _, undeclaredDiags := backend.ParseUndeclaredVariableValues(op.Variables, config.Module.Variables) + diags = diags.Append(undeclaredDiags) + // Check that all variables provided match + for varName, varCfg := range config.Module.Variables { + if _, ok := op.Variables[varName]; ok { + // Variable provided via cli/files/env/etc... + inputValue, inputDiags := op.RootCall.Variables()(varCfg) + // Variable provided via the plan + planValue, planDiags := subCall.Variables()(varCfg) + + diags = diags.Append(inputDiags).Append(planDiags) + if inputDiags.HasErrors() || planDiags.HasErrors() { + return nil, snap, diags + } + + if inputValue.Equals(planValue).False() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Mismatch between input and plan variable value", + fmt.Sprintf("Value saved in the plan file for variable %q is different from the one given to the current command.", varName), + )) + } + } + } + + if diags.HasErrors() { + return nil, snap, diags + } + // NOTE: We're intentionally comparing the current locks with the // configuration snapshot, rather than the lock snapshot in the plan file, // because it's the current locks which dictate our plugin selections diff --git a/internal/command/apply.go b/internal/command/apply.go index fe71902816..cb49b8161a 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -89,17 +89,6 @@ func (c *ApplyCommand) Run(rawArgs []string) int { return 1 } - // Check for invalid combination of plan file and variable overrides - if planFile != nil && !args.Vars.Empty() { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Can't set variables when applying a saved plan", - "The -var and -var-file options cannot be used when applying a saved plan file, because a saved plan includes the variable values that were set when it was created.", - )) - view.Diagnostics(diags) - return 1 - } - // FIXME: the -input flag value is needed to initialize the backend and the // operation, but there is no clear path to pass this value down, so we // continue to mutate the Meta object state for now. diff --git a/internal/command/e2etest/encryption_test.go b/internal/command/e2etest/encryption_test.go index f5d82b17fd..4e6971593a 100644 --- a/internal/command/e2etest/encryption_test.go +++ b/internal/command/e2etest/encryption_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "github.com/google/uuid" "github.com/opentofu/opentofu/internal/e2e" ) @@ -48,7 +49,7 @@ func (r tofuResult) Failure() tofuResult { func SanitizeStderr(msg string) string { // ANSI escape sequence regex for removing terminal color codes and control characters msg = stripAnsi(msg) - //Pipe and carriage return replacement in order to correctly sanitze the stderr output + // Pipe and carriage return replacement in order to correctly sanitze the stderr output msg = strings.ReplaceAll( strings.ReplaceAll(msg, "│", ""), "\n", "", @@ -102,17 +103,27 @@ func TestEncryptionFlow(t *testing.T) { stdout, stderr, err := tf.Run(args...) return tofuResult{t, stdout, stderr, err} } - apply := func() tofuResult { + apply := func(args ...string) tofuResult { iter += 1 - return run("apply", fmt.Sprintf("-var=iter=%v", iter), "-auto-approve") + finalArgs := []string{"apply"} + finalArgs = append(finalArgs, fmt.Sprintf("-var=iter=%v", iter), "-auto-approve") + finalArgs = append(finalArgs, args...) + return run(finalArgs...) } - createPlan := func(planfile string) tofuResult { + createPlan := func(planfile string, args ...string) tofuResult { iter += 1 - return run("plan", fmt.Sprintf("-var=iter=%v", iter), "-out="+planfile) + args = append([]string{"plan", fmt.Sprintf("-var=iter=%v", iter), "-out=" + planfile}, args...) + return run(args...) } - applyPlan := func(planfile string) tofuResult { - return run("apply", "-auto-approve", planfile) + applyPlan := func(planfile string, args ...string) tofuResult { + finalArgs := []string{"apply", "-auto-approve"} + finalArgs = append(finalArgs, args...) + finalArgs = append(finalArgs, planfile) + return run(finalArgs...) + } + withVarArg := func(key, value string) string { + return fmt.Sprintf(`-var=%s=%s`, key, value) } requireUnencryptedState := func() { @@ -150,103 +161,107 @@ func TestEncryptionFlow(t *testing.T) { unencryptedPlan := "unencrypted.tfplan" encryptedPlan := "encrypted.tfplan" - + correctPassphrase := uuid.NewString() { // Everything works before adding encryption configuration - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() // Check read/write of state file - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() // Save an unencrypted plan - createPlan(unencryptedPlan).Success() + createPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).Success() + // Validate that OpenTofu does not allow different -var value for a variable between creation of the plan and its execution. + applyPlan(unencryptedPlan, withVarArg("passphrase", "different-value-than-the-one-saved-in-the-planfile")). + StderrContains(`Value saved in the plan file for variable "passphrase" is different from the one given to the current command`) // Validate unencrypted plan - applyPlan(unencryptedPlan).Success() + applyPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() } with("required.tf", func() { // Can't switch directly to encryption, need to migrate - apply().Failure().StderrContains("encountered unencrypted payload without unencrypted method") + apply(withVarArg("passphrase", correctPassphrase)).Failure().StderrContains("encountered unencrypted payload without unencrypted method") requireUnencryptedState() }) with("migrateto.tf", func() { // Migrate to using encryption - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireEncryptedState() // Make changes and confirm it's still encrypted (even with migration enabled) - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireEncryptedState() // Save an encrypted plan - createPlan(encryptedPlan).Success() + createPlan(encryptedPlan, withVarArg("passphrase", correctPassphrase)).Success() // Apply encrypted plan (with migration active) - applyPlan(encryptedPlan).Success() + applyPlan(encryptedPlan, withVarArg("passphrase", correctPassphrase)).Success() requireEncryptedState() // Apply unencrypted plan (with migration active) - applyPlan(unencryptedPlan).StderrContains("Saved plan is stale") + applyPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).StderrContains("Saved plan is stale") requireEncryptedState() }) { // Unconfigured encryption clearly fails on encrypted state - apply().Failure().StderrContains("can not be read without an encryption configuration") + apply(withVarArg("passphrase", correctPassphrase)).Failure().StderrContains("can not be read without an encryption configuration") } with("required.tf", func() { // Encryption works with fallback removed - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireEncryptedState() // Can't apply unencrypted plan - applyPlan(unencryptedPlan).Failure().StderrContains("encountered unencrypted payload without unencrypted method") + applyPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).Failure().StderrContains("encountered unencrypted payload without unencrypted method") requireEncryptedState() // Apply encrypted plan - applyPlan(encryptedPlan).StderrContains("Saved plan is stale") + applyPlan(encryptedPlan, withVarArg("passphrase", correctPassphrase)).StderrContains("Saved plan is stale") requireEncryptedState() }) - with("broken.tf", func() { + with("required.tf", func() { // But with the wrong passphrase + incorrectPassphrase := uuid.NewString() // Make sure changes to encryption keys notify the user correctly - apply().Failure().StderrContains("decryption failed for state") + apply(withVarArg("passphrase", incorrectPassphrase)).Failure().StderrContains("decryption failed for state") requireEncryptedState() - applyPlan(encryptedPlan).Failure().StderrContains("decryption failed: cipher: message authentication failed") + applyPlan(encryptedPlan, withVarArg("passphrase", incorrectPassphrase)).Failure().StderrContains("decryption failed: cipher: message authentication failed") requireEncryptedState() }) with("migratefrom.tf", func() { // Apply migration from encrypted state - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() // Make changes and confirm it's still encrypted (even with migration enabled) - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() // Apply unencrypted plan (with migration active) - applyPlan(unencryptedPlan).StderrContains("Saved plan is stale") + applyPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).StderrContains("Saved plan is stale") requireUnencryptedState() // Apply encrypted plan (with migration active) - applyPlan(encryptedPlan).StderrContains("Saved plan is stale") + applyPlan(encryptedPlan, withVarArg("passphrase", correctPassphrase)).StderrContains("Saved plan is stale") requireUnencryptedState() }) { // Back to no encryption configuration with unencrypted state - apply().Success() + apply(withVarArg("passphrase", correctPassphrase)).Success() requireUnencryptedState() // Apply unencrypted plan - applyPlan(unencryptedPlan).StderrContains("Saved plan is stale") + applyPlan(unencryptedPlan, withVarArg("passphrase", correctPassphrase)).StderrContains("Saved plan is stale") requireUnencryptedState() // Can't apply encrypted plan - applyPlan(encryptedPlan).Failure().StderrContains("the given plan file is encrypted and requires a valid encryption") + applyPlan(encryptedPlan, withVarArg("passphrase", correctPassphrase)).Failure().StderrContains("the given plan file is encrypted and requires a valid encryption") requireUnencryptedState() } } diff --git a/internal/command/e2etest/testdata/encryption-flow/broken.tf.disabled b/internal/command/e2etest/testdata/encryption-flow/broken.tf.disabled deleted file mode 100644 index 46176db92d..0000000000 --- a/internal/command/e2etest/testdata/encryption-flow/broken.tf.disabled +++ /dev/null @@ -1,29 +0,0 @@ -variable "passphrase" { - type = string - default = "aaaaaaaa-83f1-47ec-9b2d-2aebf6417167" -} - -locals { - key_length = 32 -} - -terraform { - encryption { - key_provider "pbkdf2" "basic" { - passphrase = var.passphrase - key_length = local.key_length - iterations = 200000 - hash_function = "sha512" - salt_length = 12 - } - method "aes_gcm" "example" { - keys = key_provider.pbkdf2.basic - } - state { - method = method.aes_gcm.example - } - plan { - method = method.aes_gcm.example - } - } -} diff --git a/internal/command/e2etest/testdata/encryption-flow/migratefrom.tf.disabled b/internal/command/e2etest/testdata/encryption-flow/migratefrom.tf.disabled index d16f50bef8..0d1a4deb5b 100644 --- a/internal/command/e2etest/testdata/encryption-flow/migratefrom.tf.disabled +++ b/internal/command/e2etest/testdata/encryption-flow/migratefrom.tf.disabled @@ -1,8 +1,8 @@ terraform { encryption { key_provider "pbkdf2" "basic" { - passphrase = "26281afb-83f1-47ec-9b2d-2aebf6417167" - key_length = 32 + passphrase = var.passphrase + key_length = local.key_length iterations = 200000 hash_function = "sha512" salt_length = 12 diff --git a/internal/command/e2etest/testdata/encryption-flow/migrateto.tf.disabled b/internal/command/e2etest/testdata/encryption-flow/migrateto.tf.disabled index a5d0e841df..937a7a624d 100644 --- a/internal/command/e2etest/testdata/encryption-flow/migrateto.tf.disabled +++ b/internal/command/e2etest/testdata/encryption-flow/migrateto.tf.disabled @@ -1,8 +1,8 @@ terraform { encryption { key_provider "pbkdf2" "basic" { - passphrase = "26281afb-83f1-47ec-9b2d-2aebf6417167" - key_length = 32 + passphrase = var.passphrase + key_length = local.key_length iterations = 200000 hash_function = "sha512" salt_length = 12 diff --git a/internal/command/e2etest/testdata/encryption-flow/required.tf.disabled b/internal/command/e2etest/testdata/encryption-flow/required.tf.disabled index 4aa5e7c815..3cb24cbf84 100644 --- a/internal/command/e2etest/testdata/encryption-flow/required.tf.disabled +++ b/internal/command/e2etest/testdata/encryption-flow/required.tf.disabled @@ -1,13 +1,3 @@ -variable "passphrase" { - type = string - default = "26281afb-83f1-47ec-9b2d-2aebf6417167" - sensitive = true -} - -locals { - key_length = sensitive(32) -} - terraform { encryption { key_provider "pbkdf2" "basic" { diff --git a/internal/command/e2etest/testdata/encryption-flow/variables.tf b/internal/command/e2etest/testdata/encryption-flow/variables.tf new file mode 100644 index 0000000000..10239a8f2b --- /dev/null +++ b/internal/command/e2etest/testdata/encryption-flow/variables.tf @@ -0,0 +1,14 @@ +// When the migration to encrypted plan and state is wanted, +// in case the passphrase for the encryption is given via +// -var/-var-file, it is recommended to add the variable +// handling that to the configuration before adding the +// encryption configuration. Apply this, and only after start +// with the encryption configuration. +variable "passphrase" { + type = string + sensitive = true +} + +locals { + key_length = sensitive(32) +} \ No newline at end of file diff --git a/internal/command/show.go b/internal/command/show.go index b9b5030e6c..f2b4a5c0da 100644 --- a/internal/command/show.go +++ b/internal/command/show.go @@ -12,8 +12,6 @@ import ( "os" "strings" - "github.com/hashicorp/hcl/v2" - "github.com/zclconf/go-cty/cty" otelAttr "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -460,32 +458,7 @@ func getDataFromPlanfileReader(ctx context.Context, planReader *planfile.Reader, return nil, nil, nil, err } - subCall := rootCall.WithVariables(func(variable *configs.Variable) (cty.Value, hcl.Diagnostics) { - var diags hcl.Diagnostics - - name := variable.Name - v, ok := plan.VariableValues[name] - if !ok { - if variable.Required() { - // This should not happen... - return cty.DynamicVal, diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Missing plan variable " + variable.Name, - }) - } - return variable.Default, nil - } - - parsed, parsedErr := v.Decode(cty.DynamicPseudoType) - if parsedErr != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: parsedErr.Error(), - }) - } - return parsed, diags - }) - + subCall := rootCall.WithVariables(plan.VariableMapper()) // Get config config, diags := planReader.ReadConfig(ctx, subCall) if diags.HasErrors() { diff --git a/internal/configs/static_evaluator.go b/internal/configs/static_evaluator.go index 86037c7136..b0ea82d06e 100644 --- a/internal/configs/static_evaluator.go +++ b/internal/configs/static_evaluator.go @@ -52,6 +52,10 @@ func NewStaticModuleCall(addr addrs.Module, vars StaticModuleVariables, rootPath } } +func (s StaticModuleCall) Variables() StaticModuleVariables { + return s.vars +} + func (s StaticModuleCall) WithVariables(vars StaticModuleVariables) StaticModuleCall { return StaticModuleCall{ addr: s.addr, diff --git a/internal/plans/plan.go b/internal/plans/plan.go index de2ef88628..649624123b 100644 --- a/internal/plans/plan.go +++ b/internal/plans/plan.go @@ -9,9 +9,11 @@ import ( "sort" "time" + "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" "github.com/opentofu/opentofu/internal/addrs" + "github.com/opentofu/opentofu/internal/configs" "github.com/opentofu/opentofu/internal/configs/configschema" "github.com/opentofu/opentofu/internal/lang/globalref" "github.com/opentofu/opentofu/internal/states" @@ -198,6 +200,35 @@ func (p *Plan) ProviderAddrs() []addrs.AbsProviderConfig { return ret } +// VariableMapper checks that all the provided variables match what has been provided while building the plan. +func (plan *Plan) VariableMapper() configs.StaticModuleVariables { + return func(variable *configs.Variable) (cty.Value, hcl.Diagnostics) { + var diags hcl.Diagnostics + + name := variable.Name + v, ok := plan.VariableValues[name] + if !ok { + if variable.Required() { + // This should not happen... + return cty.DynamicVal, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Missing plan variable " + variable.Name, + }) + } + return variable.Default, nil + } + + parsed, parsedErr := v.Decode(cty.DynamicPseudoType) + if parsedErr != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: parsedErr.Error(), + }) + } + return parsed, diags + } +} + // Backend represents the backend-related configuration and other data as it // existed when a plan was created. type Backend struct { diff --git a/internal/plans/plan_test.go b/internal/plans/plan_test.go index b650acc089..71f1b71505 100644 --- a/internal/plans/plan_test.go +++ b/internal/plans/plan_test.go @@ -9,6 +9,9 @@ import ( "testing" "github.com/go-test/deep" + "github.com/opentofu/opentofu/internal/configs" + "github.com/zclconf/go-cty/cty" + ctymsgpack "github.com/zclconf/go-cty/cty/msgpack" "github.com/opentofu/opentofu/internal/addrs" ) @@ -98,3 +101,56 @@ func TestModuleOutputChangesEmpty(t *testing.T) { t.Fatal("plan has no visible changes") } } + +// TestVariableMapper checks that the mapper is decoding types correctly from the plan +func TestVariableMapper(t *testing.T) { + val1 := cty.StringVal("string value") + val2 := cty.ObjectVal(map[string]cty.Value{"foo": cty.StringVal("bar")}) + val3 := cty.MapVal(map[string]cty.Value{ + "inner": cty.SetVal([]cty.Value{cty.StringVal("baz")}), + }) + val4 := cty.ListVal([]cty.Value{cty.BoolVal(false)}) + val5 := cty.SetVal([]cty.Value{ + cty.ObjectVal( + map[string]cty.Value{ + "inner": cty.ObjectVal(map[string]cty.Value{"foo": cty.NumberIntVal(25)}), + }, + ), + }) + p := Plan{VariableValues: map[string]DynamicValue{ + "raw_string": encodeDynamicValueWithType(t, val1, cty.DynamicPseudoType), + "object_of_strings": encodeDynamicValueWithType(t, val2, cty.DynamicPseudoType), + "map_of_sets_of_strings": encodeDynamicValueWithType(t, val3, cty.DynamicPseudoType), + "list_of_bools": encodeDynamicValueWithType(t, val4, cty.DynamicPseudoType), + "set_of_obj_of_obj_of_number": encodeDynamicValueWithType(t, val5, cty.DynamicPseudoType), + }} + + vm := p.VariableMapper() + + cases := map[string]cty.Value{ + "raw_string": val1, + "object_of_strings": val2, + "map_of_sets_of_strings": val3, + "list_of_bools": val4, + "set_of_obj_of_obj_of_number": val5, + } + for varName, wantVal := range cases { + t.Run(varName, func(t *testing.T) { + val, diag := vm(&configs.Variable{Name: varName}) + if diag.HasErrors() { + t.Fatalf("unexpected diagnostics from the variable mapper: %s", diag) + } + if !val.RawEquals(wantVal) { + t.Fatalf("returned value is not equal with the expected one.\n\twant:%s\n\tgot:%s\n", wantVal, val) + } + }) + } +} + +func encodeDynamicValueWithType(t *testing.T, value cty.Value, ty cty.Type) []byte { + data, err := ctymsgpack.Marshal(value, ty) + if err != nil { + t.Fatalf("failed to marshal JSON: %s", err) + } + return data +}