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.

Sign up for GitHub

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

Jump to bottom

fix(rds): pass the ARN of master instead of its ID in DatabaseInstanceReadReplica #5702

Merged
merged 2 commits into from
Jan 8, 2020
Merged

fix(rds): pass the ARN of master instead of its ID in DatabaseInstanceReadReplica #5702

merged 2 commits into from
Jan 8, 2020

Conversation

skinny85
Copy link
Contributor

@skinny85 skinny85 commented Jan 8, 2020 •
edited
Loading

This works around the "DbSubnetGroupName should not be specified for read replicas that are created in the same region as the master" RDS validation.

See hashicorp/terraform-provider-aws#528 (comment) for details.

Fixes #5530

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@skinny85 skinny85 requested a review from NetaNir January 8, 2020 00:49
@skinny85 skinny85 self-assigned this Jan 8, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 8, 2020
@skinny85
Copy link
Contributor Author

skinny85 commented Jan 8, 2020

This is good enough to solve the bug, but creates an extra AWS::RDS::DBSubnetGroup resource that is not used. Probably need to get rid of that as well.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@skinny85
Copy link
Contributor Author

skinny85 commented Jan 8, 2020

Pertinent read on the topic, thanks to @NetaNir: hashicorp/terraform-provider-aws#528 (comment)

@skinny85
fix(rds): pass the ARN of master instead of its ID in DatabaseInstanc…
d1bee53 
…eReadReplica

This works around the
"DbSubnetGroupName should not be specified for read replicas that are
created in the same region as the master" RDS validation.

See hashicorp/terraform-provider-aws#528 (comment)
for details.

Fixes aws#5530
@skinny85 skinny85 force-pushed the fix/rds-replica-same-region-master-subnet-group branch from 11c9844 to d1bee53 Compare January 8, 2020 02:21
@skinny85 skinny85 changed the title fix(rds): read replicas in the same region as master have subnet group name filled fix(rds): pass the ARN of master instead of its ID in DatabaseInstanceReadReplica Jan 8, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

NetaNir
NetaNir approved these changes Jan 8, 2020
View reviewed changes
Copy link
Contributor

@NetaNir NetaNir left a comment

Choose a reason for hiding this comment

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

Since we were managed to enable this we should probably expose the dbSubnetGroupName in the DatabaseInstanceReadReplicaProps

@mergify
Copy link
Contributor

mergify bot commented Jan 8, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jan 8, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit d323c0c into aws:master Jan 8, 2020
@eladb
Copy link
Contributor

eladb commented Jan 8, 2020

@skinny85 @NetaNir the title of bug fix PRs should describe the bug, not the solution. Think that this title is going under "Bug Fixes" in the CHANGELOG, so I'd expect to see there "which bugs were fixed".

@skinny85
Copy link
Contributor Author

skinny85 commented Jan 8, 2020

@skinny85 @NetaNir the title of bug fix PRs should describe the bug, not the solution. Think that this title is going under "Bug Fixes" in the CHANGELOG, so I'd expect to see there "which bugs were fixed".

At first, that's what I did, but this fix is so removed from the actual bug that I changed it, because nobody would see what the actual behavior change is if I only talked about the bug.

@skinny85 skinny85 deleted the fix/rds-replica-same-region-master-subnet-group branch January 8, 2020 17:26
@eladb
Copy link
Contributor

eladb commented Jan 8, 2020

Still, the title should be the bug that's fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NetaNir NetaNir NetaNir approved these changes

Assignees

@skinny85 skinny85

Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DbSubnetGroupName should not be specified for read replicas that are created in the same region as the master
4 participants
@skinny85 @aws-cdk-automation @eladb @NetaNir