core: Legacy SDK providers opt out of our new apply result check

The shim layer for the legacy SDK type system is not precise enough to
guarantee it will produce identical results between plan and apply. In
particular, values that are null during plan will often become zero-valued
during apply.

To avoid breaking those existing providers while still allowing us to
introduce this check in the future, we'll introduce a rather-hacky new
flag that allows the legacy SDK to signal that it is the legacy SDK and
thus disable the check.

Once we start phasing out the legacy SDK in favor of one that natively
understands our new type system, we can stop setting this flag and thus
get the additional safety of this check without breaking any
previously-released providers.

No other SDK is permitted to set this flag, and we will remove it if we
ever introduce protocol version 6 in future, assuming that any provider
supporting that protocol will always produce consistent results.

helper/plugin/grpc_provider.go

@@ -791,6 +791,15 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		return resp, nil
 	}
 	resp.Private = meta
+
+	// This is a signal to Terraform Core that we're doing the best we can to
+	// shim the legacy type system of the SDK onto the Terraform type system
+	// but we need it to cut us some slack. This setting should not be taken
+	// forward to any new SDK implementations, since setting it prevents us
+	// from catching certain classes of provider bug that can lead to
+	// confusing downstream errors.
+	resp.LegacyTypeSystem = true
+
 	return resp, nil
 }
 

internal/tfplugin5/tfplugin5.proto

@@ -254,6 +254,19 @@ message ApplyResourceChange {
         DynamicValue new_state = 1;
         bytes private = 2; 
         repeated Diagnostic diagnostics = 3;
+
+        // This may be set only by the helper/schema "SDK" in the main Terraform
+        // repository, to request that Terraform Core >=0.12 permit additional
+        // inconsistencies that can result from the legacy SDK type system
+        // and its imprecise mapping to the >=0.12 type system.
+        // The change in behavior implied by this flag makes sense only for the
+        // specific details of the legacy SDK type system, and are not a general
+        // mechanism to avoid proper type handling in providers.
+        //
+        //     ====              DO NOT USE THIS              ====
+        //     ==== THIS MUST BE LEFT UNSET IN ALL OTHER SDKS ====
+        //     ====              DO NOT USE THIS              ====
+        bool legacy_type_system = 4;
     }
 }
 

plugin/grpc_provider.go

@@ -453,6 +453,8 @@ func (p *GRPCProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques
 	}
 	resp.NewState = state
 
+	resp.LegacyTypeSystem = protoResp.LegacyTypeSystem
+
 	return resp
 }
 

providers/provider.go

@@ -261,6 +261,13 @@ type ApplyResourceChangeResponse struct {
 
 	// Diagnostics contains any warnings or errors from the method call.
 	Diagnostics tfdiags.Diagnostics
+
+	// LegacyTypeSystem is set only if the provider is using the legacy SDK
+	// whose type system cannot be precisely mapped into the Terraform type
+	// system. We use this to bypass certain consistency checks that would
+	// otherwise fail due to this imprecise mapping. No other provider or SDK
+	// implementation is permitted to set this.
+	LegacyTypeSystem bool
 }
 
 type ImportResourceStateRequest struct {

terraform/eval_apply.go

@@ -172,20 +172,35 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 		newVal = cty.UnknownAsNull(newVal)
 	}
 
-	// Only values that were marked as unknown in the planned value are allowed
+	if resp.LegacyTypeSystem {
-	// to change during the apply operation. (We do this after the unknown-ness
+		// The shimming of the old type system in the legacy SDK is not precise
-	// check above so that we also catch anything that became unknown after
+		// enough to pass this consistency check, so we'll give it a pass here,
-	// being known during plan.)
+		// but we will generate a warning about it so that we are more likely
-	if errs := objchange.AssertObjectCompatible(schema, change.After, newVal); len(errs) > 0 {
+		// to notice in the logs if an inconsistency beyond the type system
-		for _, err := range errs {
+		// leads to a downstream provider failure.
-			diags = diags.Append(tfdiags.Sourceless(
+		log.Printf("[WARN] Provider %s is using the legacy provider SDK, so we cannot check the result value for consistency with the plan; downstream errors about inconsistent plans may actually be caused by incorrect values from %s", n.ProviderAddr.ProviderConfig.Type, absAddr)
-				tfdiags.Error,
+		// The sort of inconsistency we won't catch here is if a known value
-				"Provider produced inconsistent result after apply",
+		// in the plan is changed during apply. That can cause downstream
-				fmt.Sprintf(
+		// problems because a dependent resource would make its own plan based
-					"When applying changes to %s, provider %q produced an unexpected new value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
+		// on the planned value, and thus get a different result during the
-					absAddr, n.ProviderAddr.ProviderConfig.Type, tfdiags.FormatError(err),
+		// apply phase. This will usually lead to a "Provider produced invalid plan"
-				),
+		// error that incorrectly blames the downstream resource for the change.
-			))
+	} else if change.Action != plans.Delete {
+		// Only values that were marked as unknown in the planned value are allowed
+		// to change during the apply operation. (We do this after the unknown-ness
+		// check above so that we also catch anything that became unknown after
+		// being known during plan.)
+		if errs := objchange.AssertObjectCompatible(schema, change.After, newVal); len(errs) > 0 {
+			for _, err := range errs {
+				diags = diags.Append(tfdiags.Sourceless(
+					tfdiags.Error,
+					"Provider produced inconsistent result after apply",
+					fmt.Sprintf(
+						"When applying changes to %s, provider %q produced an unexpected new value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
+						absAddr, n.ProviderAddr.ProviderConfig.Type, tfdiags.FormatError(err),
+					),
+				))
+			}
 		}
 	}