fix!: unable to create fsx for OpenZfs Multi-AZ file system (#4095)

This PR removes the `MaxItemsOne` config for the `subnetIds` property.
Originally this was a `MaxItemsOne` property in upstream, but it was
removed when Multi-AZ support was added by AWS. In Terraform removing
`MaxItemsOne` is not a breaking change, but for Pulumi it is.

Because it looks like there is very low usage of this resource, and the
change enables a (maybe) more common use case (Multi-AZ filesystem) we
have decided to take the breaking change now.

In order to upgrade users will only need to update their code so that
`subnetIds` is now a list. For example,

From
```ts
const test = new aws.fsx.OpenZfsFileSystem("test", {
    storageCapacity: 64,
    subnetIds: test1.id,
    deploymentType: "SINGLE_AZ_1",
    throughputCapacity: 64,
});
```

To
```ts
const test = new aws.fsx.OpenZfsFileSystem("test", {
    storageCapacity: 64,
    subnetIds: [test1.id],
    deploymentType: "SINGLE_AZ_1",
    throughputCapacity: 64,
});
```

Because we are including the `TransformFromState` function, users should
not need to make any changes to the state themselves.

BREAKING CHANGE: `fsx.OpenZfsFileSystem.subnetIds` now accepts a list
instead of a string

closes #3106, closes #3034

provider/cmd/pulumi-resource-aws/bridge-metadata.json

@@ -12167,7 +12167,7 @@
                         "maxItemsOne": false
                     },
                     "subnet_ids": {
-                        "maxItemsOne": true
+                        "maxItemsOne": false
                     }
                 }
             },
@@ -230633,11 +230633,6 @@
                     "certificate_authority": true
                 }
             },
-            "aws_fsx_openzfs_file_system": {
-                "maxItemsOneOverrides": {
-                    "subnet_ids": true
-                }
-            },
             "aws_lexv2models_slot": {
                 "maxItemsOneOverrides": {
                     "value_elicitation_setting.$.prompt_specification.$.message_group.$.message.$.custom_payload": false,

provider/provider_test.go

@@ -2,6 +2,8 @@
 package provider
 
 import (
+	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -11,7 +13,12 @@ import (
 	"github.com/pulumi/providertest/optproviderupgrade"
 	"github.com/pulumi/providertest/pulumitest"
 	"github.com/pulumi/providertest/pulumitest/assertpreview"
+	"github.com/pulumi/providertest/pulumitest/optnewstack"
 	"github.com/pulumi/providertest/pulumitest/opttest"
+	"github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview"
+	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -82,6 +89,107 @@ func testProviderUpgrade(t *testing.T, dir string, opts *testProviderUpgradeOpti
 	assertpreview.HasNoReplacements(t, result)
 }
 
+type testProviderCodeChangesOptions struct {
+	region               string
+	firstProgram         []byte
+	secondProgram        []byte
+	firstProgramOptions  []opttest.Option
+	secondProgramOptions []opttest.Option
+}
+
+// testProviderCodeChanges tests two different runs of a pulumi program. This allow you to run
+// pulumi up with an initial program, change the code of the program and then run another pulumi command
+func testProviderCodeChanges(t *testing.T, opts *testProviderCodeChangesOptions) *pulumitest.PulumiTest {
+	skipIfShort(t)
+	t.Parallel()
+	t.Helper()
+
+	workdir := t.TempDir()
+	cacheDir := filepath.Join("testdata", "recorded", "TestProviderUpgrade", t.Name())
+	err := os.MkdirAll(cacheDir, 0755)
+	assert.NoError(t, err)
+	stackExportFile := filepath.Join(cacheDir, "stack.json")
+
+	err = os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), opts.firstProgram, 0o600)
+	require.NoError(t, err)
+
+	options := []opttest.Option{
+		opttest.SkipInstall(),
+		opttest.NewStackOptions(optnewstack.DisableAutoDestroy()),
+	}
+	for _, o := range opts.firstProgramOptions {
+		options = append(options, o)
+	}
+
+	pt := pulumitest.NewPulumiTest(t, workdir, options...)
+	region := "us-east-2"
+	if opts != nil && opts.region != "" {
+		region = opts.region
+	}
+	pt.SetConfig("aws:region", region)
+
+	var export *apitype.UntypedDeployment
+	export, err = tryReadStackExport(stackExportFile)
+	if err != nil {
+		pt.Up()
+		grptLog := pt.GrpcLog()
+		grpcLogPath := filepath.Join(cacheDir, "grpc.json")
+		pt.T().Logf("writing grpc log to %s", grpcLogPath)
+		grptLog.WriteTo(grpcLogPath)
+
+		e := pt.ExportStack()
+		export = &e
+		err = writeStackExport(stackExportFile, export, true)
+		assert.NoError(t, err)
+	}
+
+	secondOptions := []opttest.Option{
+		opttest.SkipInstall(),
+		opttest.NewStackOptions(optnewstack.EnableAutoDestroy()),
+	}
+	for _, o := range opts.secondProgramOptions {
+		secondOptions = append(secondOptions, o)
+	}
+	err = os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), opts.secondProgram, 0o600)
+	require.NoError(t, err)
+	secondTest := pulumitest.NewPulumiTest(t, workdir, secondOptions...)
+	secondTest.SetConfig("aws:region", region)
+	secondTest.ImportStack(*export)
+
+	return secondTest
+}
+
+// pulumiUpWithSnapshot will only run the up portion of the test if the plan has changed since the
+// last time the test was run.
+//
+// This should be used when the plan is a good representation of what you are testing. Sometimes
+// there are issues where the plan is consistent, but the apply fails. In those cases a snapshot test is not
+// a good fit.
+func pulumiUpWithSnapshot(t *testing.T, pulumiTest *pulumitest.PulumiTest) {
+	workdir := os.TempDir()
+	cwd, err := os.Getwd()
+	assert.NoError(t, err)
+	cacheDir := filepath.Join(cwd, "testdata", "recorded", "TestProviderUpgrade", t.Name())
+	planFile := filepath.Join(cacheDir, "plan.json")
+	if ok, _ := exists(planFile); ok {
+		err := os.MkdirAll(cacheDir, 0755)
+		assert.NoError(t, err)
+		tmpPlanFile := filepath.Join(workdir, "plan.json")
+
+		pulumiTest.Preview(optpreview.Plan(tmpPlanFile))
+
+		if equal := planEqual(t, planFile, tmpPlanFile); equal {
+			return
+		}
+
+		pulumiTest.T().Log("Plan is not equal, re-running up")
+	}
+	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)
+}
+
 func pulumiTest(t *testing.T, dir string, opts ...opttest.Option) *pulumitest.PulumiTest {
 	if testing.Short() {
 		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without AWS creds")
@@ -110,3 +218,95 @@ func maxDuration(dur time.Duration, t *testing.T, test func(t *testing.T)) {
 	case <-done:
 	}
 }
+
+// writeStackExport writes the stack export to the given path creating any directories needed.
+func writeStackExport(path string, snapshot *apitype.UntypedDeployment, overwrite bool) error {
+	if snapshot == nil {
+		return fmt.Errorf("stack export must not be nil")
+	}
+	dir := filepath.Dir(path)
+	err := os.MkdirAll(dir, 0755)
+	if err != nil {
+		return err
+	}
+	stackBytes, err := json.MarshalIndent(snapshot, "", "  ")
+	if err != nil {
+		return err
+	}
+	pathExists, err := exists(path)
+	if err != nil {
+		return err
+	}
+	if pathExists && !overwrite {
+		return fmt.Errorf("stack export already exists at %s", path)
+	}
+	return os.WriteFile(path, stackBytes, 0644)
+}
+
+// tryReadStackExport reads a stack export from the given file path.
+// If the file does not exist, returns nil, nil.
+func tryReadStackExport(path string) (*apitype.UntypedDeployment, error) {
+	stackBytes, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read stack export at %s: %v", path, err)
+	}
+	var stackExport apitype.UntypedDeployment
+	err = json.Unmarshal(stackBytes, &stackExport)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal stack export at %s: %v", path, err)
+	}
+	return &stackExport, nil
+}
+
+func exists(filePath string) (bool, error) {
+	_, err := os.Stat(filePath)
+	switch {
+	case err == nil:
+		return true, nil
+	case !os.IsNotExist(err):
+		return false, err
+	}
+	return false, nil
+}
+
+// cleanPlan removes fields that change every time, i.e. 'time' or 'seed'
+func cleanPlan(t *testing.T, plan map[string]interface{}) map[string]interface{} {
+	t.Helper()
+	if val, exists := plan["resourcePlans"]; exists {
+		resourcePlans := val.(map[string]interface{})
+		for _, v := range resourcePlans {
+			resourcePlan := v.(map[string]interface{})
+			delete(resourcePlan, "seed")
+		}
+	}
+
+	if val, exists := plan["manifest"]; exists {
+		manifest := val.(map[string]interface{})
+		delete(manifest, "time")
+	}
+
+	return plan
+}
+
+func readPlan(t *testing.T, planPath string) map[string]interface{} {
+	t.Helper()
+	var firstData map[string]interface{}
+
+	firstFile, err := os.ReadFile(planPath)
+	assert.NoError(t, err)
+
+	err = json.Unmarshal(firstFile, &firstData)
+	assert.NoError(t, err)
+	firstData = cleanPlan(t, firstData)
+
+	return firstData
+}
+
+// planEqual asserts that two plans are equal
+func planEqual(t *testing.T, firstPlan, secondPlan string) bool {
+	t.Helper()
+	firstPlanData := readPlan(t, firstPlan)
+	secondPlanData := readPlan(t, secondPlan)
+
+	return assert.Equal(t, firstPlanData, secondPlanData)
+}

provider/provider_yaml_test.go

@@ -305,6 +305,83 @@ func TestNonIdempotentSnsTopic(t *testing.T) {
 	require.ErrorContains(t, err, "already exists")
 }
 
+func TestOpenZfsFileSystemUpgrade(t *testing.T) {
+	if testing.Short() {
+		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without credentials")
+	}
+	const pulumiYaml = `
+name: openzfs
+runtime: yaml
+resources:
+  MyFileSystem:
+    properties:
+      deploymentType: SINGLE_AZ_1
+      storageCapacity: 64
+      %s
+      throughputCapacity: 64
+    type: aws:fsx:OpenZfsFileSystem
+  MySubnet:
+    properties:
+      cidrBlock: "10.0.1.0/24"
+      vpcId: ${MyVPC.id}
+    type: aws:ec2:Subnet
+  MyVPC:
+    properties:
+      cidrBlock: "10.0.0.0/16"
+    type: aws:ec2:Vpc
+`
+
+	var (
+		providerName    string = "aws"
+		baselineVersion string = "6.41.0"
+	)
+	cwd, err := os.Getwd()
+	assert.NoError(t, err)
+	workdir := t.TempDir()
+
+	firstProgram := []byte(fmt.Sprintf(pulumiYaml, "subnetIds: ${MySubnet.id}"))
+	secondProgram := []byte(fmt.Sprintf(pulumiYaml, "subnetIds:\n        - ${MySubnet.id}"))
+	// test that we can upgrade from the previous version which accepted a string for `subnetIds`
+	// to the new version which accepts a list
+	t.Run("upgrade", func(t *testing.T) {
+		pulumiTest := testProviderCodeChanges(t, &testProviderCodeChangesOptions{
+			firstProgram: firstProgram,
+			firstProgramOptions: []opttest.Option{
+				opttest.DownloadProviderVersion(providerName, baselineVersion),
+			},
+			secondProgram: secondProgram,
+			secondProgramOptions: []opttest.Option{
+				opttest.LocalProviderPath("aws", filepath.Join(cwd, "..", "bin")),
+			},
+		})
+
+		res := pulumiTest.Preview()
+		fmt.Printf("stdout: %s \n", res.StdOut)
+		fmt.Printf("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)
+	})
+
+	// test that we can deploy a new filesystem with a list of subnetIds
+	// we use a test with a snapshot since this test is only useful the first time, once
+	// we know it works it should continue to work.
+	t.Run("new-version", func(t *testing.T) {
+		err = os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), secondProgram, 0o600)
+		assert.NoError(t, err)
+		pulumiTest := pulumitest.NewPulumiTest(t, workdir,
+			opttest.SkipInstall(),
+			opttest.LocalProviderPath("aws", filepath.Join(cwd, "..", "bin")),
+		)
+
+		pulumiTest.SetConfig("aws:region", "us-east-2")
+
+		pulumiUpWithSnapshot(t, pulumiTest)
+	})
+}
+
 // Make sure that legacy Bucket supports deleting tags out of band and detecting drift.
 func TestRegress3674(t *testing.T) {
 	ptest := pulumiTest(t, filepath.Join("test-programs", "regress-3674"), opttest.SkipInstall())

provider/resources.go

@@ -36,6 +36,7 @@ import (
 
 	pftfbridge "github.com/pulumi/pulumi-terraform-bridge/pf/tfbridge"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
+	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
 	tks "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/tokens"
 	shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
 	shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
@@ -2279,10 +2280,25 @@ compatibility shim in favor of the new "name" field.`)
 			"aws_fsx_ontap_file_system":             {Tok: awsResource(fsxMod, "OntapFileSystem")},
 			"aws_fsx_ontap_storage_virtual_machine": {Tok: awsResource(fsxMod, "OntapStorageVirtualMachine")},
 			"aws_fsx_ontap_volume":                  {Tok: awsResource(fsxMod, "OntapVolume")},
-			"aws_fsx_openzfs_file_system":           {Tok: awsResource(fsxMod, "OpenZfsFileSystem")},
-			"aws_fsx_openzfs_snapshot":              {Tok: awsResource(fsxMod, "OpenZfsSnapshot")},
-			"aws_fsx_openzfs_volume":                {Tok: awsResource(fsxMod, "OpenZfsVolume")},
-			"aws_fsx_windows_file_system":           {Tok: awsResource(fsxMod, "WindowsFileSystem")},
+			"aws_fsx_openzfs_file_system": {
+				Tok: awsResource(fsxMod, "OpenZfsFileSystem"),
+				TransformFromState: func(ctx context.Context, state resource.PropertyMap) (resource.PropertyMap, error) {
+					if val, ok := state["subnetIds"]; ok {
+						if val.IsString() {
+							state["subnetIds"] = resource.NewArrayProperty([]resource.PropertyValue{val})
+						}
+					}
+					return state, nil
+				},
+				Fields: map[string]*info.Schema{
+					"subnet_ids": {
+						MaxItemsOne: tfbridge.False(),
+					},
+				},
+			},
+			"aws_fsx_openzfs_snapshot":    {Tok: awsResource(fsxMod, "OpenZfsSnapshot")},
+			"aws_fsx_openzfs_volume":      {Tok: awsResource(fsxMod, "OpenZfsVolume")},
+			"aws_fsx_windows_file_system": {Tok: awsResource(fsxMod, "WindowsFileSystem")},
 
 			// GameLift