diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index b06de8016..f460dc652 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -84,6 +84,13 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload. log.Printf("[TRACE] backend/local: retrieving local state snapshot for workspace %q", op.Workspace) opts.State = s.State() + // Prepare a separate opts and context for validation, which doesn't use + // any state ensuring that we only validate the config, since evaluation + // will automatically reference the state when available. + validateOpts := opts + validateOpts.State = nil + var validateCtx *terraform.Context + var tfCtx *terraform.Context var ctxDiags tfdiags.Diagnostics var configSnap *configload.Snapshot @@ -101,9 +108,18 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload. // Write sources into the cache of the main loader so that they are // available if we need to generate diagnostic message snippets. op.ConfigLoader.ImportSourcesFromSnapshot(configSnap) + + // create a validation context with no state + validateCtx, _, _ = b.contextFromPlanFile(op.PlanFile, validateOpts, stateMeta) + // diags from here will be caught above + } else { log.Printf("[TRACE] backend/local: building context for current working directory") tfCtx, configSnap, ctxDiags = b.contextDirect(op, opts) + + // create a validation context with no state + validateCtx, _, _ = b.contextDirect(op, validateOpts) + // diags from here will be caught above } diags = diags.Append(ctxDiags) if diags.HasErrors() { @@ -129,7 +145,7 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload. // If validation is enabled, validate if b.OpValidation { log.Printf("[TRACE] backend/local: running validation operation") - validateDiags := tfCtx.Validate() + validateDiags := validateCtx.Validate() diags = diags.Append(validateDiags) } } diff --git a/command/console_test.go b/command/console_test.go index 5f1cfe240..b8e73fe33 100644 --- a/command/console_test.go +++ b/command/console_test.go @@ -8,8 +8,11 @@ import ( "strings" "testing" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/helper/copy" + "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" + "github.com/zclconf/go-cty/cty" ) // ConsoleCommand is tested primarily with tests in the "repl" package. @@ -60,6 +63,16 @@ func TestConsole_tfvars(t *testing.T) { } p := testProvider() + p.GetSchemaReturn = &terraform.ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "value": {Type: cty.String, Optional: true}, + }, + }, + }, + } + ui := new(cli.MockUi) c := &ConsoleCommand{ Meta: Meta{ @@ -98,6 +111,15 @@ func TestConsole_unsetRequiredVars(t *testing.T) { defer testFixCwd(t, tmp, cwd) p := testProvider() + p.GetSchemaReturn = &terraform.ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "test_instance": { + Attributes: map[string]*configschema.Attribute{ + "value": {Type: cty.String, Optional: true}, + }, + }, + }, + } ui := new(cli.MockUi) c := &ConsoleCommand{ Meta: Meta{ @@ -142,7 +164,7 @@ func TestConsole_modules(t *testing.T) { defer os.RemoveAll(td) defer testChdir(t, td)() - p := testProvider() + p := applyFixtureProvider() ui := new(cli.MockUi) c := &ConsoleCommand{ diff --git a/command/graph_test.go b/command/graph_test.go index bb54d2d7a..c7696464f 100644 --- a/command/graph_test.go +++ b/command/graph_test.go @@ -20,7 +20,7 @@ func TestGraph(t *testing.T) { ui := new(cli.MockUi) c := &GraphCommand{ Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), + testingOverrides: metaOverridesForProvider(applyFixtureProvider()), Ui: ui, }, } @@ -42,7 +42,7 @@ func TestGraph_multipleArgs(t *testing.T) { ui := new(cli.MockUi) c := &GraphCommand{ Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), + testingOverrides: metaOverridesForProvider(applyFixtureProvider()), Ui: ui, }, } @@ -69,7 +69,7 @@ func TestGraph_noArgs(t *testing.T) { ui := new(cli.MockUi) c := &GraphCommand{ Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), + testingOverrides: metaOverridesForProvider(applyFixtureProvider()), Ui: ui, }, } @@ -94,7 +94,7 @@ func TestGraph_noConfig(t *testing.T) { ui := new(cli.MockUi) c := &GraphCommand{ Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), + testingOverrides: metaOverridesForProvider(applyFixtureProvider()), Ui: ui, }, } @@ -148,7 +148,7 @@ func TestGraph_plan(t *testing.T) { ui := new(cli.MockUi) c := &GraphCommand{ Meta: Meta{ - testingOverrides: metaOverridesForProvider(testProvider()), + testingOverrides: metaOverridesForProvider(applyFixtureProvider()), Ui: ui, }, } diff --git a/command/testdata/apply-vars/main.tf b/command/testdata/apply-vars/main.tf index 005abad09..1d6da85c3 100644 --- a/command/testdata/apply-vars/main.tf +++ b/command/testdata/apply-vars/main.tf @@ -1,5 +1,5 @@ variable "foo" {} resource "test_instance" "foo" { - value = "${var.foo}" + value = var.foo } diff --git a/terraform/eval_state_upgrade.go b/terraform/eval_state_upgrade.go index acf671c68..e03349c37 100644 --- a/terraform/eval_state_upgrade.go +++ b/terraform/eval_state_upgrade.go @@ -1,6 +1,7 @@ package terraform import ( + "encoding/json" "fmt" "log" @@ -9,6 +10,7 @@ import ( "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" ) // UpgradeResourceState will, if necessary, run the provider-defined upgrade @@ -19,6 +21,17 @@ import ( // If any errors occur during upgrade, error diagnostics are returned. In that // case it is not safe to proceed with using the original state object. func UpgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Interface, src *states.ResourceInstanceObjectSrc, currentSchema *configschema.Block, currentVersion uint64) (*states.ResourceInstanceObjectSrc, tfdiags.Diagnostics) { + // Remove any attributes from state that are not present in the schema. + // This was previously taken care of by the provider, but data sources do + // not go through the UpgradeResourceState process. + // + // Legacy flatmap state is already taken care of during conversion. + // If the schema version is be changed, then allow the provider to handle + // removed attributes. + if len(src.AttrsJSON) > 0 && src.SchemaVersion == currentVersion { + src.AttrsJSON = stripRemovedStateAttributes(src.AttrsJSON, currentSchema.ImpliedType()) + } + if addr.Resource.Resource.Mode != addrs.ManagedResourceMode { // We only do state upgrading for managed resources. return src, nil @@ -105,3 +118,86 @@ func UpgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Int } return new, diags } + +// stripRemovedStateAttributes deletes any attributes no longer present in the +// schema, so that the json can be correctly decoded. +func stripRemovedStateAttributes(state []byte, ty cty.Type) []byte { + jsonMap := map[string]interface{}{} + err := json.Unmarshal(state, &jsonMap) + if err != nil { + // we just log any errors here, and let the normal decode process catch + // invalid JSON. + log.Printf("[ERROR] UpgradeResourceState: %s", err) + return state + } + + // if no changes were made, we return the original state to ensure nothing + // was altered in the marshaling process. + if !removeRemovedAttrs(jsonMap, ty) { + return state + } + + js, err := json.Marshal(jsonMap) + if err != nil { + // if the json map was somehow mangled enough to not marhsal, something + // went horribly wrong + panic(err) + } + + return js +} + +// strip out the actual missing attributes, and return a bool indicating if any +// changes were made. +func removeRemovedAttrs(v interface{}, ty cty.Type) bool { + modified := false + // we're only concerned with finding maps that correspond to object + // attributes + switch v := v.(type) { + case []interface{}: + switch { + // If these aren't blocks the next call will be a noop + case ty.IsListType() || ty.IsSetType(): + eTy := ty.ElementType() + for _, eV := range v { + modified = removeRemovedAttrs(eV, eTy) || modified + } + } + return modified + case map[string]interface{}: + switch { + case ty.IsMapType(): + // map blocks aren't yet supported, but handle this just in case + eTy := ty.ElementType() + for _, eV := range v { + modified = removeRemovedAttrs(eV, eTy) || modified + } + return modified + + case ty == cty.DynamicPseudoType: + log.Printf("[DEBUG] UpgradeResourceState: ignoring dynamic block: %#v\n", v) + return false + + case ty.IsObjectType(): + attrTypes := ty.AttributeTypes() + for attr, attrV := range v { + attrTy, ok := attrTypes[attr] + if !ok { + log.Printf("[DEBUG] UpgradeResourceState: attribute %q no longer present in schema", attr) + delete(v, attr) + modified = true + continue + } + + modified = removeRemovedAttrs(attrV, attrTy) || modified + } + return modified + default: + // This shouldn't happen, and will fail to decode further on, so + // there's no need to handle it here. + log.Printf("[WARN] UpgradeResourceState: unexpected type %#v for map in json state", ty) + return false + } + } + return modified +} diff --git a/terraform/eval_state_upgrade_test.go b/terraform/eval_state_upgrade_test.go new file mode 100644 index 000000000..11ef77b5f --- /dev/null +++ b/terraform/eval_state_upgrade_test.go @@ -0,0 +1,148 @@ +package terraform + +import ( + "reflect" + "testing" + + "github.com/zclconf/go-cty/cty" +) + +func TestStripRemovedStateAttributes(t *testing.T) { + cases := []struct { + name string + state map[string]interface{} + expect map[string]interface{} + ty cty.Type + modified bool + }{ + { + "removed string", + map[string]interface{}{ + "a": "ok", + "b": "gone", + }, + map[string]interface{}{ + "a": "ok", + }, + cty.Object(map[string]cty.Type{ + "a": cty.String, + }), + true, + }, + { + "removed null", + map[string]interface{}{ + "a": "ok", + "b": nil, + }, + map[string]interface{}{ + "a": "ok", + }, + cty.Object(map[string]cty.Type{ + "a": cty.String, + }), + true, + }, + { + "removed nested string", + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + "b": "removed", + }, + }, + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + }, + }, + cty.Object(map[string]cty.Type{ + "a": cty.String, + "b": cty.Object(map[string]cty.Type{ + "a": cty.String, + }), + }), + true, + }, + { + "removed nested list", + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + "b": []interface{}{"removed"}, + }, + }, + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + }, + }, + cty.Object(map[string]cty.Type{ + "a": cty.String, + "b": cty.Object(map[string]cty.Type{ + "a": cty.String, + }), + }), + true, + }, + { + "removed keys in set of objs", + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + "set": []interface{}{ + map[string]interface{}{ + "x": "ok", + "y": "removed", + }, + map[string]interface{}{ + "x": "ok", + "y": "removed", + }, + }, + }, + }, + map[string]interface{}{ + "a": "ok", + "b": map[string]interface{}{ + "a": "ok", + "set": []interface{}{ + map[string]interface{}{ + "x": "ok", + }, + map[string]interface{}{ + "x": "ok", + }, + }, + }, + }, + cty.Object(map[string]cty.Type{ + "a": cty.String, + "b": cty.Object(map[string]cty.Type{ + "a": cty.String, + "set": cty.Set(cty.Object(map[string]cty.Type{ + "x": cty.String, + })), + }), + }), + true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + modified := removeRemovedAttrs(tc.state, tc.ty) + if !reflect.DeepEqual(tc.state, tc.expect) { + t.Fatalf("expected: %#v\n got: %#v\n", tc.expect, tc.state) + } + if modified != tc.modified { + t.Fatal("incorrect return value") + } + }) + } +}