Skip to content

Commit

Permalink
Add pluralized lifecycle_policies to EFS file system data source (#4590)
Browse files Browse the repository at this point in the history
The `lifecycle_policy` output of the `efs.getFileSystem` data source has
`MaxItemsOne` hardcoded on the Pulumi side and this triggers panics when
there is more than one lifecycle policy on the resource.

By adding a pluralized lifecycle_policies output and fixing the singular
version to only return at most one element we're able to fix this panic
without introducing breaking changes. To incentivize users to move to
the pluralized version I've marked the singular version as deprecated.

During the next major release we can drop the patch (I'll create a
tracking ticket once this PR goes in) and the auto-aliasing should
correctly drop `MaxItemsOne` as well. At that point we're tracking
upstream behavior again.

Fixes #4568
  • Loading branch information
flostadler authored Oct 2, 2024
1 parent d80f7d1 commit 9a752fd
Show file tree
Hide file tree
Showing 16 changed files with 274 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Florian Stadler <[email protected]>
Date: Tue, 1 Oct 2024 20:39:29 +0200
Subject: [PATCH] Add pluralized lifecycle_policies to EFS file system data
source

The lifecycle_policy attribute has MaxItemsOne hardcoded on the
Pulumi side and this triggers panics when there is more than
one lifecycle policy on the resource.
By adding pluralized lifecycle_policies and fixing the singular
version to only return at most one element we're able to fix this
panic without introducing breaking changes.

diff --git a/internal/service/efs/file_system_data_source.go b/internal/service/efs/file_system_data_source.go
index f045aa36a4..a7b710fe9b 100644
--- a/internal/service/efs/file_system_data_source.go
+++ b/internal/service/efs/file_system_data_source.go
@@ -63,6 +63,28 @@ func dataSourceFileSystem() *schema.Resource {
Computed: true,
},
"lifecycle_policy": {
+ Type: schema.TypeList,
+ Computed: true,
+ Deprecated: "Use `lifecycle_policies` instead. This field will be removed in the next major version.",
+ Elem: &schema.Resource{
+ Schema: map[string]*schema.Schema{
+ "transition_to_archive": {
+ Type: schema.TypeString,
+ Computed: true,
+ },
+ "transition_to_ia": {
+ Type: schema.TypeString,
+ Computed: true,
+ },
+ "transition_to_primary_storage_class": {
+ Type: schema.TypeString,
+ Computed: true,
+ },
+ },
+ },
+ },
+ // duplicating lifecycle_policy to drop the hard coded MaxItemsOne on the Pulumi side without a breaking change
+ "lifecycle_policies": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
@@ -179,9 +201,20 @@ func dataSourceFileSystemRead(ctx context.Context, d *schema.ResourceData, meta
return sdkdiag.AppendErrorf(diags, "reading EFS File System (%s) lifecycle configuration: %s", d.Id(), err)
}

- if err := d.Set("lifecycle_policy", flattenLifecyclePolicies(output.LifecyclePolicies)); err != nil {
+ flattenedLifecyclePolicies := flattenLifecyclePolicies(output.LifecyclePolicies)
+
+ // if there are any lifecycle policies then set the first one as the singular version
+ var singularLifeCyclePolicy []interface{}
+ if len(flattenedLifecyclePolicies) > 0 {
+ singularLifeCyclePolicy = append(singularLifeCyclePolicy, flattenedLifecyclePolicies[0])
+ }
+
+ if err := d.Set("lifecycle_policy", singularLifeCyclePolicy); err != nil {
return sdkdiag.AppendErrorf(diags, "setting lifecycle_policy: %s", err)
}
+ if err := d.Set("lifecycle_policies", flattenedLifecyclePolicies); err != nil {
+ return sdkdiag.AppendErrorf(diags, "setting lifecycle_policies: %s", err)
+ }

return diags
}
8 changes: 8 additions & 0 deletions provider/cmd/pulumi-resource-aws/bridge-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -226653,6 +226653,9 @@
"current": "aws:efs/getFileSystem:getFileSystem",
"majorVersion": 6,
"fields": {
"lifecycle_policies": {
"maxItemsOne": false
},
"lifecycle_policy": {
"maxItemsOne": true
},
Expand Down Expand Up @@ -275023,6 +275026,11 @@
}
},
"datasources": {
"aws_efs_file_system": {
"maxItemsOneOverrides": {
"lifecycle_policy": true
}
},
"aws_quicksight_analysis": {
"renames": [
"aws:quicksight/getAnalysis:getAnalysis"
Expand Down

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions provider/cmd/pulumi-resource-aws/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -394253,8 +394253,15 @@
"description": "ARN for the KMS encryption key.\n",
"type": "string"
},
"lifecyclePolicies": {
"items": {
"$ref": "#/types/aws:efs/getFileSystemLifecyclePolicy:getFileSystemLifecyclePolicy"
},
"type": "array"
},
"lifecyclePolicy": {
"$ref": "#/types/aws:efs/getFileSystemLifecyclePolicy:getFileSystemLifecyclePolicy",
"deprecationMessage": "Use `lifecycle_policies` instead. This field will be removed in the next major version.",
"description": "File system [lifecycle policy](https://docs.aws.amazon.com/efs/latest/ug/API_LifecyclePolicy.html) object.\n"
},
"name": {
Expand Down Expand Up @@ -394300,6 +394307,7 @@
"encrypted",
"fileSystemId",
"kmsKeyId",
"lifecyclePolicies",
"lifecyclePolicy",
"name",
"performanceMode",
Expand Down
38 changes: 38 additions & 0 deletions provider/provider_nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/pulumi/pulumi-aws/provider/v6/pkg/elb"
"github.com/pulumi/pulumi/pkg/v3/testing/integration"
"github.com/pulumi/pulumi/sdk/v3/go/auto/optpreview"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -275,6 +276,43 @@ func TestRegress4446(t *testing.T) {
t.Logf("#%v", result.ChangeSummary)
}

func TestRegress4568(t *testing.T) {
skipIfShort(t)
dir := filepath.Join("test-programs", "regress-4568")
cwd, err := os.Getwd()
require.NoError(t, err)
providerName := "aws"
options := []opttest.Option{
opttest.LocalProviderPath(providerName, filepath.Join(cwd, "..", "bin")),
opttest.YarnLink("@pulumi/aws"),
}
test := pulumitest.NewPulumiTest(t, dir, options...)
upResult := test.Up()
t.Logf("#%v", upResult.Summary)

// The singular lifecyclePolicy should contain the first value
assert.Equal(t, map[string]interface{}{
"transitionToIa": "AFTER_30_DAYS",
"transitionToArchive": "",
"transitionToPrimaryStorageClass": "",
}, upResult.Outputs["lifecyclePolicy"].Value, "lifecyclePolicy should be set")

// The plural lifecyclePolicies should contain both values
lifecyclePolicies := upResult.Outputs["lifecyclePolicies"].Value.([]interface{})
assert.Len(t, lifecyclePolicies, 2, "lifecyclePolicies should have two elements")

assert.Contains(t, lifecyclePolicies, map[string]interface{}{
"transitionToIa": "AFTER_30_DAYS",
"transitionToArchive": "",
"transitionToPrimaryStorageClass": "",
})
assert.Contains(t, lifecyclePolicies, map[string]interface{}{
"transitionToPrimaryStorageClass": "AFTER_1_ACCESS",
"transitionToIa": "",
"transitionToArchive": "",
})
}

func getJSBaseOptions(t *testing.T) integration.ProgramTestOptions {
envRegion := getEnvRegion(t)
baseJS := integration.ProgramTestOptions{
Expand Down
12 changes: 1 addition & 11 deletions provider/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -4702,17 +4702,7 @@ compatibility shim in favor of the new "name" field.`)
"aws_ecs_task_execution": {Tok: awsDataSource(ecsMod, "getTaskExecution")},

// Elastic Filesystem
"aws_efs_file_system": {
Tok: awsDataSource(efsMod, "getFileSystem"),
Fields: map[string]*tfbridge.SchemaInfo{
// Removing `MaxItems: 1`, Seems to be a hedge
// against the future, but does not become present
// in the TF API. See
// https://github.com/hashicorp/terraform-provider-aws/commit/362c03ec27e839a571de312060e87657d617038b
// for details.
"lifecycle_policy": {MaxItemsOne: ref(true)},
},
},
"aws_efs_file_system": {Tok: awsDataSource(efsMod, "getFileSystem")},
"aws_efs_mount_target": {Tok: awsDataSource(efsMod, "getMountTarget")},
"aws_efs_access_point": {Tok: awsDataSource(efsMod, "getAccessPoint")},
"aws_efs_access_points": {Tok: awsDataSource(efsMod, "getAccessPoints")},
Expand Down
7 changes: 7 additions & 0 deletions provider/test-programs/regress-4568/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name: regress-4568
runtime: nodejs
description: A minimal AWS TypeScript Pulumi program
config:
pulumi:tags:
value:
pulumi:template: aws-typescript
12 changes: 12 additions & 0 deletions provider/test-programs/regress-4568/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as aws from "@pulumi/aws";

const efsFilesystem = new aws.efs.FileSystem("my-efs", {
lifecyclePolicies: [{
transitionToIa: "AFTER_30_DAYS",
}, {
transitionToPrimaryStorageClass: "AFTER_1_ACCESS",
}],
});

export const lifecyclePolicy = aws.efs.getFileSystemOutput({ fileSystemId: efsFilesystem.id }).lifecyclePolicy;
export const lifecyclePolicies = aws.efs.getFileSystemOutput({ fileSystemId: efsFilesystem.id }).lifecyclePolicies;
11 changes: 11 additions & 0 deletions provider/test-programs/regress-4568/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "regress-4568",
"main": "index.ts",
"devDependencies": {
"@types/node": "^18"
},
"dependencies": {
"@pulumi/pulumi": "^3.0.0",
"@pulumi/aws": "^6.0.0"
}
}
18 changes: 18 additions & 0 deletions provider/test-programs/regress-4568/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"compilerOptions": {
"strict": true,
"outDir": "bin",
"target": "es2016",
"module": "commonjs",
"moduleResolution": "node",
"sourceMap": true,
"experimentalDecorators": true,
"pretty": true,
"noFallthroughCasesInSwitch": true,
"noImplicitReturns": true,
"forceConsistentCasingInFileNames": true
},
"files": [
"index.ts"
]
}
4 changes: 4 additions & 0 deletions sdk/dotnet/Efs/GetFileSystem.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion sdk/go/aws/efs/getFileSystem.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions sdk/go/aws/efs/pulumiTypes.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 9a752fd

Please sign in to comment.