From 96b099cf352ad2d29a04bab71fce8910d78f033d Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 4 Nov 2020 11:03:12 -0500
Subject: [PATCH] allow nil config attributes in ignore_changes

The validation for ignore_changes was too broad, and makes it difficult
to ignore changes in an attribute that the user does not want to set.
While the goal of ignore_changes is to prevent changes in the
configuration alone, we don't intend to break the use-case of ignoring
drift from the provider. Since we cannot easily narrow the validation to
only detect computed attributes at the moment, we can drop this error
altogether for now.
---
 terraform/context_validate_test.go | 44 ------------------------------
 terraform/eval_refresh.go          |  2 ++
 terraform/eval_validate.go         | 35 ++++--------------------
 3 files changed, 7 insertions(+), 74 deletions(-)

diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go
index e35803464..f9c22d921 100644
--- a/terraform/context_validate_test.go
+++ b/terraform/context_validate_test.go
@@ -1752,50 +1752,6 @@ output "out" {
 	}
 }
 
-func TestContext2Validate_invalidIgnoreChanges(t *testing.T) {
-	// validate module and output depends_on
-	m := testModuleInline(t, map[string]string{
-		"main.tf": `
-resource "test_instance" "a" {
-  lifecycle {
-    ignore_changes = [foo]
-  }
-}
-
-`,
-	})
-
-	p := testProvider("test")
-	p.GetSchemaReturn = &ProviderSchema{
-		ResourceTypes: map[string]*configschema.Block{
-			"test_instance": {
-				Attributes: map[string]*configschema.Attribute{
-					"id":  {Type: cty.String, Computed: true},
-					"foo": {Type: cty.String, Computed: true, Optional: true},
-				},
-			},
-		},
-	}
-
-	ctx := testContext2(t, &ContextOpts{
-		Config: m,
-		Providers: map[addrs.Provider]providers.Factory{
-			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
-		},
-	})
-	diags := ctx.Validate()
-	if !diags.HasErrors() {
-		t.Fatal("succeeded; want errors")
-	}
-
-	for _, d := range diags {
-		des := d.Description().Summary
-		if !strings.Contains(des, "Cannot ignore") {
-			t.Fatalf(`expected "Invalid depends_on reference", got %q`, des)
-		}
-	}
-}
-
 func TestContext2Validate_rpcDiagnostics(t *testing.T) {
 	// validate module and output depends_on
 	m := testModuleInline(t, map[string]string{
diff --git a/terraform/eval_refresh.go b/terraform/eval_refresh.go
index d69bedd64..0b5481031 100644
--- a/terraform/eval_refresh.go
+++ b/terraform/eval_refresh.go
@@ -127,6 +127,8 @@ func (n *EvalRefresh) Eval(ctx EvalContext) tfdiags.Diagnostics {
 
 	// We have no way to exempt provider using the legacy SDK from this check,
 	// so we can only log inconsistencies with the updated state values.
+	// In most cases these are not errors anyway, and represent "drift" from
+	// external changes which will be handled by the subsequent plan.
 	if errs := objchange.AssertObjectCompatible(schema, priorVal, resp.NewState); len(errs) > 0 {
 		var buf strings.Builder
 		fmt.Fprintf(&buf, "[WARN] Provider %q produced an unexpected new value for %s during refresh.", n.ProviderAddr.Provider.String(), absAddr)
diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go
index 288cc62a2..182f4d1f9 100644
--- a/terraform/eval_validate.go
+++ b/terraform/eval_validate.go
@@ -362,39 +362,14 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error {
 
 		if cfg.Managed != nil { // can be nil only in tests with poorly-configured mocks
 			for _, traversal := range cfg.Managed.IgnoreChanges {
-				// This will error out if the traversal contains an invalid
-				// index step. That is OK if we want users to be able to ignore
-				// a key that is no longer specified in the config.
+				// validate the ignore_changes traversals apply.
 				moreDiags := schema.StaticValidateTraversal(traversal)
 				diags = diags.Append(moreDiags)
-				if diags.HasErrors() {
-					continue
-				}
 
-				// first check to see if this assigned in the config
-				v, _ := traversal.TraverseRel(configVal)
-				if !v.IsNull() {
-					// it's assigned, so we can also assume it's not computed-only
-					continue
-				}
-
-				// We can't ignore changes that don't exist in the configuration.
-				// We're not checking specifically if the traversal resolves to
-				// a computed-only value, but we can hint to the user that it
-				// might also be the case.
-				sourceRange := traversal.SourceRange()
-				diags = diags.Append(&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Cannot ignore argument not set in the configuration",
-					Detail: fmt.Sprintf("The ignore_changes argument is not set in the configuration.\n" +
-						"The ignore_changes mechanism only applies to changes " +
-						"within the configuration, and must be used with " +
-						"arguments set in the configuration and not computed by " +
-						"the provider.",
-					),
-					Subject: &sourceRange,
-				})
-				return diags.Err()
+				// TODO: we want to notify users that they can't use
+				// ignore_changes for computed attributes, but we don't have an
+				// easy way to correlate the config value, schema and
+				// traversal together.
 			}
 		}