From 3326ab7dae540e83995c676797b68da99001fd49 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Thu, 17 Jun 2021 14:32:47 -0400 Subject: [PATCH] json-output: Omit unchanged resource_drift entries Previously, if any resources were found to have drifted, the JSON plan output would include a drift entry for every resource in state. This commit aligns the JSON plan output with the CLI UI, and only includes those resources where the old value does not equal the new value---i.e. drift has been detected. Also fixes a bug where the "address" field was missing from the drift output, and adds some test coverage. --- internal/command/jsonplan/plan.go | 7 + internal/command/show_test.go | 15 ++ .../command/testdata/show-json/drift/main.tf | 13 ++ .../testdata/show-json/drift/output.json | 174 ++++++++++++++++++ .../show-json/drift/terraform.tfstate | 38 ++++ .../multi-resource-update/output.json | 21 +++ 6 files changed, 268 insertions(+) create mode 100644 internal/command/testdata/show-json/drift/main.tf create mode 100644 internal/command/testdata/show-json/drift/output.json create mode 100644 internal/command/testdata/show-json/drift/terraform.tfstate diff --git a/internal/command/jsonplan/plan.go b/internal/command/jsonplan/plan.go index ee9a08947..1b90daf2d 100644 --- a/internal/command/jsonplan/plan.go +++ b/internal/command/jsonplan/plan.go @@ -259,6 +259,12 @@ func (p *plan) marshalResourceDrift(oldState, newState *states.State, schemas *t } else { newVal = cty.NullVal(ty) } + + if oldVal.RawEquals(newVal) { + // No drift if the two values are semantically equivalent + continue + } + oldSensitive := jsonstate.SensitiveAsBool(oldVal) newSensitive := jsonstate.SensitiveAsBool(newVal) oldVal, _ = oldVal.UnmarkDeep() @@ -290,6 +296,7 @@ func (p *plan) marshalResourceDrift(oldState, newState *states.State, schemas *t } change := resourceChange{ + Address: addr.String(), ModuleAddress: addr.Module.String(), Mode: "managed", // drift reporting is only for managed resources Name: addr.Resource.Resource.Name, diff --git a/internal/command/show_test.go b/internal/command/show_test.go index b5ed87797..b70ce14c1 100644 --- a/internal/command/show_test.go +++ b/internal/command/show_test.go @@ -636,6 +636,20 @@ func showFixtureSensitiveSchema() *providers.GetProviderSchemaResponse { func showFixtureProvider() *terraform.MockProvider { p := testProvider() p.GetProviderSchemaResponse = showFixtureSchema() + p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { + idVal := req.PriorState.GetAttr("id") + amiVal := req.PriorState.GetAttr("ami") + if amiVal.RawEquals(cty.StringVal("refresh-me")) { + amiVal = cty.StringVal("refreshed") + } + return providers.ReadResourceResponse{ + NewState: cty.ObjectVal(map[string]cty.Value{ + "id": idVal, + "ami": amiVal, + }), + Private: req.Private, + } + } p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { idVal := req.ProposedNewState.GetAttr("id") amiVal := req.ProposedNewState.GetAttr("ami") @@ -758,6 +772,7 @@ type plan struct { FormatVersion string `json:"format_version,omitempty"` Variables map[string]interface{} `json:"variables,omitempty"` PlannedValues map[string]interface{} `json:"planned_values,omitempty"` + ResourceDrift []interface{} `json:"resource_drift,omitempty"` ResourceChanges []interface{} `json:"resource_changes,omitempty"` OutputChanges map[string]interface{} `json:"output_changes,omitempty"` PriorState priorState `json:"prior_state,omitempty"` diff --git a/internal/command/testdata/show-json/drift/main.tf b/internal/command/testdata/show-json/drift/main.tf new file mode 100644 index 000000000..18995de7a --- /dev/null +++ b/internal/command/testdata/show-json/drift/main.tf @@ -0,0 +1,13 @@ +# In state with `ami = "foo"`, so this should be a regular update. The provider +# should not detect changes on refresh. +resource "test_instance" "no_refresh" { + ami = "bar" +} + +# In state with `ami = "refresh-me"`, but the provider will return +# `"refreshed"` after the refresh phase. The plan should show the drift +# (`"refresh-me"` to `"refreshed"`) and plan the update (`"refreshed"` to +# `"baz"`). +resource "test_instance" "should_refresh" { + ami = "baz" +} diff --git a/internal/command/testdata/show-json/drift/output.json b/internal/command/testdata/show-json/drift/output.json new file mode 100644 index 000000000..7badb45e5 --- /dev/null +++ b/internal/command/testdata/show-json/drift/output.json @@ -0,0 +1,174 @@ +{ + "format_version": "0.2", + "planned_values": { + "root_module": { + "resources": [ + { + "address": "test_instance.no_refresh", + "mode": "managed", + "type": "test_instance", + "name": "no_refresh", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "bar", + "id": "placeholder" + }, + "sensitive_values": {} + }, + { + "address": "test_instance.should_refresh", + "mode": "managed", + "type": "test_instance", + "name": "should_refresh", + "provider_name": "registry.terraform.io/hashicorp/test", + "schema_version": 0, + "values": { + "ami": "baz", + "id": "placeholder" + }, + "sensitive_values": {} + } + ] + } + }, + "resource_drift": [ + { + "address": "test_instance.should_refresh", + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "should_refresh", + "change": { + "actions": [ + "update" + ], + "before": { + "ami": "refresh-me", + "id": "placeholder" + }, + "after": { + "ami": "refreshed", + "id": "placeholder" + }, + "after_sensitive": {}, + "before_sensitive": {} + } + } + ], + "resource_changes": [ + { + "address": "test_instance.no_refresh", + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "no_refresh", + "change": { + "actions": [ + "update" + ], + "before": { + "ami": "foo", + "id": "placeholder" + }, + "after": { + "ami": "bar", + "id": "placeholder" + }, + "after_unknown": {}, + "after_sensitive": {}, + "before_sensitive": {} + } + }, + { + "address": "test_instance.should_refresh", + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "should_refresh", + "change": { + "actions": [ + "update" + ], + "before": { + "ami": "refreshed", + "id": "placeholder" + }, + "after": { + "ami": "baz", + "id": "placeholder" + }, + "after_unknown": {}, + "after_sensitive": {}, + "before_sensitive": {} + } + } + ], + "prior_state": { + "format_version": "0.2", + "values": { + "root_module": { + "resources": [ + { + "address": "test_instance.no_refresh", + "mode": "managed", + "type": "test_instance", + "name": "no_refresh", + "schema_version": 0, + "provider_name": "registry.terraform.io/hashicorp/test", + "values": { + "ami": "foo", + "id": "placeholder" + }, + "sensitive_values": {} + }, + { + "address": "test_instance.should_refresh", + "mode": "managed", + "type": "test_instance", + "name": "should_refresh", + "schema_version": 0, + "provider_name": "registry.terraform.io/hashicorp/test", + "values": { + "ami": "refreshed", + "id": "placeholder" + }, + "sensitive_values": {} + } + ] + } + } + }, + "configuration": { + "root_module": { + "resources": [ + { + "address": "test_instance.no_refresh", + "mode": "managed", + "type": "test_instance", + "name": "no_refresh", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "constant_value": "bar" + } + } + }, + { + "address": "test_instance.should_refresh", + "mode": "managed", + "type": "test_instance", + "name": "should_refresh", + "provider_config_key": "test", + "schema_version": 0, + "expressions": { + "ami": { + "constant_value": "baz" + } + } + } + ] + } + } +} diff --git a/internal/command/testdata/show-json/drift/terraform.tfstate b/internal/command/testdata/show-json/drift/terraform.tfstate new file mode 100644 index 000000000..02b8944d8 --- /dev/null +++ b/internal/command/testdata/show-json/drift/terraform.tfstate @@ -0,0 +1,38 @@ +{ + "version": 4, + "terraform_version": "0.12.0", + "serial": 7, + "lineage": "configuredUnchanged", + "resources": [ + { + "mode": "managed", + "type": "test_instance", + "name": "no_refresh", + "provider": "provider[\"registry.terraform.io/hashicorp/test\"]", + "instances": [ + { + "schema_version": 0, + "attributes": { + "ami": "foo", + "id": "placeholder" + } + } + ] + }, + { + "mode": "managed", + "type": "test_instance", + "name": "should_refresh", + "provider": "provider[\"registry.terraform.io/hashicorp/test\"]", + "instances": [ + { + "schema_version": 0, + "attributes": { + "ami": "refresh-me", + "id": "placeholder" + } + } + ] + } + ] +} diff --git a/internal/command/testdata/show-json/multi-resource-update/output.json b/internal/command/testdata/show-json/multi-resource-update/output.json index 4bf67352c..d84bc5b08 100644 --- a/internal/command/testdata/show-json/multi-resource-update/output.json +++ b/internal/command/testdata/show-json/multi-resource-update/output.json @@ -45,6 +45,27 @@ ] } }, + "resource_drift": [ + { + "address": "test_instance.test", + "mode": "managed", + "type": "test_instance", + "provider_name": "registry.terraform.io/hashicorp/test", + "name": "test", + "change": { + "actions": [ + "delete" + ], + "before": { + "ami": "bar", + "id": "placeholder" + }, + "after": null, + "before_sensitive": {}, + "after_sensitive": false + } + } + ], "resource_changes": [ { "address": "test_instance.test[0]",