Merge pull request #26810 from hashicorp/jbardin/validate-ignore-changes

Allow null attributes to be referenced in ignore_changes

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{

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)

terraform/eval_validate.go

@@ -252,10 +252,6 @@ type EvalValidateResource struct {
 	Config         *configs.Resource
 	ProviderMetas  map[addrs.Provider]*configs.ProviderMeta
 
-	// IgnoreWarnings means that warnings will not be passed through. This allows
-	// "just-in-time" passes of validation to continue execution through warnings.
-	IgnoreWarnings bool
-
 	// ConfigVal, if non-nil, will be updated with the value resulting from
 	// evaluating the given configuration body. Since validation is performed
 	// very early, this value is likely to contain lots of unknown values,
@@ -264,12 +260,14 @@ type EvalValidateResource struct {
 	ConfigVal *cty.Value
 }
 
-func (n *EvalValidateResource) Validate(ctx EvalContext) error {
+func (n *EvalValidateResource) Validate(ctx EvalContext) tfdiags.Diagnostics {
-	if n.ProviderSchema == nil || *n.ProviderSchema == nil {
+	var diags tfdiags.Diagnostics
-		return fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr)
+
+	if *n.ProviderSchema == nil {
+		diags = diags.Append(fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr))
+		return diags
 	}
 
-	var diags tfdiags.Diagnostics
 	provider := *n.Provider
 	cfg := *n.Config
 	schema := *n.ProviderSchema
@@ -351,50 +349,25 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error {
 				Detail:   fmt.Sprintf("The provider %s does not support resource type %q.", cfg.ProviderConfigAddr(), cfg.Type),
 				Subject:  &cfg.TypeRange,
 			})
-			return diags.Err()
+			return diags
 		}
 
 		configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData)
 		diags = diags.Append(valDiags)
 		if valDiags.HasErrors() {
-			return diags.Err()
+			return diags
 		}
 
 		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
+				// validate the ignore_changes traversals apply.
-				// index step. That is OK if we want users to be able to ignore
-				// a key that is no longer specified in the config.
 				moreDiags := schema.StaticValidateTraversal(traversal)
 				diags = diags.Append(moreDiags)
-				if diags.HasErrors() {
-					continue
-				}
 
-				// first check to see if this assigned in the config
+				// TODO: we want to notify users that they can't use
-				v, _ := traversal.TraverseRel(configVal)
+				// ignore_changes for computed attributes, but we don't have an
-				if !v.IsNull() {
+				// easy way to correlate the config value, schema and
-					// it's assigned, so we can also assume it's not computed-only
+				// traversal together.
-					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()
 			}
 		}
 
@@ -419,13 +392,13 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error {
 				Detail:   fmt.Sprintf("The provider %s does not support data source %q.", cfg.ProviderConfigAddr(), cfg.Type),
 				Subject:  &cfg.TypeRange,
 			})
-			return diags.Err()
+			return diags
 		}
 
 		configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData)
 		diags = diags.Append(valDiags)
 		if valDiags.HasErrors() {
-			return diags.Err()
+			return diags
 		}
 
 		req := providers.ValidateDataSourceConfigRequest{
@@ -437,17 +410,7 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error {
 		diags = diags.Append(resp.Diagnostics.InConfigBody(cfg.Config))
 	}
 
-	if n.IgnoreWarnings {
+	return diags
-		// If we _only_ have warnings then we'll return nil.
-		if diags.HasErrors() {
-			return diags.NonFatalErr()
-		}
-		return nil
-	} else {
-		// We'll return an error if there are any diagnostics at all, even if
-		// some of them are warnings.
-		return diags.NonFatalErr()
-	}
 }
 
 func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expression) tfdiags.Diagnostics {

terraform/eval_validate_test.go

@@ -238,45 +238,6 @@ func TestEvalValidateResource_warningsAndErrorsPassedThrough(t *testing.T) {
 	}
 }
 
-func TestEvalValidateResource_ignoreWarnings(t *testing.T) {
-	mp := simpleMockProvider()
-	mp.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) providers.ValidateResourceTypeConfigResponse {
-		var diags tfdiags.Diagnostics
-		diags = diags.Append(tfdiags.SimpleWarning("warn"))
-		return providers.ValidateResourceTypeConfigResponse{
-			Diagnostics: diags,
-		}
-	}
-
-	p := providers.Interface(mp)
-	rc := &configs.Resource{
-		Mode:   addrs.ManagedResourceMode,
-		Type:   "test_object",
-		Name:   "foo",
-		Config: configs.SynthBody("", map[string]cty.Value{}),
-	}
-	node := &EvalValidateResource{
-		Addr: addrs.Resource{
-			Mode: addrs.ManagedResourceMode,
-			Type: "test-object",
-			Name: "foo",
-		},
-		Provider:       &p,
-		Config:         rc,
-		ProviderSchema: &mp.GetSchemaReturn,
-
-		IgnoreWarnings: true,
-	}
-
-	ctx := &MockEvalContext{}
-	ctx.installSimpleEval()
-
-	err := node.Validate(ctx)
-	if err != nil {
-		t.Fatalf("Expected no error, got: %s", err)
-	}
-}
-
 func TestEvalValidateResource_invalidDependsOn(t *testing.T) {
 	mp := simpleMockProvider()
 	mp.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) providers.ValidateResourceTypeConfigResponse {
@@ -323,9 +284,9 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) {
 	ctx := &MockEvalContext{}
 	ctx.installSimpleEval()
 
-	err := node.Validate(ctx)
+	diags := node.Validate(ctx)
-	if err != nil {
+	if diags.HasErrors() {
-		t.Fatalf("error for supposedly-valid config: %s", err)
+		t.Fatalf("error for supposedly-valid config: %s", diags.ErrWithWarnings())
 	}
 
 	// Now we'll make it invalid by adding additional traversal steps at
@@ -344,11 +305,11 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) {
 		},
 	})
 
-	err = node.Validate(ctx)
+	diags = node.Validate(ctx)
-	if err == nil {
+	if !diags.HasErrors() {
 		t.Fatal("no error for invalid depends_on")
 	}
-	if got, want := err.Error(), "Invalid depends_on reference"; !strings.Contains(got, want) {
+	if got, want := diags.Err().Error(), "Invalid depends_on reference"; !strings.Contains(got, want) {
 		t.Fatalf("wrong error\ngot:  %s\nwant: Message containing %q", got, want)
 	}
 
@@ -360,11 +321,11 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) {
 		},
 	})
 
-	err = node.Validate(ctx)
+	diags = node.Validate(ctx)
-	if err == nil {
+	if !diags.HasErrors() {
 		t.Fatal("no error for invalid depends_on")
 	}
-	if got, want := err.Error(), "Invalid depends_on reference"; !strings.Contains(got, want) {
+	if got, want := diags.Err().Error(), "Invalid depends_on reference"; !strings.Contains(got, want) {
 		t.Fatalf("wrong error\ngot:  %s\nwant: Message containing %q", got, want)
 	}
 }