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

resource/aws_db_option_group: Read option attribute into Terraform state and skip ModifyOptionGroup when no option updates #7125

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Jan 12, 2019

Closes #209 (covered by TestAccAWSDBOptionGroup_Option_OptionSettings_IAMRole)
Fixes #1167 (covered by ImportState TestStep added to all acceptance tests)
Closes #1876 (covered by TestAccAWSDBOptionGroup_Option_OptionSettings_MultipleNonDefault)
Fixes #6365 (covered by ImportState TestStep added to all acceptance tests)
Fixes #7114 (covered by TestAccAWSDBOptionGroup_Tags_WithOptions)

Previously, this Terraform resource was not able to perform any drift detection on or import the state of the option attribute. The caveat is that while RDS returns only modified options, it will return all option settings whether modified or not from their default values. To workaround this option settings issue, we pass in the options from the Terraform configuration and ignore default values that are not present in the configuration.

This also fixes an issue on update where ModifyOptionGroup was being called without any actual option updates, which would generate an error and prevent other updates from occurring. While the update logic was correctly using d.HasChange() before, when working with complex TypeSet attributes like option it can apparently be fooled. We use similar length check logic in the aws_db_parameter_group resource for example.

Output from acceptance testing (AWS Standard):

--- PASS: TestAccAWSDBOptionGroup_timeoutBlock (13.50s)
--- PASS: TestAccAWSDBOptionGroup_generatedName (13.50s)
--- PASS: TestAccAWSDBOptionGroup_namePrefix (13.58s)
--- PASS: TestAccAWSDBOptionGroup_basic (13.67s)
--- PASS: TestAccAWSDBOptionGroup_multipleOptions (14.08s)
--- PASS: TestAccAWSDBOptionGroup_sqlServerOptionsUpdate (23.90s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings (27.86s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_MultipleNonDefault (29.37s)
--- PASS: TestAccAWSDBOptionGroup_OracleOptionsUpdate (32.14s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_IAMRole (32.40s)
--- PASS: TestAccAWSDBOptionGroup_Tags (38.88s)
--- PASS: TestAccAWSDBOptionGroup_Tags_WithOptions (43.07s)
--- PASS: TestAccAWSDBOptionGroup_basicDestroyWithInstance (499.04s)

Output from acceptance testing (AWS GovCloud (US), failures will be separately addressed with other GovCloud fixes):

--- FAIL: TestAccAWSDBOptionGroup_OracleOptionsUpdate (12.33s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_db_option_group.bar: 1 error occurred:
        	* aws_db_option_group.bar: Error modifying DB Option Group: InvalidParameterValue: Version 12.1.0.4.v1 is invalid for OEM_AGENT option. Valid versions are 13.2.0.0.v1,13.2.0.0.v2
        	status code: 400, request id: 748c6b88-9406-436b-b755-461feb20abbf

--- PASS: TestAccAWSDBOptionGroup_namePrefix (14.68s)
--- PASS: TestAccAWSDBOptionGroup_basic (14.73s)
--- PASS: TestAccAWSDBOptionGroup_generatedName (14.80s)
--- PASS: TestAccAWSDBOptionGroup_timeoutBlock (14.82s)
--- PASS: TestAccAWSDBOptionGroup_multipleOptions (15.53s)
--- FAIL: TestAccAWSDBOptionGroup_Option_OptionSettings_IAMRole (19.14s)
    testing.go:538: Step 0 error: Check failed: Check 4/4 error: Expected option setting to be a valid IAM role but received arn:aws-us-gov:iam::--OMITTED--:role/rds-backup-option-group-test-terraform-0w89k
--- PASS: TestAccAWSDBOptionGroup_sqlServerOptionsUpdate (25.33s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_MultipleNonDefault (26.42s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings (28.46s)
--- PASS: TestAccAWSDBOptionGroup_Tags (35.49s)
--- PASS: TestAccAWSDBOptionGroup_Tags_WithOptions (39.27s)
--- PASS: TestAccAWSDBOptionGroup_basicDestroyWithInstance (498.37s)

@bflad bflad added bug Addresses a defect in current functionality. service/rds Issues and PRs that pertain to the rds service. labels Jan 12, 2019
@bflad bflad requested a review from a team January 12, 2019 15:15
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jan 12, 2019
…ate and skip ModifyOptionGroup when no option updates

References:
* #209
* #1167
* #1876
* #6365
* #7114

Previously, this Terraform resource was not able to perform any drift detection on or import the state of the `option` attribute. The caveat is that while RDS returns only modified options, it will return all option settings whether modified or not from their default values. To workaround this option settings issue, we pass in the options from the Terraform configuration and ignore default values that are not present in the configuration.

This also fixes an issue on update where `ModifyOptionGroup` was being called without any actual option updates, which would generate an error and prevent other updates from occurring. While the update logic was correctly using `d.HasChange()` before, when working with complex `TypeSet` attributes like `option` it can apparently be fooled. We use similar length check logic in the `aws_db_parameter_group` resource for example.

Output from acceptance testing (AWS Standard):

```
--- PASS: TestAccAWSDBOptionGroup_timeoutBlock (13.50s)
--- PASS: TestAccAWSDBOptionGroup_generatedName (13.50s)
--- PASS: TestAccAWSDBOptionGroup_namePrefix (13.58s)
--- PASS: TestAccAWSDBOptionGroup_basic (13.67s)
--- PASS: TestAccAWSDBOptionGroup_multipleOptions (14.08s)
--- PASS: TestAccAWSDBOptionGroup_sqlServerOptionsUpdate (23.90s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings (27.86s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_MultipleNonDefault (29.37s)
--- PASS: TestAccAWSDBOptionGroup_OracleOptionsUpdate (32.14s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_IAMRole (32.40s)
--- PASS: TestAccAWSDBOptionGroup_Tags (38.88s)
--- PASS: TestAccAWSDBOptionGroup_Tags_WithOptions (43.07s)
--- PASS: TestAccAWSDBOptionGroup_basicDestroyWithInstance (499.04s)
```

Output from acceptance testing (AWS GovCloud (US), failures will be separately addressed with other GovCloud fixes):

```
--- FAIL: TestAccAWSDBOptionGroup_OracleOptionsUpdate (12.33s)
    testing.go:538: Step 0 error: Error applying: 1 error occurred:
        	* aws_db_option_group.bar: 1 error occurred:
        	* aws_db_option_group.bar: Error modifying DB Option Group: InvalidParameterValue: Version 12.1.0.4.v1 is invalid for OEM_AGENT option. Valid versions are 13.2.0.0.v1,13.2.0.0.v2
        	status code: 400, request id: 748c6b88-9406-436b-b755-461feb20abbf

--- PASS: TestAccAWSDBOptionGroup_namePrefix (14.68s)
--- PASS: TestAccAWSDBOptionGroup_basic (14.73s)
--- PASS: TestAccAWSDBOptionGroup_generatedName (14.80s)
--- PASS: TestAccAWSDBOptionGroup_timeoutBlock (14.82s)
--- PASS: TestAccAWSDBOptionGroup_multipleOptions (15.53s)
--- FAIL: TestAccAWSDBOptionGroup_Option_OptionSettings_IAMRole (19.14s)
    testing.go:538: Step 0 error: Check failed: Check 4/4 error: Expected option setting to be a valid IAM role but received arn:aws-us-gov:iam::--OMITTED--:role/rds-backup-option-group-test-terraform-0w89k
--- PASS: TestAccAWSDBOptionGroup_sqlServerOptionsUpdate (25.33s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings_MultipleNonDefault (26.42s)
--- PASS: TestAccAWSDBOptionGroup_Option_OptionSettings (28.46s)
--- PASS: TestAccAWSDBOptionGroup_Tags (35.49s)
--- PASS: TestAccAWSDBOptionGroup_Tags_WithOptions (39.27s)
--- PASS: TestAccAWSDBOptionGroup_basicDestroyWithInstance (498.37s)
```
Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM

@bflad bflad added this to the v1.56.0 milestone Jan 14, 2019
@bflad bflad merged commit 7e2b875 into master Jan 14, 2019
@bflad bflad deleted the b-aws_db_option_group-tags branch January 14, 2019 13:29
bflad added a commit that referenced this pull request Jan 14, 2019
bflad added a commit that referenced this pull request Jan 14, 2019
@bflad
Copy link
Contributor Author

bflad commented Jan 16, 2019

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

@ghost
Copy link

ghost commented Apr 1, 2020

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 and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/rds Issues and PRs that pertain to the rds service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
2 participants