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

feat(aws): bump aws-sdk-go to v1.44.174 #1686

Merged
merged 1 commit into from
Mar 6, 2023
Merged

feat(aws): bump aws-sdk-go to v1.44.174 #1686

merged 1 commit into from
Mar 6, 2023

Conversation

AlexLast
Copy link
Contributor

@AlexLast AlexLast commented Mar 1, 2023

Description of your changes

We need to make use of a new RDS DBInstance parameter that is in a later version of the AWS SDK, specifically being able to set CACertificateIdentifier. This pull request bumps aws-sdk-go to version v1.44.174, the version where this parameter was introduced.

I've added all new resources to ignore.resource_names so they can be introduced more carefully, however let me know if this isn't the correct approach.

Also needed to slightly refactor how tagging works in cloudwatchlogs/loggroup/setup.go to work around some deprecation warnings.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Yet to do any testing, however I'll be running a build locally to test out some resources and ensure things are working as expected.

  • Run a local build and ensured the following resources that we use were working as expected: S3.Bucket, S3.BucketPolicy, IAM.Role, IAM.Policy, IAM.RolePolicyAttachment, EC2.SecurityGroup, EC2.SecurityGroupRule, RDS.DBSubnetGroup, RDS.DBCluster, RDS.DBClusterParameterGroup, RDS.DBInstance, Kinesis.Stream, DynamoDB.Table, KMS.Key.
  • Ensured refactored tagging for CloudwatchLogs.LogGroup is working as expected.

@haarchri
Copy link
Member

haarchri commented Mar 3, 2023

thanks for driving this - overall looks good
lets implement first #1689 then we have a check for breaking changes in the crds ;)

@AlexLast
Copy link
Contributor Author

AlexLast commented Mar 3, 2023

@haarchri Sure, let me know when it's merged and I'll rebase

@haarchri
Copy link
Member

haarchri commented Mar 6, 2023

done! @AlexLast can you rebase ? thanks ;)

@AlexLast
Copy link
Contributor Author

AlexLast commented Mar 6, 2023

@haarchri Have rebased and re run make reviewable test - no changes.

Looks like there are a couple of resources reported by report-breaking-changes, although seems to be some new status fields and some parameters that are no longer required fields.

@haarchri
Copy link
Member

haarchri commented Mar 6, 2023

thats what i wanted to see ;)

Copy link
Member

@haarchri haarchri left a comment

Choose a reason for hiding this comment

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

LGTM

@haarchri haarchri merged commit 58d2a6e into crossplane-contrib:master Mar 6, 2023
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.

2 participants