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

RDS - Throw error when name or username supplied on DB snapshot restore #17156

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

brent-au
Copy link
Contributor

This PR will introduce a behaviour to throw an error when restoring from a MySQL, PostgreSQL or MariaDB RDS snapshot and a name or username attribute is supplied as part of a template.

This is to change the current behaviour where the RDS instance is restored from snapshot, but subsequent applies of the same template cause the RDS to be recreated because the name and/or username attribute has not been honoured during creation.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #17037

Release note for CHANGELOG:

An error will now be thrown when supplying a name and/or username attribute when restoring a snapshot to a MySQL, PostgreSQL or MariaDB to an RDS instance.

Output from acceptance testing:

I modifed the MySQL snapshot restore with engine version test template here to include a name attribute for the mysql_restore resource, to provoke an error

$ make testacc TESTARGS='-run=TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion -timeout 120m
=== RUN   TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion
=== PAUSE TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion
=== CONT  TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion
    resource_aws_db_instance_test.go:2193: Step 1/1 error: Error running apply: 
        Error: Error restoring database from AWS DB Snapshot: tf-acc-test-4765336381358846440: name attribute is not supported when engine is mysql
        
        

--- FAIL: TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion (590.06s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       590.092s
FAIL
make: *** [GNUmakefile:28: testacc] Error 1
...

for rds snapshot restore of mysql, postgresql or mariadb
@brent-au brent-au requested a review from a team as a code owner January 18, 2021 12:54
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/rds Issues and PRs that pertain to the rds service. labels Jan 18, 2021
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jan 18, 2021
Base automatically changed from master to main January 23, 2021 01:00
@gdavison gdavison self-assigned this Feb 11, 2021
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @brent-au! I've moved the validation to plan-time and fixed an acceptance test that now causes an error

@gdavison gdavison removed the needs-triage Waiting for first response or review from a maintainer. label Feb 18, 2021
gdavison added a commit that referenced this pull request Feb 18, 2021
@gdavison gdavison merged commit 0c34cd5 into hashicorp:main Feb 18, 2021
@github-actions github-actions bot added this to the v3.29.0 milestone Feb 18, 2021
@ghost
Copy link

ghost commented Feb 19, 2021

This has been released in version 3.29.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@konstl000
Copy link

konstl000 commented Feb 19, 2021

It seems, that raising an error is not enough, since the name of the DB instance is present in the state after it is created from the snapshot. It seems that this PR introduces #17712.

@JeremyPDC
Copy link

JeremyPDC commented Feb 19, 2021

It seems, that raising an error is not enough, since the name of the DB instance is present in the state after it is created from the snapshot. It seems that this PR introduces #17712.

Yeah we are having this problem now.

@aleon1220
Copy link

is happenning to me now and i am on terraform cloud. I believe the conflict is in
the duplicate use of the var.snapshot_identifier

Take a look at line 63 and 139

https://github.com/terraform-aws-modules/terraform-aws-rds/blob/master/modules/db_instance/main.tf#L139

@aleon1220
Copy link

I think the majority of the community is tracking the issue at #17712

@ghost
Copy link

ghost commented Mar 21, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/rds Issues and PRs that pertain to the rds service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RDS instance got recreated after another apply without any change
5 participants