From e1bb8a510338f5daa80a913fd2857f36c12c9a1b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 15:29:09 -0400 Subject: [PATCH 1/7] rds/option_group: Fix bad diffs with version and port --- internal/service/rds/flex.go | 10 +- internal/service/rds/option_group_test.go | 208 ++++++++++++++++++++++ 2 files changed, 216 insertions(+), 2 deletions(-) diff --git a/internal/service/rds/flex.go b/internal/service/rds/flex.go index b450a29a8e96..c745ea562eca 100644 --- a/internal/service/rds/flex.go +++ b/internal/service/rds/flex.go @@ -259,11 +259,17 @@ func flattenOptions(apiOptions []*rds.Option, optionConfigurations []*rds.Option "db_security_group_memberships": schema.NewSet(schema.HashString, dbSecurityGroupMemberships), "option_name": aws.StringValue(apiOption.OptionName), "option_settings": schema.NewSet(schema.HashResource(optionSettingsResource), optionSettings), - "port": aws.Int64Value(apiOption.Port), - "version": aws.StringValue(apiOption.OptionVersion), "vpc_security_group_memberships": schema.NewSet(schema.HashString, vpcSecurityGroupMemberships), } + if apiOption.OptionVersion != nil && configuredOption.OptionVersion != nil { + r["version"] = aws.StringValue(apiOption.OptionVersion) + } + + if apiOption.Port != nil && configuredOption.Port != nil { + r["port"] = aws.Int64Value(apiOption.Port) + } + result = append(result, r) } diff --git a/internal/service/rds/option_group_test.go b/internal/service/rds/option_group_test.go index 7b6f31d06683..0091f2da9178 100644 --- a/internal/service/rds/option_group_test.go +++ b/internal/service/rds/option_group_test.go @@ -521,6 +521,59 @@ func TestAccRDSOptionGroup_Tags_withOptions(t *testing.T) { }) } +// https://github.com/hashicorp/terraform-provider-aws/issues/21367 +func TestAccRDSOptionGroup_badDiffs(t *testing.T) { + ctx := acctest.Context(t) + var optionGroup1 rds.OptionGroup + resourceName := "aws_db_option_group.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckOptionGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccOptionGroupConfig_badDiffs1(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckOptionGroupExists(ctx, resourceName, &optionGroup1), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "port": "3872", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "option_name": "SQLT", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "option_name": "S3_INTEGRATION", + }), + ), + }, + { + Config: testAccOptionGroupConfig_badDiffs1(rName), + PlanOnly: true, + }, + { + Config: testAccOptionGroupConfig_badDiffs2(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckOptionGroupExists(ctx, resourceName, &optionGroup1), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "port": "3873", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "option_name": "SQLT", + "version": "2018-07-25.v1", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "option.*", map[string]string{ + "option_name": "S3_INTEGRATION", + "version": "1.0", + }), + ), + }, + }, + }) +} + func testAccCheckOptionGroupOptionSettingsIAMRole(optionGroup *rds.OptionGroup) resource.TestCheckFunc { return func(s *terraform.State) error { if optionGroup == nil { @@ -1053,3 +1106,158 @@ resource "aws_db_option_group" "test" { } `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } + +func testAccOptionGroupConfig_badDiffs1(rName string) string { + return fmt.Sprintf(` +resource "aws_security_group" "test" { + name = %[1]q +} + +data "aws_rds_engine_version" "default" { + engine = "oracle-ee" +} + +resource "aws_db_option_group" "test" { + name = %[1]q + option_group_description = "Option Group for Numagove" + engine_name = data.aws_rds_engine_version.default.engine + major_engine_version = regex("^\\d+", data.aws_rds_engine_version.default.version) + + option{ + option_name = "S3_INTEGRATION" + } + + option { + option_name = "SQLT" + option_settings { + name = "LICENSE_PACK" + value = "T" + } + } + + option { + option_name = "OEM_AGENT" + version = "13.5.0.0.v1" + port = 3872 + vpc_security_group_memberships = [aws_security_group.test.id] + + option_settings { + name = "AGENT_REGISTRATION_PASSWORD" + value = "TESTPASSWORDBGY" + } + option_settings { + name = "MINIMUM_TLS_VERSION" + value = "TLSv1.2" + } + option_settings { + name = "TLS_CIPHER_SUITE" + value = "TLS_RSA_WITH_AES_128_CBC_SHA" + } + option_settings { + name = "OMS_HOST" + value = "BGY-TEST" + } + option_settings { + name = "OMS_PORT" + value = "1159" + } + } +} +`, rName) +} + +func testAccOptionGroupConfig_badDiffs2(rName string) string { + return fmt.Sprintf(` +resource "aws_security_group" "test" { + name = %[1]q +} + +data "aws_rds_engine_version" "default" { + engine = "oracle-ee" +} + +resource "aws_db_option_group" "test" { + name = %[1]q + option_group_description = "Option Group for Numagove" + engine_name = data.aws_rds_engine_version.default.engine + major_engine_version = regex("^\\d+", data.aws_rds_engine_version.default.version) + + option{ + option_name = "S3_INTEGRATION" + version = "1.0" + } + + option { + option_name = "SQLT" + option_settings { + name = "LICENSE_PACK" + value = "T" + } + version = "2018-07-25.v1" + } + + option { + option_name = "OEM_AGENT" + version = "13.5.0.0.v1" + port = 3873 + vpc_security_group_memberships = [aws_security_group.test.id] + + option_settings { + name = "AGENT_REGISTRATION_PASSWORD" + value = "TESTPASSWORDBGY" + } + option_settings { + name = "MINIMUM_TLS_VERSION" + value = "TLSv1.2" + } + option_settings { + name = "TLS_CIPHER_SUITE" + value = "TLS_RSA_WITH_AES_128_CBC_SHA" + } + option_settings { + name = "OMS_HOST" + value = "BGY-TEST" + } + option_settings { + name = "OMS_PORT" + value = "1159" + } + } +} +`, rName) +} + +/* + option { + option_name = "Timezone" + option_settings { + name = "TIME_ZONE" + value = "Europe/London" + } + } + option{ + option_name = "UTL_MAIL" + } + + + option { + option_name = "NATIVE_NETWORK_ENCRYPTION" + option_settings { + name = "SQLNET.ENCRYPTION_SERVER" + value = "REQUIRED" + } + option_settings{ + name = "SQLNET.ENCRYPTION_TYPES_SERVER" + value = "AES256,AES192,AES128,RC4_256,RC4_128,RC4_56,RC4_40" + } + option_settings{ + name = "SQLNET.CRYPTO_CHECKSUM_SERVER" + value = "ACCEPTED" + } + option_settings{ + name = "SQLNET.CRYPTO_CHECKSUM_TYPES_SERVER" + value = "SHA256,SHA384,SHA512,SHA1,MD5" + } + } + +*/ From 254173234ed1b78103a11cbc2821259fd671d975 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 15:31:44 -0400 Subject: [PATCH 2/7] Add changelog --- .changelog/33511.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/33511.txt diff --git a/.changelog/33511.txt b/.changelog/33511.txt new file mode 100644 index 000000000000..8913bd9bb539 --- /dev/null +++ b/.changelog/33511.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_db_option_group: Avoid erroneous differences being reported when an `option` `port` and/or `version` is not set +``` \ No newline at end of file From 448d6f1d557aa7886eb905bc6371aad0988b41e0 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 15:33:16 -0400 Subject: [PATCH 3/7] Fix lint --- internal/service/rds/option_group_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/rds/option_group_test.go b/internal/service/rds/option_group_test.go index 0091f2da9178..db2dfc19ba4a 100644 --- a/internal/service/rds/option_group_test.go +++ b/internal/service/rds/option_group_test.go @@ -1123,7 +1123,7 @@ resource "aws_db_option_group" "test" { engine_name = data.aws_rds_engine_version.default.engine major_engine_version = regex("^\\d+", data.aws_rds_engine_version.default.version) - option{ + option { option_name = "S3_INTEGRATION" } @@ -1182,7 +1182,7 @@ resource "aws_db_option_group" "test" { engine_name = data.aws_rds_engine_version.default.engine major_engine_version = regex("^\\d+", data.aws_rds_engine_version.default.version) - option{ + option { option_name = "S3_INTEGRATION" version = "1.0" } From 8b15fd9d8c37ed3d2b3a8a28edfb907bc8284018 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 15:46:45 -0400 Subject: [PATCH 4/7] rds/option_group: Protect against null pointer --- internal/service/rds/flex.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/rds/flex.go b/internal/service/rds/flex.go index c745ea562eca..ca95cd449051 100644 --- a/internal/service/rds/flex.go +++ b/internal/service/rds/flex.go @@ -262,11 +262,11 @@ func flattenOptions(apiOptions []*rds.Option, optionConfigurations []*rds.Option "vpc_security_group_memberships": schema.NewSet(schema.HashString, vpcSecurityGroupMemberships), } - if apiOption.OptionVersion != nil && configuredOption.OptionVersion != nil { + if apiOption.OptionVersion != nil && configuredOption != nil && configuredOption.OptionVersion != nil { r["version"] = aws.StringValue(apiOption.OptionVersion) } - if apiOption.Port != nil && configuredOption.Port != nil { + if apiOption.Port != nil && configuredOption != nil && configuredOption.Port != nil { r["port"] = aws.Int64Value(apiOption.Port) } From 7ffa8c0907f8c1e95f06ae1c942a6d0bbb3bc686 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 16:12:12 -0400 Subject: [PATCH 5/7] docs/db_option_group: Clarify version, port --- website/docs/r/db_option_group.html.markdown | 38 ++++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/website/docs/r/db_option_group.html.markdown b/website/docs/r/db_option_group.html.markdown index dfdcfff0c548..c56882cd0798 100644 --- a/website/docs/r/db_option_group.html.markdown +++ b/website/docs/r/db_option_group.html.markdown @@ -62,35 +62,35 @@ More information about this can be found [here](https://docs.aws.amazon.com/Amaz This resource supports the following arguments: -* `name` - (Optional, Forces new resource) The name of the option group. If omitted, Terraform will assign a random, unique name. Must be lowercase, to match as it is stored in AWS. +* `name` - (Optional, Forces new resource) Name of the option group. If omitted, Terraform will assign a random, unique name. Must be lowercase, to match as it is stored in AWS. * `name_prefix` - (Optional, Forces new resource) Creates a unique name beginning with the specified prefix. Conflicts with `name`. Must be lowercase, to match as it is stored in AWS. -* `option_group_description` - (Optional) The description of the option group. Defaults to "Managed by Terraform". +* `option_group_description` - (Optional) Description of the option group. Defaults to "Managed by Terraform". * `engine_name` - (Required) Specifies the name of the engine that this option group should be associated with. * `major_engine_version` - (Required) Specifies the major version of the engine that this option group should be associated with. -* `option` - (Optional) A list of Options to apply. -* `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. +* `option` - (Optional) List of options to apply. +* `tags` - (Optional) Map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. -Option blocks support the following: +`option` blocks support the following: -* `option_name` - (Required) The Name of the Option (e.g., MEMCACHED). -* `option_settings` - (Optional) A list of option settings to apply. -* `port` - (Optional) The Port number when connecting to the Option (e.g., 11211). -* `version` - (Optional) The version of the option (e.g., 13.1.0.0). -* `db_security_group_memberships` - (Optional) A list of DB Security Groups for which the option is enabled. -* `vpc_security_group_memberships` - (Optional) A list of VPC Security Groups for which the option is enabled. +* `option_name` - (Required) Name of the option (e.g., MEMCACHED). +* `option_settings` - (Optional) List of option settings to apply. +* `port` - (Optional) Port number when connecting to the option (e.g., 11211). Leaving out or removing `port` from your configuration does not remove or clear a port from the option in AWS. AWS may assign a default port. Not including `port` in your configuration means that the AWS provider will ignore a previously set value, a value set by AWS, and any port changes. +* `version` - (Optional) Version of the option (e.g., 13.1.0.0). Leaving out or removing `version` from your configuration does not remove or clear a version from the option in AWS. AWS may assign a default version. Not including `version` in your configuration means that the AWS provider will ignore a previously set value, a value set by AWS, and any version changes. +* `db_security_group_memberships` - (Optional) List of DB Security Groups for which the option is enabled. +* `vpc_security_group_memberships` - (Optional) List of VPC Security Groups for which the option is enabled. -Option Settings blocks support the following: +`option_settings` blocks support the following: -* `name` - (Optional) The Name of the setting. -* `value` - (Optional) The Value of the setting. +* `name` - (Optional) Name of the setting. +* `value` - (Optional) Value of the setting. ## Attribute Reference This resource exports the following attributes in addition to the arguments above: -* `id` - The db option group name. -* `arn` - The ARN of the db option group. -* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). +* `id` - DB option group name. +* `arn` - ARN of the db option group. +* `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). ## Timeouts @@ -100,7 +100,7 @@ This resource exports the following attributes in addition to the arguments abov ## Import -In Terraform v1.5.0 and later, use an [`import` block](https://developer.hashicorp.com/terraform/language/import) to import DB Option groups using the `name`. For example: +In Terraform v1.5.0 and later, use an [`import` block](https://developer.hashicorp.com/terraform/language/import) to import DB option groups using the `name`. For example: ```terraform import { @@ -109,7 +109,7 @@ import { } ``` -Using `terraform import`, import DB Option groups using the `name`. For example: +Using `terraform import`, import DB option groups using the `name`. For example: ```console % terraform import aws_db_option_group.example mysql-option-group From 197ffea553d793b6c550f382beb3d98f920e0d15 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 16:13:11 -0400 Subject: [PATCH 6/7] Removed unnecessary comment --- internal/service/rds/option_group_test.go | 35 ----------------------- 1 file changed, 35 deletions(-) diff --git a/internal/service/rds/option_group_test.go b/internal/service/rds/option_group_test.go index db2dfc19ba4a..13aaa8bc6eef 100644 --- a/internal/service/rds/option_group_test.go +++ b/internal/service/rds/option_group_test.go @@ -1226,38 +1226,3 @@ resource "aws_db_option_group" "test" { } `, rName) } - -/* - option { - option_name = "Timezone" - option_settings { - name = "TIME_ZONE" - value = "Europe/London" - } - } - option{ - option_name = "UTL_MAIL" - } - - - option { - option_name = "NATIVE_NETWORK_ENCRYPTION" - option_settings { - name = "SQLNET.ENCRYPTION_SERVER" - value = "REQUIRED" - } - option_settings{ - name = "SQLNET.ENCRYPTION_TYPES_SERVER" - value = "AES256,AES192,AES128,RC4_256,RC4_128,RC4_56,RC4_40" - } - option_settings{ - name = "SQLNET.CRYPTO_CHECKSUM_SERVER" - value = "ACCEPTED" - } - option_settings{ - name = "SQLNET.CRYPTO_CHECKSUM_TYPES_SERVER" - value = "SHA256,SHA384,SHA512,SHA1,MD5" - } - } - -*/ From 3f6de002ede9c188e118686d1e11d6b4b3d157b0 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 18 Sep 2023 16:19:16 -0400 Subject: [PATCH 7/7] docs/db_option_group: Fix capitalization --- website/docs/r/db_option_group.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/db_option_group.html.markdown b/website/docs/r/db_option_group.html.markdown index c56882cd0798..da7ac98d8b2a 100644 --- a/website/docs/r/db_option_group.html.markdown +++ b/website/docs/r/db_option_group.html.markdown @@ -89,7 +89,7 @@ This resource supports the following arguments: This resource exports the following attributes in addition to the arguments above: * `id` - DB option group name. -* `arn` - ARN of the db option group. +* `arn` - ARN of the DB option group. * `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). ## Timeouts