-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Adds domain and domain_iam_role_name parameters to resource aws_db_instance #1363
Conversation
Originally submitted as part of hashicorp/terraform#8190.
Signed-off-by: Valentin Pichard <valentin.pichard@corp.ovh.com>
This example demonstrates both creating a network architecture *and* the use of data resources to minimize the number of variables needed for a child module by discovering additional data automatically.
* Append the info about IAM Server Certificate import * Update iam_server_certificate.html.markdown
r/aws_eip_association: Avoid crash in EC2 Classic
…-removal r/elasticache_parameter_group: Allow removing parameters
increasing timeout for delete and update
…-kinesis-stream Add docs for timeouts added to Kinesis Stream
fix for issue where s3 lifecycle rule prefix key value is not set corr…
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.
Hey @nckslvrmn! Thanks so much for migrating this. Couple requests:
-
Because it looks like @mikewalker125 originally wrote the code in the PR, could just update the authorship info on the commits that are comprised of their code? I'd like to make sure they get credit for their work! Of course, any changes not in the original PR should be under your name, so you get credit for your work, as well. :)
-
Before we can merge this, we need to add some integration tests that exercise the new fields.
aws/resource_aws_db_instance_test.go
should have some examples in there already you can use as a basis. If you need any help or guidance, please feel free to ask. If you don't have the time/inclination to take up the task, let me know, and I'll be happy to carry this forward. :)
Thanks for your interest in contributing to Terraform!
Hey @paddycarver. I can certainly take a look at those integration tests and give those a shot. As for @mikewalker125 's correct authorship. i would need his name and email no? I can deduce his name from his username but I'm not sure how I'd get his email to properly change the commit authorship. |
Sorry, been away from this for a few weeks. I can update the authorship of commit that @nckslvrmn pulled over. @nckslvrmn, let me know if you'd like help with the integration tests. Thanks for pulling this over! Mike |
…der-aws into rds-add-domain
i borked the whole thing up, my apologies. |
@mikewalker125 if you wanna move the PR over again that would be excellent. The only thing i changed was i made the domain iam role name variable have underscores rather than dashes to follow common patterns. |
@nckslvrmn Will do it now. If you're interested in doing integration tests, I can grant you access to push to my branch. |
Did this ever get implemented? This PR is from over a year ago and yet I see no mention of it here? https://www.terraform.io/docs/providers/aws/r/db_instance.html |
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! |
I have pulled this code into a new PR as requested by @paddycarver. This was originally written by @mikewalker125.