-
Notifications
You must be signed in to change notification settings - Fork 155
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
upstream v5.49.0 #3936
upstream v5.49.0 #3936
Conversation
a6afd9a
to
9c328c6
Compare
Does the PR have any schema changes?Found 12 breaking changes: Functions
Types
New resources:
New functions:
Maintainer note: consult the runbook for dealing with any breaking changes. |
The breaking changes related to the The previous TF schema had mismatches with the underlying AWS API schema; rendering this datasource almost unusable. Here's an excerpt from the PR:
I'd argue that those are not breaking changes in reality because the data source wasn't really usable ("Data source state is not persisted at all"...) |
--- a/internal/service/s3/bucket_object.go | ||
+++ b/internal/service/s3/bucket_object.go | ||
@@ -68,7 +68,7 @@ func resourceBucketObject() *schema.Resource { | ||
Computed: true, | ||
}, | ||
"bucket": { | ||
names.AttrBucket: { | ||
- Deprecated: "Use the aws_s3_object resource instead", | ||
+ // FORK: Stack72 removed the deprecation warning as it was misleading for Pulumi users |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good find, relevant to some S3 revisiting work that I'm doing, going to log that.
--- a/names/data/names_data.csv | ||
+++ b/names/data/names_data.csv | ||
@@ -218,7 +218,7 @@ kinesis-video-media,kinesisvideomedia,kinesisvideomedia,kinesisvideomedia,,kines | ||
kinesis-video-signaling,kinesisvideosignaling,kinesisvideosignalingchannels,kinesisvideosignaling,,kinesisvideosignaling,,kinesisvideosignalingchannels,KinesisVideoSignaling,KinesisVideoSignalingChannels,,1,,,aws_kinesisvideosignaling_,,kinesisvideosignaling_,Kinesis Video Signaling,Amazon,,x,,,,,Kinesis Video Signaling,,, | ||
kms,kms,kms,kms,,kms,,,KMS,KMS,,1,,,aws_kms_,,kms_,KMS (Key Management),AWS,,,,,,,KMS,ListKeys,, | ||
kms,kms,kms,kms,,kms,,,KMS,KMS,,,2,,aws_kms_,,kms_,KMS (Key Management),AWS,,,,,,,KMS,ListKeys,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSV diff is a bit hard to read, did this change OK? This CSV drives some code-generator but we don't fully check that changes to the CSV keep the generated code consistent. I recently had to make some quick edits in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was changed upstream here: hashicorp/terraform-provider-aws@8a2d87d
How would I confirm whether this had any negative effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some digging and the data in the CSV follows the format documented here: https://github.com/hashicorp/terraform-provider-aws/blob/main/names/README.md?plain=1#L34
What changed here was index 12, which indicates whether AWS SDKv2 is used or not. If SDK v2 is used, the value needs to be set to 2
. And that value was set to 2 now because they switched KMS over to use SDKv2: https://github.com/hashicorp/terraform-provider-aws/blame/main/internal/service/kms/service_package_gen.go#L132
So this change is fine.
}, | ||
names.AttrID: framework.IDAttribute(), | ||
names.AttrTags: tftags.TagsAttribute(), | ||
- names.AttrTagsAll: tftags.TagsAttributeComputedOnly(), | ||
+ names.AttrTagsAll: tftags.TagsAttribute(), | ||
}, | ||
Blocks: map[string]schema.Block{ | ||
"source": schema.ListNestedBlock{ | ||
"source": schema.SetNestedBlock{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious is this new? Is it Set or List? I thought this patch is only interested in patching up tftags.TagsAttribute()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was recently changed in upstream: https://github.com/hashicorp/terraform-provider-aws/blame/main/internal/service/securitylake/subscriber.go#L105
It's not part of the changes in this patch, just the surrounding code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got confused by the placement of +/-
in the diff display. Looked like a change in the patch.
provider/resources.go
Outdated
@@ -108,6 +108,7 @@ const ( | |||
dataexchangeMod = "DataExchange" // Data exchange | |||
datapipelineMod = "DataPipeline" // Data Pipeline | |||
datasyncMod = "DataSync" // DataSync | |||
datazoneMod = "DataZone" // DataZone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this gofmt'd right? Looks a bit odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 Didn't commit after go fmt'ing
The rest of the breaking changes are bug fixes, so a minor upgrade on our end should be good here: aws:securitylake/SubscriberNotificationConfigurationHttpsNotificationConfiguration:SubscriberNotificationConfigurationHttpsNotificationConfiguration": required: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got some questions on patch edits to confirm but generally LGTM! 🙇 thanks for taking this, a fair bit of work that is necessary.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | minor | [`6.35.0` -> `6.36.0`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.35.0/6.36.0) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.36.0`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.36.0) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.35.0...v6.36.0) ##### What's Changed - upstream v5.49.0 by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3936](https://togithub.com/pulumi/pulumi-aws/pull/3936) - fix: sns topic creation fails in non-standard partitions by [@​corymhall](https://togithub.com/corymhall) in [https://github.com/pulumi/pulumi-aws/pull/3932](https://togithub.com/pulumi/pulumi-aws/pull/3932) - fix: wafv2 permanent diff issues ([#​3880](https://togithub.com/pulumi/pulumi-aws/issues/3880) [#​3306](https://togithub.com/pulumi/pulumi-aws/issues/3306) [#​3190](https://togithub.com/pulumi/pulumi-aws/issues/3190) [#​3454](https://togithub.com/pulumi/pulumi-aws/issues/3454)) - Enroll aws_wafv2\_rule_group in PlanResourceChange by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3948](https://togithub.com/pulumi/pulumi-aws/pull/3948) - Upgrade pulumi-terraform-bridge to v3.82.0 by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3929](https://togithub.com/pulumi/pulumi-aws/pull/3929) - Dependencies: terraform converter v1.0.17 by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/3923](https://togithub.com/pulumi/pulumi-aws/pull/3923) **Full Changelog**: pulumi/pulumi-aws@v6.35.0...v6.36.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjMuOSIsInVwZGF0ZWRJblZlciI6IjM3LjM2My45IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@pulumi/aws](https://pulumi.io) ([source](https://togithub.com/pulumi/pulumi-aws)) | dependencies | minor | [`6.35.0` -> `6.36.0`](https://renovatebot.com/diffs/npm/@pulumi%2faws/6.35.0/6.36.0) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>pulumi/pulumi-aws (@​pulumi/aws)</summary> ### [`v6.36.0`](https://togithub.com/pulumi/pulumi-aws/releases/tag/v6.36.0) [Compare Source](https://togithub.com/pulumi/pulumi-aws/compare/v6.35.0...v6.36.0) ##### What's Changed - upstream v5.49.0 by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3936](https://togithub.com/pulumi/pulumi-aws/pull/3936) - fix: sns topic creation fails in non-standard partitions by [@​corymhall](https://togithub.com/corymhall) in [https://github.com/pulumi/pulumi-aws/pull/3932](https://togithub.com/pulumi/pulumi-aws/pull/3932) - fix: wafv2 permanent diff issues ([#​3880](https://togithub.com/pulumi/pulumi-aws/issues/3880) [#​3306](https://togithub.com/pulumi/pulumi-aws/issues/3306) [#​3190](https://togithub.com/pulumi/pulumi-aws/issues/3190) [#​3454](https://togithub.com/pulumi/pulumi-aws/issues/3454)) - Enroll aws_wafv2\_rule_group in PlanResourceChange by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3948](https://togithub.com/pulumi/pulumi-aws/pull/3948) - Upgrade pulumi-terraform-bridge to v3.82.0 by [@​flostadler](https://togithub.com/flostadler) in [https://github.com/pulumi/pulumi-aws/pull/3929](https://togithub.com/pulumi/pulumi-aws/pull/3929) - Dependencies: terraform converter v1.0.17 by [@​t0yv0](https://togithub.com/t0yv0) in [https://github.com/pulumi/pulumi-aws/pull/3923](https://togithub.com/pulumi/pulumi-aws/pull/3923) **Full Changelog**: pulumi/pulumi-aws@v6.35.0...v6.36.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjMuOSIsInVwZGF0ZWRJblZlciI6IjM3LjM2My45IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21pbm9yIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
This includes quite a bit of changes to the patches because upstream added new constants for some names (e.g. description, arn, etc.) and replaced magic strings with those.
It also adds a new AWS service (DataZone) to the provider
Closes #3919