Enable PlanResourceChange (#4419)

Enable PlanResourceChange by default for every resource in the provider. This pulumi-terraform-bridge feature was incubated under a flag and deployed selectively or quite some time. It should be ready to become the new default. Improvements include prevention of panics and undesirable plans such as permanent diff cycling, as the flow is brought more in line with how TF operates the provider e.g. the expected behavior. --------- Co-authored-by: Venelin <[email protected]> Co-authored-by: VenelinMartinov <[email protected]>
pulumi · Sep 3, 2024 · 3ff4fa4 · 3ff4fa4
1 parent 290c8c3
commit 3ff4fa4
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 185 deletions.
diff --git a/examples/examples_go_test.go b/examples/examples_go_test.go
@@ -202,6 +202,13 @@ func (st tagsState) expectedTags() map[string]string {
 	return r
 }
 
+func normTags(tags map[string]string) map[string]string {
+	if tags == nil {
+		return map[string]string{}
+	}
+	return tags
+}
+
 type tagsFetcher = func() (map[string]string, error)
 
 func (st tagsState) assertTagsEqualWithRetry(
@@ -214,7 +221,7 @@ func (st tagsState) assertTagsEqualWithRetry(
 	for attempt := 0; attempt < 10; attempt++ {
 		var err error = nil
 		actualTags, err = getActualTags()
-		if err == nil && assert.ObjectsAreEqual(expectTags, actualTags) {
+		if err == nil && assert.ObjectsAreEqual(normTags(expectTags), normTags(actualTags)) {
 			break
 		}
 		if err != nil {
@@ -226,7 +233,7 @@ func (st tagsState) assertTagsEqualWithRetry(
 		time.Sleep(5 * time.Second)
 		t.Logf("Trying to fetch tags again, attempt %d", attempt+1)
 	}
-	require.Equalf(t, expectTags, actualTags, msg)
+	require.Equalf(t, normTags(expectTags), normTags(actualTags), msg)
 }
 
 func (st tagsState) validateStateResult(phase int) func(
@@ -247,7 +254,7 @@ func (st tagsState) validateStateResult(phase int) func(
 			require.NoError(t, err)
 			t.Logf("phase: %d", phase)
 			t.Logf("state: %v", st.serialize(t))
-			require.Equalf(t, st.expectedTags(), actualTags, "key=%s", k)
+			require.Equalf(t, normTags(st.expectedTags()), normTags(actualTags), "key=%s", k)
 			t.Logf("key=%s tags are as expected: %v", k, actualTagsJSON)
 
 			if k == "bucket" {

diff --git a/examples/examples_yaml_test.go b/examples/examples_yaml_test.go
@@ -380,9 +380,9 @@ outputs:
 // The first test ensures we don't regress on https://github.com/pulumi/pulumi-aws/issues/2682
 //
 // The second test is when upgrading from pulumi-aws version <5.0.0 to v6.x.x, and
-// prevents regressions on https://github.com/pulumi/pulumi-aws/issues/2823.
+// prevents regressions on https://github.com/pulumi/pulumi-aws/issues/2823
 //
-// Updated in https://github.com/pulumi/pulumi-aws/pull/3881 to accept CHANGES_SOME so long as they are not
+// Updated in https://github.com/pulumi/pulumi-aws/pull/3881
 // replacements.
 func TestMigrateRdsInstance(t *testing.T) {
 	case1 := `[{
@@ -533,143 +533,7 @@ func TestMigrateRdsInstance(t *testing.T) {
 		"__meta": "{\"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0\":{\"create\":2400000000000,\"delete\":3600000000000,\"update\":4800000000000},\"schema_version\":\"1\"}",
 		"address": "rds2f5ed54.c1xxca33i6kr.us-east-2.rds.amazonaws.com",
 		"allocatedStorage": 16,
-		"arn": "arn:aws:rds:us-east-2:616138583583:db:rds2f5ed54",
-		"autoMinorVersionUpgrade": true,
-		"availabilityZone": "us-east-2c",
-		"backupRetentionPeriod": 0,
-		"backupWindow": "07:34-08:04",
-		"caCertIdentifier": "rds-ca-2019",
-		"copyTagsToSnapshot": false,
-		"dbSubnetGroupName": "default",
-		"deleteAutomatedBackups": true,
-		"deletionProtection": false,
-		"domain": "",
-		"domainIamRoleName": "",
-		"enabledCloudwatchLogsExports": [],
-		"endpoint": "rds2f5ed54.c1xxca33i6kr.us-east-2.rds.amazonaws.com:3306",
-		"engine": "mysql",
-		"engineVersion": "8.0.33",
-		"hostedZoneId": "Z2XHWR1WZ565X2",
-		"iamDatabaseAuthenticationEnabled": false,
-		"id": "rds2f5ed54",
-		"identifier": "rds2f5ed54",
-		"instanceClass": "db.t3.micro",
-		"iops": 0,
-		"kmsKeyId": "",
-		"latestRestorableTime": "0001-01-01T00:00:00Z",
-		"licenseModel": "general-public-license",
-		"maintenanceWindow": "sun:03:59-sun:04:29",
-		"maxAllocatedStorage": 0,
-		"monitoringInterval": 0,
-		"monitoringRoleArn": "",
-		"multiAz": false,
-		"name": "name",
-		"optionGroupName": "default:mysql-8-0",
-		"parameterGroupName": "default.mysql8.0",
-		"password": "FOO-BAR-FIZZ1!2",
-		"performanceInsightsEnabled": false,
-		"performanceInsightsKmsKeyId": "",
-		"performanceInsightsRetentionPeriod": 0,
-		"port": 3306,
-		"publiclyAccessible": false,
-		"replicas": [],
-		"replicateSourceDb": "",
-		"resourceId": "db-N57SF65OZ5KO3TPK73R7DQMLZA",
-		"securityGroupNames": [],
-		"skipFinalSnapshot": true,
-		"status": "available",
-		"storageEncrypted": false,
-		"storageType": "gp2",
-		"tags": {
-		  "some": "change"
-		},
-		"timezone": "",
-		"username": "root",
-		"vpcSecurityGroupIds": [
-		  "sg-1928d262"
-		]
-	      },
-	      "news": {
-		"__defaults": [
-		  "applyImmediately",
-		  "autoMinorVersionUpgrade",
-		  "copyTagsToSnapshot",
-		  "deleteAutomatedBackups",
-		  "identifier",
-		  "monitoringInterval",
-		  "performanceInsightsEnabled",
-		  "publiclyAccessible"
-		],
-		"allocatedStorage": 16,
 		"applyImmediately": false,
-		"autoMinorVersionUpgrade": true,
-		"copyTagsToSnapshot": false,
-		"dbName": "name",
-		"deleteAutomatedBackups": true,
-		"engine": "mysql",
-		"identifier": "rds2f5ed54",
-		"instanceClass": "db.t3.micro",
-		"monitoringInterval": 0,
-		"password": "FOO-BAR-FIZZ1!2",
-		"performanceInsightsEnabled": false,
-		"publiclyAccessible": false,
-		"skipFinalSnapshot": true,
-		"tags": {
-		  "__defaults": [],
-		  "some": "change"
-		},
-		"username": "root"
-	      },
-	      "oldInputs": {
-		"__defaults": [
-		  "applyImmediately",
-		  "autoMinorVersionUpgrade",
-		  "copyTagsToSnapshot",
-		  "deleteAutomatedBackups",
-		  "identifier",
-		  "monitoringInterval",
-		  "performanceInsightsEnabled",
-		  "publiclyAccessible"
-		],
-		"allocatedStorage": 16,
-		"applyImmediately": false,
-		"autoMinorVersionUpgrade": true,
-		"copyTagsToSnapshot": false,
-		"deleteAutomatedBackups": true,
-		"engine": "mysql",
-		"identifier": "rds2f5ed54",
-		"instanceClass": "db.t3.micro",
-		"monitoringInterval": 0,
-		"name": "name",
-		"password": "FOO-BAR-FIZZ1!2",
-		"performanceInsightsEnabled": false,
-		"publiclyAccessible": false,
-		"skipFinalSnapshot": true,
-		"tags": {
-		  "__defaults": [],
-		  "some": "change"
-		},
-		"username": "root"
-	      }
-	    },
-	    "response": {
-	      "stables": "*",
-	      "changes": "*",
-	      "hasDetailedDiff": true
-	    }
-	  }
-	]`
-
-	// Like case2 but permits detailedDiff.
-	case2a := `[{
-	    "method": "/pulumirpc.ResourceProvider/Diff",
-	    "request": {
-	      "id": "rds2f5ed54",
-	      "urn": "urn:pulumi:exp2::secret-random-yaml::aws:rds/instance:Instance::rds",
-	      "olds": {
-		"__meta": "{\"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0\":{\"create\":2400000000000,\"delete\":3600000000000,\"update\":4800000000000},\"schema_version\":\"1\"}",
-		"address": "rds2f5ed54.c1xxca33i6kr.us-east-2.rds.amazonaws.com",
-		"allocatedStorage": 16,
 		"arn": "arn:aws:rds:us-east-2:616138583583:db:rds2f5ed54",
 		"autoMinorVersionUpgrade": true,
 		"availabilityZone": "us-east-2c",
@@ -790,20 +654,15 @@ func TestMigrateRdsInstance(t *testing.T) {
 	      }
 	    },
 	    "response": {
-              "diffs": "*",
 	      "stables": "*",
 	      "changes": "*",
-	      "hasDetailedDiff": true,
-              "detailedDiff": "*"
+	      "hasDetailedDiff": true
 	    }
 	  }
 	]`
 
 	t.Run("case1", func(t *testing.T) { replay(t, case1) })
 	t.Run("case2", func(t *testing.T) { replay(t, case2) })
-	t.Setenv("PULUMI_ENABLE_PLAN_RESOURCE_CHANGE", "true")
-	t.Run("case1-plan-resource-change", func(t *testing.T) { replay(t, case1) })
-	t.Run("case2-plan-resource-change", func(t *testing.T) { replay(t, case2a) })
 }
 
 func TestRegressUnknownTags(t *testing.T) {
@@ -1042,4 +901,4 @@ func TestSourceCodeHashImportedLambdaChecksCleanly(t *testing.T) {
         "name": "aws"
     }
 }]`)
-}
+}
diff --git a/patches/0066-Legacy-bucket-Read-now-sets-acl-and-force_destroy-de.patch b/patches/0066-Legacy-bucket-Read-now-sets-acl-and-force_destroy-de.patch
@@ -0,0 +1,58 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Anton Tayanovskyy <[email protected]>
+Date: Thu, 29 Aug 2024 13:17:47 -0400
+Subject: [PATCH] Legacy bucket Read now sets acl and force_destroy defaults
+
+
+diff --git a/internal/service/s3legacy/bucket_legacy.go b/internal/service/s3legacy/bucket_legacy.go
+index d5c03b22fb..8c26fee574 100644
+--- a/internal/service/s3legacy/bucket_legacy.go
++++ b/internal/service/s3legacy/bucket_legacy.go
+@@ -35,6 +35,10 @@ import (
+ 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
+ )
+
++const (
++	bucketACLDefaultValue = "private"
++)
++
+ func ResourceBucketLegacy() *schema.Resource {
+ 	return &schema.Resource{
+ 		CreateWithoutTimeout: resourceBucketLegacyCreate,
+@@ -80,7 +84,7 @@ func ResourceBucketLegacy() *schema.Resource {
+
+ 			"acl": {
+ 				Type:          schema.TypeString,
+-				Default:       "private",
++				Default:       bucketACLDefaultValue,
+ 				Optional:      true,
+ 				ConflictsWith: []string{"grant"},
+ 				ValidateFunc:  validation.StringInSlice(BucketCannedACL_Values(), false),
+@@ -943,7 +947,7 @@ func resourceBucketLegacyRead(ctx context.Context, d *schema.ResourceData, meta
+ 	}
+
+ 	//Read the Grant ACL. Reset if `acl` (canned ACL) is set.
+-	if acl, ok := d.GetOk("acl"); ok && acl.(string) != "private" {
++	if acl, ok := d.GetOk("acl"); ok && acl.(string) != bucketACLDefaultValue {
+ 		if err := d.Set("grant", nil); err != nil {
+ 			return diag.Errorf("error resetting grant %s", err)
+ 		}
+@@ -1403,6 +1407,18 @@ func resourceBucketLegacyRead(ctx context.Context, d *schema.ResourceData, meta
+ 	}.String()
+ 	d.Set("arn", arn)
+
++	// Set ACL default if unset, this fixes resource import option operation under Pulumi.
++	_, gotGrant := d.GetOk("grant")
++	_, gotACL := d.GetOk("acl")
++	if !gotGrant && !gotACL {
++		d.Set("acl", bucketACLDefaultValue)
++	}
++
++	// Similarly, set force_destroy to default value if unset, to fix resource import operation.
++	if _, fdSet := d.GetOk("force_destroy"); !fdSet {
++		d.Set("force_destroy", false)
++	}
++
+ 	return nil
+ }
+
diff --git a/provider/provider_test.go b/provider/provider_test.go
@@ -205,8 +205,8 @@ func pulumiUpWithSnapshot(t *testing.T, pulumiTest *pulumitest.PulumiTest) {
 	}
 	pulumiTest.Preview(optpreview.Plan(planFile))
 	upResult := pulumiTest.Up(optup.Plan(planFile))
-	fmt.Printf("stdout: %s \n", upResult.StdOut)
-	fmt.Printf("stderr: %s \n", upResult.StdErr)
+	t.Logf("stdout: %s \n", upResult.StdOut)
+	t.Logf("stderr: %s \n", upResult.StdErr)
 }
 
 func pulumiTest(t *testing.T, dir string, opts ...opttest.Option) *pulumitest.PulumiTest {

diff --git a/provider/provider_yaml_test.go b/provider/provider_yaml_test.go
@@ -371,13 +371,13 @@ resources:
 		})
 
 		res := pulumiTest.Preview()
-		fmt.Printf("stdout: %s \n", res.StdOut)
-		fmt.Printf("stderr: %s \n", res.StdErr)
+		t.Logf("stdout: %s \n", res.StdOut)
+		t.Logf("stderr: %s \n", res.StdErr)
 		assertpreview.HasNoChanges(t, res)
 
 		upResult := pulumiTest.Up()
-		fmt.Printf("stdout: %s \n", upResult.StdOut)
-		fmt.Printf("stderr: %s \n", upResult.StdErr)
+		t.Logf("stdout: %s \n", upResult.StdOut)
+		t.Logf("stderr: %s \n", upResult.StdErr)
 	})
 
 	// test that we can deploy a new filesystem with a list of subnetIds
@@ -548,7 +548,7 @@ type tagsTestStep struct {
 
 // TestAccDefaultTags tries to test all the scenarios that might affect provider defaultTags / resource tags
 // i.e. up, refresh, preview, import, etc
-func TestAccDefaultTags(t *testing.T) {
+func TestAccDefaultTagsWithImport(t *testing.T) {
 	t.Parallel()
 	if testing.Short() {
 		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without credentials")

diff --git a/provider/resources.go b/provider/resources.go
@@ -819,20 +819,8 @@ func ProviderFromMeta(metaInfo *tfbridge.MetadataInfo) *tfbridge.ProviderInfo {
 
 	v2p := shimv2.NewProvider(upstreamProvider.SDKV2Provider,
 		shimv2.WithDiffStrategy(shimv2.PlanState),
-		shimv2.WithPlanResourceChange(func(s string) bool {
-			switch s {
-			case "aws_ssm_document",
-				"aws_wafv2_web_acl",
-				"aws_wafv2_rule_group",
-				"aws_batch_job_definition",
-				"aws_lb_listener",
-				"aws_lb_listener_rule",
-				"aws_alb_listener",
-				"aws_alb_listener_rule":
-				return true
-			default:
-				return false
-			}
+		shimv2.WithPlanResourceChange(func(string) bool {
+			return true
 		}))
 
 	p := pftfbridge.MuxShimWithDisjointgPF(ctx, v2p, upstreamProvider.PluginFrameworkProvider)
@@ -2204,6 +2192,18 @@ compatibility shim in favor of the new "name" field.`)
 					"node_group_name": tfbridge.AutoName("nodeGroupName", 255, "-"),
 				},
 			},
+			"aws_eks_cluster": {
+				TransformFromState: func(_ context.Context, pm resource.PropertyMap) (resource.PropertyMap, error) {
+					// if the defaultOutboundAccessEnabled property is not set, set it to the default value of true
+					// this prevents an unnecessary replacement when upgrading the provider
+					// There is a TF migration which should handle this but due to [pulumi/pulumi-terraform-bridge#1667]
+					// it does not work as expected.
+					if _, ok := pm["bootstrapSelfManagedAddons"]; !ok {
+						pm["bootstrapSelfManagedAddons"] = resource.NewBoolProperty(true)
+					}
+					return pm, nil
+				},
+			},
 			"aws_eks_fargate_profile": {
 				Tok: awsResource(eksMod, "FargateProfile"),
 				Fields: map[string]*tfbridge.SchemaInfo{