Remove InstanceStateStrategy (#1745)

A [search](https://github.com/search?q=org%3Apulumi%20InstanceStateStrategy&type=code) shows that `InstanceStateStrategy` isn't being used, so we can safely remove it. This simplifies our code.
pulumi · Mar 11, 2024 · 22fdb29 · 22fdb29
1 parent 19acefe
commit 22fdb29
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 321 deletions.
diff --git a/pkg/tests/regress_aws_1423_test.go b/pkg/tests/regress_aws_1423_test.go
@@ -19,8 +19,8 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-
 	testutils "github.com/pulumi/providertest/replay"
+
 	webaclschema "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tests/internal/webaclschema"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
 	shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
@@ -38,7 +38,11 @@ func TestRegressAws1423(t *testing.T) {
 		},
 	}
 
-	p := shimv2.NewProvider(tfProvider, shimv2.WithDiffStrategy(shimv2.PlanState))
+	p := shimv2.NewProvider(tfProvider,
+		shimv2.WithDiffStrategy(shimv2.PlanState),
+		shimv2.WithPlanResourceChange(func(s string) bool {
+			return s == "aws_wafv2_web_acl"
+		}))
 
 	info := tfbridge.ProviderInfo{
 		P:           p,
@@ -63,8 +67,6 @@ func TestRegressAws1423(t *testing.T) {
 		[]byte{}, /* pulumiSchema */
 	)
 
-	shimv2.SetInstanceStateStrategy(p.ResourcesMap().Get("aws_wafv2_web_acl"), shimv2.CtyInstanceState)
-
 	testCase1 := `
 	{
 	  "method": "/pulumirpc.ResourceProvider/Create",
@@ -254,189 +256,9 @@ func TestRegressAws1423(t *testing.T) {
 	}
         `
 
-	testCase2 := `
-	{
-	  "method": "/pulumirpc.ResourceProvider/Diff",
-	  "request": {
-	    "id": "6ff5298f-7bcd-4e73-ba75-8784c2c77bcb",
-	    "urn": "urn:pulumi:dev::aws-2264::aws:wafv2/webAcl:WebAcl::my-web-acl-2",
-	    "olds": {
-	      "arn": "arn:aws:wafv2:us-east-1:616138583583:regional/webacl/my-web-acl-2/6ff5298f-7bcd-4e73-ba75-8784c2c77bcb",
-	      "associationConfig": null,
-	      "capacity": 1,
-	      "captchaConfig": null,
-	      "customResponseBodies": [],
-	      "defaultAction": {
-		"allow": null,
-		"block": {
-		  "customResponse": null
-		}
-	      },
-	      "description": "",
-	      "id": "6ff5298f-7bcd-4e73-ba75-8784c2c77bcb",
-	      "lockToken": "a58a7783-ac87-4ba9-8801-324ae5c63a07",
-	      "name": "my-web-acl-2",
-	      "rules": [
-		{
-		  "action": {
-		    "allow": {
-		      "customRequestHandling": null
-		    },
-		    "block": null,
-		    "captcha": null,
-		    "challenge": null,
-		    "count": null
-		  },
-		  "captchaConfig": null,
-		  "name": "US-access-only",
-		  "overrideAction": null,
-		  "priority": 0,
-		  "ruleLabels": [],
-		  "statement": {
-		    "andStatement": null,
-		    "byteMatchStatement": null,
-		    "geoMatchStatement": {
-		      "countryCodes": [
-			"US"
-		      ],
-		      "forwardedIpConfig": null
-		    },
-		    "ipSetReferenceStatement": null,
-		    "labelMatchStatement": null,
-		    "managedRuleGroupStatement": null,
-		    "notStatement": null,
-		    "orStatement": null,
-		    "rateBasedStatement": null,
-		    "regexMatchStatement": null,
-		    "regexPatternSetReferenceStatement": null,
-		    "ruleGroupReferenceStatement": null,
-		    "sizeConstraintStatement": null,
-		    "sqliMatchStatement": null,
-		    "xssMatchStatement": null
-		  },
-		  "visibilityConfig": {
-		    "cloudwatchMetricsEnabled": true,
-		    "metricName": "US-access-only",
-		    "sampledRequestsEnabled": true
-		  }
-		}
-	      ],
-	      "scope": "REGIONAL",
-	      "tags": {},
-	      "tagsAll": {},
-	      "tokenDomains": [],
-	      "visibilityConfig": {
-		"cloudwatchMetricsEnabled": true,
-		"metricName": "my-web-acl-2",
-		"sampledRequestsEnabled": true
-	      }
-	    },
-	    "news": {
-	      "__defaults": [],
-	      "defaultAction": {
-		"__defaults": [],
-		"block": {
-		  "__defaults": []
-		}
-	      },
-	      "name": "my-web-acl-2",
-	      "rules": [
-		{
-		  "__defaults": [],
-		  "action": {
-		    "__defaults": [],
-		    "allow": {
-		      "__defaults": []
-		    }
-		  },
-		  "name": "US-access-only",
-		  "priority": 0,
-		  "statement": {
-		    "__defaults": [],
-		    "geoMatchStatement": {
-		      "__defaults": [],
-		      "countryCodes": [
-			"US"
-		      ]
-		    }
-		  },
-		  "visibilityConfig": {
-		    "__defaults": [],
-		    "cloudwatchMetricsEnabled": true,
-		    "metricName": "US-access-only",
-		    "sampledRequestsEnabled": true
-		  }
-		}
-	      ],
-	      "scope": "REGIONAL",
-	      "visibilityConfig": {
-		"__defaults": [],
-		"cloudwatchMetricsEnabled": true,
-		"metricName": "my-web-acl-2",
-		"sampledRequestsEnabled": true
-	      }
-	    },
-	    "oldInputs": {
-	      "__defaults": [],
-	      "defaultAction": {
-		"__defaults": [],
-		"block": {
-		  "__defaults": []
-		}
-	      },
-	      "name": "my-web-acl-2",
-	      "rules": [
-		{
-		  "__defaults": [],
-		  "action": {
-		    "__defaults": [],
-		    "allow": {
-		      "__defaults": []
-		    }
-		  },
-		  "name": "US-access-only",
-		  "priority": 0,
-		  "statement": {
-		    "__defaults": [],
-		    "geoMatchStatement": {
-		      "__defaults": [],
-		      "countryCodes": [
-			"US"
-		      ]
-		    }
-		  },
-		  "visibilityConfig": {
-		    "__defaults": [],
-		    "cloudwatchMetricsEnabled": true,
-		    "metricName": "US-access-only",
-		    "sampledRequestsEnabled": true
-		  }
-		}
-	      ],
-	      "scope": "REGIONAL",
-	      "visibilityConfig": {
-		"__defaults": [],
-		"cloudwatchMetricsEnabled": true,
-		"metricName": "my-web-acl-2",
-		"sampledRequestsEnabled": true
-	      }
-	    }
-	  },
-	  "response": {
-	    "stables": "*",
-	    "changes": "DIFF_NONE",
-	    "hasDetailedDiff": true
-	  }
-	}`
-
 	t.Run("testCase2/createPreview", func(t *testing.T) {
-		// This is wrong; this test case is from a preview after an up wihtout edits, it should not detect
+		// This is wrong; this test case is from a preview after an up without edits, it should not detect
 		// diffs.
 		testutils.Replay(t, server, testCase2CreatePreview)
 	})
-	t.Run("testCase2/diff", func(t *testing.T) {
-		// This is wrong; this test case is from a preview after an up wihtout edits, it should not detect
-		// diffs.
-		testutils.Replay(t, server, testCase2)
-	})
 }
diff --git a/pkg/tfshim/sdk-v2/instance_state.go b/pkg/tfshim/sdk-v2/instance_state.go
@@ -63,31 +63,9 @@ func (s v2InstanceState) ID() string {
 }
 
 func (s v2InstanceState) Object(sch shim.SchemaMap) (map[string]interface{}, error) {
-	strategy := GetInstanceStateStrategy(v2Resource{s.resource})
-	if strategy == CtyInstanceState {
-		return s.objectViaCty(sch)
-	}
 	return s.objectV1(sch)
 }
 
-// This version of Object uses TF built-ins AttrsAsObjectValue and schema.ApplyDiff with cty.Value intermediate form.
-func (s v2InstanceState) objectViaCty(sch shim.SchemaMap) (map[string]interface{}, error) {
-	rSchema := s.resource.CoreConfigSchema()
-	ty := rSchema.ImpliedType()
-
-	// Read InstanceState as a cty.Value.
-	v, err := s.tf.AttrsAsObjectValue(ty)
-	contract.AssertNoErrorf(err, "AttrsAsObjectValue failed")
-
-	// If there is a diff, apply it.
-	if s.diff != nil {
-		v, err = schema.ApplyDiff(v, s.diff, rSchema)
-		contract.AssertNoErrorf(err, "schema.ApplyDiff failed")
-	}
-
-	return objectFromCtyValue(v), nil
-}
-
 func objectFromCtyValue(v cty.Value) map[string]interface{} {
 	// Now we need to translate cty.Value to a JSON-like form. This could have been avoided if surrounding Pulumi
 	// code accepted a cty.Value and translated that to resource.PropertyValue, but that is currently not the case.

diff --git a/pkg/tfshim/sdk-v2/instance_state_read_strategy.go b/pkg/tfshim/sdk-v2/instance_state_read_strategy.go
diff --git a/pkg/tfshim/sdk-v2/upgrade_state.go b/pkg/tfshim/sdk-v2/upgrade_state.go
@@ -67,13 +67,8 @@ func upgradeResourceState(ctx context.Context, p *schema.Provider, res *schema.R
 		return nil, err
 	}
 
-	// Opt-out of normalization under a flag. Normalization replaces nulls with empty blocks for the state value,
-	// but when processing config the codebase calls NewResourceConfigShimmed which in turn calls
-	// ConfigValueFromHCL2Block which removes empty blocks. This leads to non-empty diffs.
-	if GetInstanceStateStrategy(v2Resource{res}) != CtyInstanceState {
-		// Normalize the value and fill in any missing blocks.
-		v = schema.NormalizeObjectFromLegacySDK(v, configBlock)
-	}
+	// Normalize the value and fill in any missing blocks.
+	v = schema.NormalizeObjectFromLegacySDK(v, configBlock)
 
 	// Convert the value back to an InstanceState.
 	newState, err := res.ShimInstanceStateFromValue(v)