Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade terraform-provider-aws to v5.82.1 #4957

Merged
merged 6 commits into from
Dec 20, 2024

Conversation

flostadler
Copy link
Contributor

@flostadler flostadler commented Dec 20, 2024

This PR was generated via $ upgrade-provider pulumi/pulumi-aws --kind=provider --target-version=5.82.1.

New resources:

apigateway/domainNameAccessAssociation.DomainNameAccessAssociation
cloudfront/vpcOrigin.VpcOrigin
memorydb/multiRegionCluster.MultiRegionCluster
networkmanager/dxGatewayAttachment.DxGatewayAttachment
rds/clusterSnapshotCopy.ClusterSnapshotCopy

New functions:

servicecatalog/getAppregistryAttributeGroupAssociations.getAppregistryAttributeGroupAssociations


Fixes #4760

@flostadler flostadler added the needs-release/minor When a PR with this label merges, it initiates a release of vX.Y+1.0 label Dec 20, 2024
@flostadler flostadler requested review from t0yv0, corymhall and a team December 20, 2024 08:59
@flostadler flostadler self-assigned this Dec 20, 2024
Copy link

github-actions bot commented Dec 20, 2024

Does the PR have any schema changes?

Found 2 breaking changes:

Types

  • 🟢 "aws:codeconnections/HostVpcConfiguration:HostVpcConfiguration": required: "tlsCertificate" property is no longer Required
  • 🟢 "aws:licensemanager/getReceivedLicenseEntitlement:getReceivedLicenseEntitlement": required: "overage" property has changed to Required

New resources:

  • apigateway/domainNameAccessAssociation.DomainNameAccessAssociation
  • cloudfront/vpcOrigin.VpcOrigin
  • memorydb/multiRegionCluster.MultiRegionCluster
  • networkmanager/dxGatewayAttachment.DxGatewayAttachment
  • rds/clusterSnapshotCopy.ClusterSnapshotCopy

New functions:

  • servicecatalog/getAppregistryAttributeGroupAssociations.getAppregistryAttributeGroupAssociations

Maintainer note: consult the runbook for dealing with any breaking changes.

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 48.05195% with 40 lines in your changes missing coverage. Please review.

Project coverage is 24.42%. Comparing base (f9da959) to head (4ae03c1).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
sdk/go/aws/rds/cluster.go 0.00% 11 Missing ⚠️
sdk/go/aws/autoscaling/group.go 0.00% 9 Missing ⚠️
sdk/go/aws/resourcegroups/resource.go 52.63% 9 Missing ⚠️
sdk/go/aws/directconnect/gateway.go 25.00% 6 Missing ⚠️
sdk/go/aws/memorydb/cluster.go 54.54% 5 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #4957    +/-   ##
========================================
  Coverage   24.41%   24.42%            
========================================
  Files         360      360            
  Lines      143270   143403   +133     
========================================
+ Hits        34984    35023    +39     
- Misses     108187   108281    +94     
  Partials       99       99            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@flostadler
Copy link
Contributor Author

"aws:elasticache/replicationGroup:ReplicationGroup":
🟡 inputs: "atRestEncryptionEnabled" type changed from "boolean" to "string"
🟡 properties: "atRestEncryptionEnabled" type changed from "boolean" to "string"

That seems bad at a first glance. Checking why that happened

@flostadler
Copy link
Contributor Author

hashicorp/terraform-provider-aws#40514 changed atRestEncryptionEnabled of aws:elasticache/replicationGroup:ReplicationGroup to a nullable bool which we model as a string in the bridge (pulumi/pulumi-terraform-bridge#1514).

The ReplicationGroup is a rather widely used resource and atRestEncryptionEnabled is a setting almost everybody running Redis/Valkey in production would set. This is a rather severe breaking change that we cannot ship in a minor IMO

@flostadler
Copy link
Contributor Author

flostadler commented Dec 20, 2024

regress-4457 is failing because a new property was added to autoscaling.Group. That test is executing pulumi import and then editing the code to add the necessary changes. The problem is that our python tests use the latest release version of the SDK and that does not include newly added properties, but pulumi import will include them in the generated code. Chicken/Egg problem...

We should rewrite that test to use nodejs (or yaml?) because they are not susceptible to this issue. Skipping for now, opened issue: #4958

@flostadler
Copy link
Contributor Author

Added override for ReplicationGroup atRestEncryptionEnabled to be a boolean and added a test for it

@@ -317,3 +324,32 @@ func planEqual(t *testing.T, firstPlan, secondPlan string) bool {

return assert.Equal(t, firstPlanData, secondPlanData)
}

func inPlacePulumiTest(t *testing.T, dir string, opts ...opttest.Option) *pulumitest.PulumiTest {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us now to use an in-process provider for the tests. Which will also give us code coverage :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good but might want to be careful to not use this for production release verification in case there's accidentally some behavioral difference between the prod binary and this form of running the provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, before adopting this for all tests we should add some sort of flag/env variable to fall back to attaching the binary for releases

@@ -179,6 +179,7 @@ func TestRegress2534(t *testing.T) {
}

func TestRegress4457(t *testing.T) {
t.Skipf("TODO[pulumi/pulumi-aws#4957]")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's happening here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait, just noticed I linked the PR and not the issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected the link, thanks for noticing!

replicationGroupArn: ${replicationGroup.arn}
resources:
replicationGroup:
type: aws:elasticache:ReplicationGroup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@t0yv0
Copy link
Member

t0yv0 commented Dec 20, 2024

I think it fixes #4760 too

@t0yv0
Copy link
Member

t0yv0 commented Dec 20, 2024

@flostadler flostadler enabled auto-merge (squash) December 20, 2024 15:24
@flostadler flostadler merged commit 8517f88 into master Dec 20, 2024
26 checks passed
@flostadler flostadler deleted the upgrade-terraform-provider-aws-to-v5.82.1 branch December 20, 2024 16:43
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v6.66.0.

@github-actions github-actions bot removed the needs-release/minor When a PR with this label merges, it initiates a release of vX.Y+1.0 label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade terraform-provider-aws to v5.82.1 SQS RedriveAllowPolicy shows spurious JSON diff
3 participants