Relax resource identity validation (#36989)

* Remove identity change validation

* Remove validation for null checks in identities

Author: dbanck

internal/terraform/context_plan_identity_test.go

@@ -153,6 +153,7 @@ func TestContext2Plan_resource_identity_refresh(t *testing.T) {
 			ExpectedError:                       fmt.Errorf("failed to upgrade resource identity: provider was unable to do so"),
 		},
 		"identity sent to provider differs from returned one": {
+			// We don't throw an error here, because there are resource types with mutable identities
 			StoredIdentitySchemaVersion: 0,
 			StoredIdentityJSON:          []byte(`{"id": "foo"}`),
 			IdentitySchema: providers.IdentitySchema{
@@ -171,9 +172,8 @@ func TestContext2Plan_resource_identity_refresh(t *testing.T) {
 				"id": cty.StringVal("bar"),
 			}),
 			ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{
-				"id": cty.StringVal("foo"),
+				"id": cty.StringVal("bar"),
 			}),
-			ExpectedError: fmt.Errorf("Provider produced different identity: Provider \"registry.terraform.io/hashicorp/aws\" returned a different identity for aws_instance.web than the previously stored one. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
 		},
 		"identity with unknowns": {
 			IdentitySchema: providers.IdentitySchema{
@@ -446,12 +446,13 @@ func TestContext2Plan_resource_identity_plan(t *testing.T) {
 			},
 		},
 		"create - null planned identity schema": {
+			// We allow null values in identities
 			plannedIdentity: cty.ObjectVal(map[string]cty.Value{
 				"id": cty.NullVal(cty.String),
 			}),
-			expectDiagnostics: tfdiags.Diagnostics{
-				tfdiags.Sourceless(tfdiags.Error, "Provider produced an identity that doesn't match the schema", "Provider \"registry.terraform.io/hashicorp/test\" returned an identity for test_resource.test that doesn't match the identity schema: attributes \"id\" are required and must not be null. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
-			},
+			expectedIdentity: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.NullVal(cty.String),
+			}),
 		},
 		"update": {
 			prevRunState: states.BuildState(func(s *states.SyncState) {
@@ -536,6 +537,7 @@ func TestContext2Plan_resource_identity_plan(t *testing.T) {
 		},
 
 		"update - changing identity": {
+			// We don't throw an error here, because there are resource types with mutable identities
 			prevRunState: states.BuildState(func(s *states.SyncState) {
 				s.SetResourceInstanceCurrent(
 					addrs.Resource{
@@ -559,9 +561,9 @@ func TestContext2Plan_resource_identity_plan(t *testing.T) {
 				"id": cty.StringVal("foo"),
 			}),
 
-			expectDiagnostics: tfdiags.Diagnostics{
-				tfdiags.Sourceless(tfdiags.Error, "Provider produced different identity", "Provider \"registry.terraform.io/hashicorp/test\" returned a different identity for test_resource.test than the previously stored one. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."),
-			},
+			expectedIdentity: cty.ObjectVal(map[string]cty.Value{
+				"id": cty.StringVal("foo"),
+			}),
 		},
 
 		"update - updating identity schema version": {

internal/terraform/node_resource_abstract_instance.go

@@ -671,7 +671,6 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state
 		if !resp.Identity.IsNull() {
 			diags = diags.Append(n.validateIdentityKnown(resp.Identity))
 			diags = diags.Append(n.validateIdentity(resp.Identity, schema.Identity))
-			diags = diags.Append(n.validateIdentityDidNotChange(state, resp.Identity))
 		}
 		if resp.Deferred != nil {
 			deferred = resp.Deferred
@@ -1122,10 +1121,6 @@ func (n *NodeAbstractResourceInstance) plan(
 	if !plannedIdentity.IsNull() {
 		if !action.IsReplace() && action != plans.Create {
 			diags = diags.Append(n.validateIdentityKnown(plannedIdentity))
-			// If the identity is not known we can not validate it did not change
-			if !diags.HasErrors() {
-				diags = diags.Append(n.validateIdentityDidNotChange(currentState, plannedIdentity))
-			}
 		}
 
 		diags = diags.Append(n.validateIdentity(plannedIdentity, schema.Identity))
@@ -2648,9 +2643,6 @@ func (n *NodeAbstractResourceInstance) apply(
 		if !resp.NewIdentity.IsNull() {
 			diags = diags.Append(n.validateIdentityKnown(resp.NewIdentity))
 			diags = diags.Append(n.validateIdentity(resp.NewIdentity, schema.Identity))
-			if !change.Action.IsReplace() {
-				diags = diags.Append(n.validateIdentityDidNotChange(state, resp.NewIdentity))
-			}
 		}
 	}
 	applyDiags := resp.Diagnostics
@@ -2916,21 +2908,6 @@ func (n *NodeAbstractResourceInstance) validateIdentityKnown(newIdentity cty.Val
 	return diags
 }
 
-func (n *NodeAbstractResourceInstance) validateIdentityDidNotChange(state *states.ResourceInstanceObject, newIdentity cty.Value) (diags tfdiags.Diagnostics) {
-	if state != nil && !state.Identity.IsNull() && state.Identity.Equals(newIdentity).False() {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Provider produced different identity",
-			fmt.Sprintf(
-				"Provider %q returned a different identity for %s than the previously stored one. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
-				n.ResolvedProvider.Provider, n.Addr,
-			),
-		))
-	}
-
-	return diags
-}
-
 func (n *NodeAbstractResourceInstance) validateIdentity(newIdentity cty.Value, identitySchema *configschema.Object) (diags tfdiags.Diagnostics) {
 	if _, marks := newIdentity.UnmarkDeep(); len(marks) > 0 {
 		diags = diags.Append(tfdiags.Sourceless(
@@ -2972,25 +2949,6 @@ func (n *NodeAbstractResourceInstance) validateIdentity(newIdentity cty.Value, i
 		return diags
 	}
 
-	// Check for required attributes
-	names := make([]string, 0, len(identitySchema.Attributes))
-	for name, attrS := range identitySchema.Attributes {
-		if attrS.Required && newIdentity.GetAttr(name).IsNull() {
-			names = append(names, name)
-		}
-	}
-	if len(names) > 0 {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Provider produced an identity that doesn't match the schema",
-			fmt.Sprintf(
-				"Provider %q returned an identity for %s that doesn't match the identity schema: attributes %q are required and must not be null. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
-				n.ResolvedProvider.Provider, n.Addr,
-				strings.Join(names, ", "),
-			),
-		))
-	}
-
 	return diags
 }