From 73fcefea559e64129803899ad006e73665ad87df Mon Sep 17 00:00:00 2001 From: architec <32494274+architec@users.noreply.github.com> Date: Thu, 22 Aug 2024 22:58:28 +0000 Subject: [PATCH 01/15] fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.13 and ActiveMQ 5.18 change to their corresponding patch version --- internal/service/mq/broker.go | 17 ++- internal/service/mq/broker_test.go | 238 ++++++++++++++++++++++++++++- 2 files changed, 251 insertions(+), 4 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index cae69098a26..f91f1d45953 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "golang.org/x/mod/semver" "log" "reflect" "strconv" @@ -462,6 +463,20 @@ func resourceBrokerCreate(ctx context.Context, d *schema.ResourceData, meta inte return append(diags, resourceBrokerRead(ctx, d, meta)...) } +func simplifiedVersion(engineType types.EngineType, autoMinorVersionUpgrade *bool, engineVersion *string) string { + if autoMinorVersionUpgrade == nil || !*autoMinorVersionUpgrade { + return *engineVersion + } + majorMinorEngineVersion := semver.MajorMinor("v" + *engineVersion) + rabbitSimplifiedVersion := "v3.13" + activeSimplifiedVersion := "v5.18" + if (engineType == types.EngineTypeRabbitmq && semver.Compare(majorMinorEngineVersion, rabbitSimplifiedVersion) == 0) || + (engineType == types.EngineTypeActivemq && semver.Compare(majorMinorEngineVersion, activeSimplifiedVersion) == 0) { + return majorMinorEngineVersion[1:] + } + return *engineVersion +} + func resourceBrokerRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics @@ -486,7 +501,7 @@ func resourceBrokerRead(ctx context.Context, d *schema.ResourceData, meta interf d.Set("data_replication_mode", output.DataReplicationMode) d.Set("deployment_mode", output.DeploymentMode) d.Set("engine_type", output.EngineType) - d.Set(names.AttrEngineVersion, output.EngineVersion) + d.Set(names.AttrEngineVersion, simplifiedVersion(output.EngineType, output.AutoMinorVersionUpgrade, output.EngineVersion)) d.Set("host_instance_type", output.HostInstanceType) d.Set("instances", flattenBrokerInstances(output.BrokerInstances)) d.Set("pending_data_replication_mode", output.PendingDataReplicationMode) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 93ab9f530e5..ec34f3df99c 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -241,9 +241,11 @@ func TestDiffUsers(t *testing.T) { } const ( - testAccBrokerVersionNewer = "5.17.6" // before changing, check b/c must be valid on GovCloud - testAccBrokerVersionOlder = "5.16.7" // before changing, check b/c must be valid on GovCloud - testAccRabbitVersion = "3.11.20" // before changing, check b/c must be valid on GovCloud + testAccBrokerVersionNewer = "5.17.6" // before changing, check b/c must be valid on GovCloud + testAccBrokerVersionOlder = "5.16.7" // before changing, check b/c must be valid on GovCloud + testAccActiveVersionSimplified = "5.18" + testAccRabbitVersion = "3.11.20" // before changing, check b/c must be valid on GovCloud + testAccRabbitVersionSimplified = "3.13" ) func TestAccMQBroker_basic(t *testing.T) { @@ -325,6 +327,85 @@ func TestAccMQBroker_basic(t *testing.T) { }) } +func TestAccMQBroker_basic_simplified_version(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var broker mq.DescribeBrokerOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_mq_broker.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.MQEndpointID) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.MQServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBrokerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBrokerConfig_basic_autoMinorVersionUpgrade(rName, testAccActiveVersionSimplified, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBrokerExists(ctx, resourceName, &broker), + acctest.MatchResourceAttrRegionalARN(resourceName, names.AttrARN, "mq", regexache.MustCompile(`broker:+.`)), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "authentication_strategy", "simple"), + resource.TestCheckResourceAttr(resourceName, "broker_name", rName), + resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), + resource.TestMatchResourceAttr(resourceName, "configuration.0.id", regexache.MustCompile(`^c-[0-9a-z-]+$`)), + resource.TestMatchResourceAttr(resourceName, "configuration.0.revision", regexache.MustCompile(`^[0-9]+$`)), + resource.TestCheckResourceAttr(resourceName, "deployment_mode", "SINGLE_INSTANCE"), + resource.TestCheckResourceAttr(resourceName, "encryption_options.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "encryption_options.0.use_aws_owned_key", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "engine_type", "ActiveMQ"), + resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccActiveVersionSimplified), + resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t2.micro"), + resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), + resource.TestMatchResourceAttr(resourceName, "instances.0.console_url", + regexache.MustCompile(`^https://[0-9a-f-]+\.mq.[0-9a-z-]+.amazonaws.com:8162$`)), + resource.TestCheckResourceAttr(resourceName, "instances.0.endpoints.#", "5"), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.0", regexache.MustCompile(`^ssl://[0-9a-z.-]+:61617$`)), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.1", regexache.MustCompile(`^amqp\+ssl://[0-9a-z.-]+:5671$`)), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.2", regexache.MustCompile(`^stomp\+ssl://[0-9a-z.-]+:61614$`)), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.3", regexache.MustCompile(`^mqtt\+ssl://[0-9a-z.-]+:8883$`)), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.4", regexache.MustCompile(`^wss://[0-9a-z.-]+:61619$`)), + resource.TestMatchResourceAttr(resourceName, "instances.0.ip_address", + regexache.MustCompile(`^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$`)), + resource.TestCheckResourceAttr(resourceName, "maintenance_window_start_time.#", acctest.Ct1), + resource.TestCheckResourceAttrSet(resourceName, "maintenance_window_start_time.0.day_of_week"), + resource.TestCheckResourceAttrSet(resourceName, "maintenance_window_start_time.0.time_of_day"), + resource.TestCheckResourceAttr(resourceName, "logs.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "logs.0.general", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "logs.0.audit", acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "maintenance_window_start_time.0.time_zone", "UTC"), + resource.TestCheckResourceAttr(resourceName, names.AttrPubliclyAccessible, acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "security_groups.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, names.AttrStorageType, "efs"), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, acctest.Ct0), + resource.TestCheckResourceAttr(resourceName, "user.#", acctest.Ct1), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "user.*", map[string]string{ + "console_access": acctest.CtFalse, + "groups.#": acctest.Ct0, + names.AttrUsername: "Test", + names.AttrPassword: "TestTest1234", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrApplyImmediately, "user"}, + }, + }, + }) +} + func TestAccMQBroker_disappears(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { @@ -1126,6 +1207,99 @@ func TestAccMQBroker_RabbitMQ_basic(t *testing.T) { }) } +func TestAccMQBroker_RabbitMQ_basic_with_auto_minor_version_upgrade(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var broker mq.DescribeBrokerOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_mq_broker.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.MQEndpointID) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.MQServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBrokerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, testAccRabbitVersion, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckBrokerExists(ctx, resourceName, &broker), + resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "engine_type", "RabbitMQ"), + resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccRabbitVersion), + resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t3.micro"), + resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "instances.0.endpoints.#", acctest.Ct1), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.0", regexache.MustCompile(`^amqps://[0-9a-z.-]+:5671$`)), + resource.TestCheckResourceAttr(resourceName, "logs.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "logs.0.general", acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "logs.0.audit", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrApplyImmediately, "user"}, + }, + }, + }) +} + +func TestAccMQBroker_RabbitMQ_basic_simplified_version(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var broker mq.DescribeBrokerOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_mq_broker.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + acctest.PreCheck(ctx, t) + acctest.PreCheckPartitionHasService(t, names.MQEndpointID) + testAccPreCheck(ctx, t) + }, + ErrorCheck: acctest.ErrorCheck(t, names.MQServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBrokerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, testAccRabbitVersionSimplified, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckBrokerExists(ctx, resourceName, &broker), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "engine_type", "RabbitMQ"), + resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccRabbitVersionSimplified), + resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t3.micro"), + resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "instances.0.endpoints.#", acctest.Ct1), + resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.0", regexache.MustCompile(`^amqps://[0-9a-z.-]+:5671$`)), + resource.TestCheckResourceAttr(resourceName, "logs.#", acctest.Ct1), + resource.TestCheckResourceAttr(resourceName, "logs.0.general", acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, "logs.0.audit", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{names.AttrApplyImmediately, "user"}, + }, + }, + }) +} + func TestAccMQBroker_RabbitMQ_config(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { @@ -1602,6 +1776,38 @@ resource "aws_mq_broker" "test" { `, rName, version) } +func testAccBrokerConfig_basic_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { + return fmt.Sprintf(` +resource "aws_security_group" "test" { + name = %[1]q + + tags = { + Name = %[1]q + } +} + +resource "aws_mq_broker" "test" { + broker_name = %[1]q + engine_type = "ActiveMQ" + engine_version = %[2]q + host_instance_type = "mq.t2.micro" + security_groups = [aws_security_group.test.id] + authentication_strategy = "simple" + storage_type = "efs" + auto_minor_version_upgrade = %[3]t + + logs { + general = true + } + + user { + username = "Test" + password = "TestTest1234" + } +} +`, rName, version, autoMinorVersionUpgrade) +} + func testAccBrokerConfig_ebs(rName, version string) string { return fmt.Sprintf(` resource "aws_security_group" "test" { @@ -2146,6 +2352,32 @@ resource "aws_mq_broker" "test" { `, rName, version) } +func testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { + return fmt.Sprintf(` +resource "aws_security_group" "test" { + name = %[1]q + + tags = { + Name = %[1]q + } +} + +resource "aws_mq_broker" "test" { + broker_name = %[1]q + engine_type = "RabbitMQ" + engine_version = %[2]q + host_instance_type = "mq.t3.micro" + security_groups = [aws_security_group.test.id] + auto_minor_version_upgrade = %[3]t + + user { + username = "Test" + password = "TestTest1234" + } +} +`, rName, version, autoMinorVersionUpgrade) +} + func testAccBrokerConfig_rabbitConfig(rName, version string) string { return fmt.Sprintf(` resource "aws_security_group" "test" { From 635d82580f580a26bf16bf6679af47a904314395 Mon Sep 17 00:00:00 2001 From: architec <32494274+architec@users.noreply.github.com> Date: Thu, 22 Aug 2024 22:58:28 +0000 Subject: [PATCH 02/15] fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.13 and ActiveMQ 5.18 change to their corresponding patch version --- internal/service/mq/broker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index ec34f3df99c..9ce8935eeb2 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -352,7 +352,7 @@ func TestAccMQBroker_basic_simplified_version(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), acctest.MatchResourceAttrRegionalARN(resourceName, names.AttrARN, "mq", regexache.MustCompile(`broker:+.`)), - resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtFalse), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), resource.TestCheckResourceAttr(resourceName, "authentication_strategy", "simple"), resource.TestCheckResourceAttr(resourceName, "broker_name", rName), resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), From 3e775e7e8d6f7454c793a0347aae4da1c9f629c4 Mon Sep 17 00:00:00 2001 From: architec <32494274+architec@users.noreply.github.com> Date: Thu, 22 Aug 2024 22:58:28 +0000 Subject: [PATCH 03/15] fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.13 and ActiveMQ 5.18 change to their corresponding patch version --- internal/service/mq/broker.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index f91f1d45953..dd4739577f1 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -470,8 +470,8 @@ func simplifiedVersion(engineType types.EngineType, autoMinorVersionUpgrade *boo majorMinorEngineVersion := semver.MajorMinor("v" + *engineVersion) rabbitSimplifiedVersion := "v3.13" activeSimplifiedVersion := "v5.18" - if (engineType == types.EngineTypeRabbitmq && semver.Compare(majorMinorEngineVersion, rabbitSimplifiedVersion) == 0) || - (engineType == types.EngineTypeActivemq && semver.Compare(majorMinorEngineVersion, activeSimplifiedVersion) == 0) { + if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, rabbitSimplifiedVersion) == 0) || + (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, activeSimplifiedVersion) == 0) { return majorMinorEngineVersion[1:] } return *engineVersion @@ -571,10 +571,14 @@ func resourceBrokerUpdate(ctx context.Context, d *schema.ResourceData, meta inte } if d.HasChanges(names.AttrConfiguration, "logs", names.AttrEngineVersion) { + output, errors := findBrokerByID(ctx, conn, d.Id()) + if errors != nil { + return sdkdiag.AppendErrorf(diags, "reading MQ Broker (%s): %s", d.Id(), errors) + } input := &mq.UpdateBrokerInput{ BrokerId: aws.String(d.Id()), Configuration: expandConfigurationId(d.Get(names.AttrConfiguration).([]interface{})), - EngineVersion: aws.String(d.Get(names.AttrEngineVersion).(string)), + EngineVersion: aws.String(simplifiedVersion(output.EngineType, output.AutoMinorVersionUpgrade, output.EngineVersion)), Logs: expandLogs(d.Get("engine_type").(string), d.Get("logs").([]interface{})), } From 8331fda23ea7425e605e3a59ac5f3dbd2898705b Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 11 Sep 2024 11:30:56 -0400 Subject: [PATCH 04/15] chore: make testacc-lint-fix --- internal/service/mq/broker_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 9ce8935eeb2..0467f23cca0 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -1787,13 +1787,13 @@ resource "aws_security_group" "test" { } resource "aws_mq_broker" "test" { - broker_name = %[1]q - engine_type = "ActiveMQ" - engine_version = %[2]q - host_instance_type = "mq.t2.micro" - security_groups = [aws_security_group.test.id] - authentication_strategy = "simple" - storage_type = "efs" + broker_name = %[1]q + engine_type = "ActiveMQ" + engine_version = %[2]q + host_instance_type = "mq.t2.micro" + security_groups = [aws_security_group.test.id] + authentication_strategy = "simple" + storage_type = "efs" auto_minor_version_upgrade = %[3]t logs { @@ -2363,11 +2363,11 @@ resource "aws_security_group" "test" { } resource "aws_mq_broker" "test" { - broker_name = %[1]q - engine_type = "RabbitMQ" - engine_version = %[2]q - host_instance_type = "mq.t3.micro" - security_groups = [aws_security_group.test.id] + broker_name = %[1]q + engine_type = "RabbitMQ" + engine_version = %[2]q + host_instance_type = "mq.t3.micro" + security_groups = [aws_security_group.test.id] auto_minor_version_upgrade = %[3]t user { From 9501238278d6a4e3df5c8601908f9d2620055f71 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 11 Sep 2024 13:34:23 -0400 Subject: [PATCH 05/15] r/aws_mq_broker: fix import lints --- internal/service/mq/broker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index dd4739577f1..e84ef065e05 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "golang.org/x/mod/semver" "log" "reflect" "strconv" @@ -37,6 +36,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" "github.com/mitchellh/copystructure" + "golang.org/x/mod/semver" ) // @SDKResource("aws_mq_broker", name="Broker") From 73df9e263107ca739df8a79d2a897ce28eb560d1 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 11 Sep 2024 15:55:53 -0400 Subject: [PATCH 06/15] r/aws_mq_broker(test): tidy normalized engine version tests --- internal/service/mq/broker_test.go | 86 ++++++++---------------------- 1 file changed, 23 insertions(+), 63 deletions(-) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 0467f23cca0..f16b31d5107 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -241,11 +241,12 @@ func TestDiffUsers(t *testing.T) { } const ( - testAccBrokerVersionNewer = "5.17.6" // before changing, check b/c must be valid on GovCloud - testAccBrokerVersionOlder = "5.16.7" // before changing, check b/c must be valid on GovCloud - testAccActiveVersionSimplified = "5.18" - testAccRabbitVersion = "3.11.20" // before changing, check b/c must be valid on GovCloud - testAccRabbitVersionSimplified = "3.13" + testAccBrokerVersionNewer = "5.17.6" // before changing, check b/c must be valid on GovCloud + testAccBrokerVersionOlder = "5.16.7" // before changing, check b/c must be valid on GovCloud + testAccRabbitVersion = "3.11.20" // before changing, check b/c must be valid on GovCloud + + testAccActiveVersionNormalized5_18 = "5.18" + testAccRabbitVersionNormalized3_13 = "3.13" ) func TestAccMQBroker_basic(t *testing.T) { @@ -327,7 +328,7 @@ func TestAccMQBroker_basic(t *testing.T) { }) } -func TestAccMQBroker_basic_simplified_version(t *testing.T) { +func TestAccMQBroker_normalizedEngineVersion(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -348,52 +349,15 @@ func TestAccMQBroker_basic_simplified_version(t *testing.T) { CheckDestroy: testAccCheckBrokerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccBrokerConfig_basic_autoMinorVersionUpgrade(rName, testAccActiveVersionSimplified, true), + Config: testAccBrokerConfig_autoMinorVersionUpgrade(rName, testAccActiveVersionNormalized5_18, true), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), - acctest.MatchResourceAttrRegionalARN(resourceName, names.AttrARN, "mq", regexache.MustCompile(`broker:+.`)), resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), - resource.TestCheckResourceAttr(resourceName, "authentication_strategy", "simple"), - resource.TestCheckResourceAttr(resourceName, "broker_name", rName), - resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), - resource.TestMatchResourceAttr(resourceName, "configuration.0.id", regexache.MustCompile(`^c-[0-9a-z-]+$`)), - resource.TestMatchResourceAttr(resourceName, "configuration.0.revision", regexache.MustCompile(`^[0-9]+$`)), - resource.TestCheckResourceAttr(resourceName, "deployment_mode", "SINGLE_INSTANCE"), - resource.TestCheckResourceAttr(resourceName, "encryption_options.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "encryption_options.0.use_aws_owned_key", acctest.CtTrue), resource.TestCheckResourceAttr(resourceName, "engine_type", "ActiveMQ"), - resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccActiveVersionSimplified), - resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t2.micro"), - resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), - resource.TestMatchResourceAttr(resourceName, "instances.0.console_url", - regexache.MustCompile(`^https://[0-9a-f-]+\.mq.[0-9a-z-]+.amazonaws.com:8162$`)), - resource.TestCheckResourceAttr(resourceName, "instances.0.endpoints.#", "5"), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.0", regexache.MustCompile(`^ssl://[0-9a-z.-]+:61617$`)), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.1", regexache.MustCompile(`^amqp\+ssl://[0-9a-z.-]+:5671$`)), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.2", regexache.MustCompile(`^stomp\+ssl://[0-9a-z.-]+:61614$`)), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.3", regexache.MustCompile(`^mqtt\+ssl://[0-9a-z.-]+:8883$`)), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.4", regexache.MustCompile(`^wss://[0-9a-z.-]+:61619$`)), - resource.TestMatchResourceAttr(resourceName, "instances.0.ip_address", - regexache.MustCompile(`^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$`)), - resource.TestCheckResourceAttr(resourceName, "maintenance_window_start_time.#", acctest.Ct1), - resource.TestCheckResourceAttrSet(resourceName, "maintenance_window_start_time.0.day_of_week"), - resource.TestCheckResourceAttrSet(resourceName, "maintenance_window_start_time.0.time_of_day"), - resource.TestCheckResourceAttr(resourceName, "logs.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "logs.0.general", acctest.CtTrue), - resource.TestCheckResourceAttr(resourceName, "logs.0.audit", acctest.CtFalse), - resource.TestCheckResourceAttr(resourceName, "maintenance_window_start_time.0.time_zone", "UTC"), - resource.TestCheckResourceAttr(resourceName, names.AttrPubliclyAccessible, acctest.CtFalse), - resource.TestCheckResourceAttr(resourceName, "security_groups.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, names.AttrStorageType, "efs"), - resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, acctest.Ct0), - resource.TestCheckResourceAttr(resourceName, "user.#", acctest.Ct1), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "user.*", map[string]string{ - "console_access": acctest.CtFalse, - "groups.#": acctest.Ct0, - names.AttrUsername: "Test", - names.AttrPassword: "TestTest1234", - }), + // Starting in v5.18, the engine version should be normalized to remove + // the patch version as AWS automatically handles patch updates when + // `auto_minor_version_upgrade` is enabled. + resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccActiveVersionNormalized5_18), ), }, { @@ -1207,7 +1171,7 @@ func TestAccMQBroker_RabbitMQ_basic(t *testing.T) { }) } -func TestAccMQBroker_RabbitMQ_basic_with_auto_minor_version_upgrade(t *testing.T) { +func TestAccMQBroker_RabbitMQ_autoMinorVersionUpgrade(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1228,11 +1192,12 @@ func TestAccMQBroker_RabbitMQ_basic_with_auto_minor_version_upgrade(t *testing.T CheckDestroy: testAccCheckBrokerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, testAccRabbitVersion, true), + Config: testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, testAccRabbitVersion, true), Check: resource.ComposeTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), resource.TestCheckResourceAttr(resourceName, "engine_type", "RabbitMQ"), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccRabbitVersion), resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t3.micro"), resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), @@ -1253,7 +1218,7 @@ func TestAccMQBroker_RabbitMQ_basic_with_auto_minor_version_upgrade(t *testing.T }) } -func TestAccMQBroker_RabbitMQ_basic_simplified_version(t *testing.T) { +func TestAccMQBroker_RabbitMQ_normalizedEngineVersion(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { t.Skip("skipping long-running test in short mode") @@ -1274,20 +1239,15 @@ func TestAccMQBroker_RabbitMQ_basic_simplified_version(t *testing.T) { CheckDestroy: testAccCheckBrokerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, testAccRabbitVersionSimplified, true), + Config: testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, testAccRabbitVersionNormalized3_13, true), Check: resource.ComposeTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), - resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), resource.TestCheckResourceAttr(resourceName, "engine_type", "RabbitMQ"), - resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccRabbitVersionSimplified), - resource.TestCheckResourceAttr(resourceName, "host_instance_type", "mq.t3.micro"), - resource.TestCheckResourceAttr(resourceName, "instances.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "instances.0.endpoints.#", acctest.Ct1), - resource.TestMatchResourceAttr(resourceName, "instances.0.endpoints.0", regexache.MustCompile(`^amqps://[0-9a-z.-]+:5671$`)), - resource.TestCheckResourceAttr(resourceName, "logs.#", acctest.Ct1), - resource.TestCheckResourceAttr(resourceName, "logs.0.general", acctest.CtFalse), - resource.TestCheckResourceAttr(resourceName, "logs.0.audit", ""), + // Starting in v3.13, the engine version should be normalized to remove + // the patch version as AWS automatically handles patch updates when + // `auto_minor_version_upgrade` is enabled. + resource.TestCheckResourceAttr(resourceName, names.AttrEngineVersion, testAccRabbitVersionNormalized3_13), ), }, { @@ -1776,7 +1736,7 @@ resource "aws_mq_broker" "test" { `, rName, version) } -func testAccBrokerConfig_basic_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { +func testAccBrokerConfig_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { return fmt.Sprintf(` resource "aws_security_group" "test" { name = %[1]q @@ -2352,7 +2312,7 @@ resource "aws_mq_broker" "test" { `, rName, version) } -func testAccBrokerConfig_rabbit_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { +func testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { return fmt.Sprintf(` resource "aws_security_group" "test" { name = %[1]q From 16b4fb5e4bd50036030a551ebcab92afdccc05a3 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 11 Sep 2024 16:47:19 -0400 Subject: [PATCH 07/15] r/aws_mq_broker(test): fix test config name --- internal/service/mq/broker_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index f16b31d5107..0dabc143760 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -1192,7 +1192,7 @@ func TestAccMQBroker_RabbitMQ_autoMinorVersionUpgrade(t *testing.T) { CheckDestroy: testAccCheckBrokerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, testAccRabbitVersion, true), + Config: testAccBrokerConfig_rabbitAutoMinorVersionUpgrade(rName, testAccRabbitVersion, true), Check: resource.ComposeTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), resource.TestCheckResourceAttr(resourceName, "configuration.#", acctest.Ct1), @@ -1239,7 +1239,7 @@ func TestAccMQBroker_RabbitMQ_normalizedEngineVersion(t *testing.T) { CheckDestroy: testAccCheckBrokerDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, testAccRabbitVersionNormalized3_13, true), + Config: testAccBrokerConfig_rabbitAutoMinorVersionUpgrade(rName, testAccRabbitVersionNormalized3_13, true), Check: resource.ComposeTestCheckFunc( testAccCheckBrokerExists(ctx, resourceName, &broker), resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), @@ -2312,7 +2312,7 @@ resource "aws_mq_broker" "test" { `, rName, version) } -func testAccBrokerConfig_RabbitMQ_autoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { +func testAccBrokerConfig_rabbitAutoMinorVersionUpgrade(rName, version string, autoMinorVersionUpgrade bool) string { return fmt.Sprintf(` resource "aws_security_group" "test" { name = %[1]q From 51e25a3376f7160c59b772dcf128de6c58bdf5d6 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Wed, 11 Sep 2024 16:51:58 -0400 Subject: [PATCH 08/15] r/aws_mq_broker: adjust normalization function args, unit test ```console % make testacc PKG=mq TESTS=TestAccMQBroker_normalizedEngineVersion make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.6 test ./internal/service/mq/... -v -count 1 -parallel 20 -run='TestAccMQBroker_normalizedEngineVersion' -timeout 360m --- PASS: TestAccMQBroker_normalizedEngineVersion (973.99s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 980.237s ``` ```console % go test -count=1 ./internal/service/mq/... -run=TestNormalizeEngineVersion ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 6.269s ``` --- internal/service/mq/broker.go | 59 ++++++++++++++----- internal/service/mq/broker_test.go | 91 +++++++++++++++++++++++++++++ internal/service/mq/exports_test.go | 2 + 3 files changed, 136 insertions(+), 16 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index e84ef065e05..127871ba26a 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -463,20 +463,6 @@ func resourceBrokerCreate(ctx context.Context, d *schema.ResourceData, meta inte return append(diags, resourceBrokerRead(ctx, d, meta)...) } -func simplifiedVersion(engineType types.EngineType, autoMinorVersionUpgrade *bool, engineVersion *string) string { - if autoMinorVersionUpgrade == nil || !*autoMinorVersionUpgrade { - return *engineVersion - } - majorMinorEngineVersion := semver.MajorMinor("v" + *engineVersion) - rabbitSimplifiedVersion := "v3.13" - activeSimplifiedVersion := "v5.18" - if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, rabbitSimplifiedVersion) == 0) || - (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, activeSimplifiedVersion) == 0) { - return majorMinorEngineVersion[1:] - } - return *engineVersion -} - func resourceBrokerRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics @@ -501,7 +487,7 @@ func resourceBrokerRead(ctx context.Context, d *schema.ResourceData, meta interf d.Set("data_replication_mode", output.DataReplicationMode) d.Set("deployment_mode", output.DeploymentMode) d.Set("engine_type", output.EngineType) - d.Set(names.AttrEngineVersion, simplifiedVersion(output.EngineType, output.AutoMinorVersionUpgrade, output.EngineVersion)) + d.Set(names.AttrEngineVersion, normalizeEngineVersion(output)) d.Set("host_instance_type", output.HostInstanceType) d.Set("instances", flattenBrokerInstances(output.BrokerInstances)) d.Set("pending_data_replication_mode", output.PendingDataReplicationMode) @@ -575,10 +561,11 @@ func resourceBrokerUpdate(ctx context.Context, d *schema.ResourceData, meta inte if errors != nil { return sdkdiag.AppendErrorf(diags, "reading MQ Broker (%s): %s", d.Id(), errors) } + input := &mq.UpdateBrokerInput{ BrokerId: aws.String(d.Id()), Configuration: expandConfigurationId(d.Get(names.AttrConfiguration).([]interface{})), - EngineVersion: aws.String(simplifiedVersion(output.EngineType, output.AutoMinorVersionUpgrade, output.EngineVersion)), + EngineVersion: aws.String(normalizeEngineVersion(output)), Logs: expandLogs(d.Get("engine_type").(string), d.Get("logs").([]interface{})), } @@ -931,6 +918,46 @@ func DiffBrokerUsers(bId string, oldUsers, newUsers []interface{}) (cr []*mq.Cre return cr, di, ur, nil } +// normalizeEngineVersion normalizes the engine version depending on whether auto +// minor version upgrades are enabled +// +// Beginning with RabbitMQ v3.13 and ActiveMQ 5.18, brokers with `auto_minor_version_upgrade` +// set to `true` will automatically receive patch updates during scheduled maintenance +// windows. To account for automated changes to patch versions, the returned engine +// value is normalized to only major/minor when these conditions are met. +// +// If `auto_minor_version_upgrade` is not enabled, or the engine version is less than +// the version in which AWS began automatically applying patch upgrades, the engine +// version is returned unmodified. +// +// Ref: https://docs.aws.amazon.com/amazon-mq/latest/developer-guide/upgrading-brokers.html +func normalizeEngineVersion(output *mq.DescribeBrokerOutput) string { + if output == nil { + return "" + } + + autoMinorVersionUpgrade := aws.ToBool(output.AutoMinorVersionUpgrade) + engineType := output.EngineType + engineVersion := aws.ToString(output.EngineVersion) + majorMinorEngineVersion := semver.MajorMinor("v" + engineVersion) + + // initial versions where `auto_minor_version_upgrade` triggers automatic + // patch updates, and only the major/minor should be supplied to the update API + initRabbitVersion := "v3.13" + initActiveVersion := "v5.18" + + if !autoMinorVersionUpgrade { + return engineVersion + } + + if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, initRabbitVersion) == 0) || + (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, initActiveVersion) == 0) { + return majorMinorEngineVersion[1:] + } + + return engineVersion +} + func expandEncryptionOptions(l []interface{}) *types.EncryptionOptions { if len(l) == 0 || l[0] == nil { return nil diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 0dabc143760..241bf1ca4ef 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -240,6 +240,97 @@ func TestDiffUsers(t *testing.T) { } } +func TestNormalizeEngineVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + output *mq.DescribeBrokerOutput + want string + }{ + {"nil", nil, ""}, // should never happen in practice, but ensure no panic + { + "RabbitMQ, no auto, pre 3.13", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeRabbitmq, + EngineVersion: aws.String("3.12.0"), + AutoMinorVersionUpgrade: aws.Bool(false), + }, + "3.12.0", + }, + { + "RabbitMQ, auto, pre 3.13", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeRabbitmq, + EngineVersion: aws.String("3.12.0"), + AutoMinorVersionUpgrade: aws.Bool(true), + }, + "3.12.0", + }, + { + "RabbitMQ, no auto, post 3.13", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeRabbitmq, + EngineVersion: aws.String("3.13.2"), + AutoMinorVersionUpgrade: aws.Bool(false), + }, + "3.13.2", + }, + { + "RabbitMQ, auto, post 3.13", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeRabbitmq, + EngineVersion: aws.String("3.13.2"), + AutoMinorVersionUpgrade: aws.Bool(true), + }, + "3.13", + }, + { + "ActiveMQ, no auto, pre 5.18", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeActivemq, + EngineVersion: aws.String("5.17.0"), + AutoMinorVersionUpgrade: aws.Bool(false), + }, + "5.17.0", + }, + { + "ActiveMQ, auto, pre 5.18", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeActivemq, + EngineVersion: aws.String("5.17.0"), + AutoMinorVersionUpgrade: aws.Bool(true), + }, + "5.17.0", + }, + { + "ActiveMQ, no auto, post 5.18", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeActivemq, + EngineVersion: aws.String("5.18.2"), + AutoMinorVersionUpgrade: aws.Bool(false), + }, + "5.18.2", + }, + { + "ActiveMQ, auto, post 5.18", + &mq.DescribeBrokerOutput{ + EngineType: types.EngineTypeActivemq, + EngineVersion: aws.String("5.18.2"), + AutoMinorVersionUpgrade: aws.Bool(true), + }, + "5.18", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tfmq.NormalizeEngineVersion(tt.output); got != tt.want { + t.Errorf("NormalizeEngineVersion() = %v, want %v", got, tt.want) + } + }) + } +} + const ( testAccBrokerVersionNewer = "5.17.6" // before changing, check b/c must be valid on GovCloud testAccBrokerVersionOlder = "5.16.7" // before changing, check b/c must be valid on GovCloud diff --git a/internal/service/mq/exports_test.go b/internal/service/mq/exports_test.go index 6d71465acf5..f727b6b4e5f 100644 --- a/internal/service/mq/exports_test.go +++ b/internal/service/mq/exports_test.go @@ -11,6 +11,8 @@ var ( FindBrokerByID = findBrokerByID FindConfigurationByID = findConfigurationByID + NormalizeEngineVersion = normalizeEngineVersion + WaitBrokerRebooted = waitBrokerRebooted WaitBrokerDeleted = waitBrokerDeleted ) From 391e82f3a285b668833221095912994abdbf447d Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 10:09:42 -0400 Subject: [PATCH 09/15] r/aws_mq_broker(test): parallel unit test lint --- internal/service/mq/broker_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 241bf1ca4ef..5c0ef3fa9b2 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -324,6 +324,7 @@ func TestNormalizeEngineVersion(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() if got := tfmq.NormalizeEngineVersion(tt.output); got != tt.want { t.Errorf("NormalizeEngineVersion() = %v, want %v", got, tt.want) } From 567daf6633303a3c8d812a73bb496bc95c27289a Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 10:11:00 -0400 Subject: [PATCH 10/15] r/aws_mq_broker: compare engine versions with greater or equal --- internal/service/mq/broker.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index 127871ba26a..e9ec6362ac4 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -921,7 +921,7 @@ func DiffBrokerUsers(bId string, oldUsers, newUsers []interface{}) (cr []*mq.Cre // normalizeEngineVersion normalizes the engine version depending on whether auto // minor version upgrades are enabled // -// Beginning with RabbitMQ v3.13 and ActiveMQ 5.18, brokers with `auto_minor_version_upgrade` +// Beginning with RabbitMQ 3.13 and ActiveMQ 5.18, brokers with `auto_minor_version_upgrade` // set to `true` will automatically receive patch updates during scheduled maintenance // windows. To account for automated changes to patch versions, the returned engine // value is normalized to only major/minor when these conditions are met. @@ -950,8 +950,8 @@ func normalizeEngineVersion(output *mq.DescribeBrokerOutput) string { return engineVersion } - if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, initRabbitVersion) == 0) || - (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, initActiveVersion) == 0) { + if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, initRabbitVersion) >= 0) || + (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, initActiveVersion) >= 0) { return majorMinorEngineVersion[1:] } From ca45ed936bcdc684d4532dd5a77b1b52c21a6189 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 10:12:43 -0400 Subject: [PATCH 11/15] chore: changelog --- .changelog/39024.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/39024.txt diff --git a/.changelog/39024.txt b/.changelog/39024.txt new file mode 100644 index 00000000000..8cdbd83dffc --- /dev/null +++ b/.changelog/39024.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_mq_broker: Fix `engine_version` mismatch with RabbitMQ 3.13 and ActiveMQ 5.18 and above +``` From 01ab5f8c1830471458d349eb600e17e518eed54d Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 13:23:46 -0400 Subject: [PATCH 12/15] r/aws_mq_broker: tidy normalizeEngineVersion --- internal/service/mq/broker.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index e9ec6362ac4..9c2e102a850 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -937,22 +937,22 @@ func normalizeEngineVersion(output *mq.DescribeBrokerOutput) string { } autoMinorVersionUpgrade := aws.ToBool(output.AutoMinorVersionUpgrade) - engineType := output.EngineType + engineType := string(output.EngineType) engineVersion := aws.ToString(output.EngineVersion) - majorMinorEngineVersion := semver.MajorMinor("v" + engineVersion) + majorMinor := semver.MajorMinor("v" + engineVersion) // initial versions where `auto_minor_version_upgrade` triggers automatic // patch updates, and only the major/minor should be supplied to the update API - initRabbitVersion := "v3.13" - initActiveVersion := "v5.18" + minRabbit := "v3.13" + minActive := "v5.18" if !autoMinorVersionUpgrade { return engineVersion } - if (strings.EqualFold(string(engineType), string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinorEngineVersion, initRabbitVersion) >= 0) || - (strings.EqualFold(string(engineType), string(types.EngineTypeActivemq)) && semver.Compare(majorMinorEngineVersion, initActiveVersion) >= 0) { - return majorMinorEngineVersion[1:] + if (strings.EqualFold(engineType, string(types.EngineTypeRabbitmq)) && semver.Compare(majorMinor, minRabbit) >= 0) || + (strings.EqualFold(engineType, string(types.EngineTypeActivemq)) && semver.Compare(majorMinor, minActive) >= 0) { + return majorMinor[1:] } return engineVersion From b189df93d571b24ac266f014b0f427ae8c438a71 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 13:29:23 -0400 Subject: [PATCH 13/15] chore: make clean-tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 5b8c85d7d36..bb0b57daca7 100644 --- a/go.mod +++ b/go.mod @@ -290,6 +290,7 @@ require ( github.com/pquerna/otp v1.4.0 github.com/shopspring/decimal v1.4.0 golang.org/x/crypto v0.27.0 + golang.org/x/mod v0.20.0 golang.org/x/text v0.18.0 golang.org/x/tools v0.24.0 gopkg.in/dnaeon/go-vcr.v3 v3.2.1 @@ -357,7 +358,6 @@ require ( go.opentelemetry.io/otel v1.30.0 // indirect go.opentelemetry.io/otel/metric v1.30.0 // indirect go.opentelemetry.io/otel/trace v1.30.0 // indirect - golang.org/x/mod v0.20.0 // indirect golang.org/x/net v0.29.0 // indirect golang.org/x/sync v0.8.0 // indirect golang.org/x/sys v0.25.0 // indirect From 7ce13851fc1754213e6e1494afcc71e63f7d4642 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 15:43:20 -0400 Subject: [PATCH 14/15] r/aws_mq_broker: fix engine_version update logic Previously the `engine_version` sent to the update API was the version read from the remote resource, rather than the configured value. This change passes the configured values for `engine_type`, `engine_version`, and `auto_minor_version_upgrade` to the `normalizeEngineVersion` function instead, allowing explicitly configured engine version updates to once again function properly. ```console % TF_ACC_TERRAFORM_VERSION=1.7.5 make testacc PKG=mq TESTS="TestAccMQBroker_" make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.22.6 test ./internal/service/mq/... -v -count 1 -parallel 20 -run='TestAccMQBroker_' -timeout 360m --- PASS: TestAccMQBroker_RabbitMQ_validationAuditLog (631.07s) === CONT TestAccMQBroker_normalizedEngineVersion --- PASS: TestAccMQBroker_RabbitMQ_normalizedEngineVersion (647.54s) === CONT TestAccMQBroker_RabbitMQ_basic --- PASS: TestAccMQBroker_RabbitMQ_autoMinorVersionUpgrade (659.43s) === CONT TestAccMQBroker_Update_hostInstanceType --- PASS: TestAccMQBroker_RabbitMQ_config (662.56s) --- PASS: TestAccMQBroker_RabbitMQ_cluster (669.10s) --- PASS: TestAccMQBroker_RabbitMQ_logs (672.09s) --- PASS: TestAccMQBroker_throughputOptimized (939.31s) --- PASS: TestAccMQBroker_ldap (953.94s) --- PASS: TestAccMQBroker_EncryptionOptions_kmsKeyID (989.45s) --- PASS: TestAccMQBroker_EncryptionOptions_managedKeyEnabled (1038.29s) --- PASS: TestAccMQBroker_EncryptionOptions_managedKeyDisabled (1043.15s) --- PASS: TestAccMQBroker_disappears (1060.67s) --- PASS: TestAccMQBroker_basic (1064.66s) --- PASS: TestAccMQBroker_tags (1078.43s) --- PASS: TestAccMQBroker_Update_engineVersion (1240.34s) --- PASS: TestAccMQBroker_Update_securityGroup (1262.55s) --- PASS: TestAccMQBroker_RabbitMQ_basic (660.77s) --- PASS: TestAccMQBroker_Update_users (1429.44s) --- PASS: TestAccMQBroker_normalizedEngineVersion (843.55s) --- PASS: TestAccMQBroker_AllFields_defaultVPC (1593.85s) --- PASS: TestAccMQBroker_AllFields_customVPC (1740.74s) --- PASS: TestAccMQBroker_Update_hostInstanceType (1353.62s) --- PASS: TestAccMQBroker_dataReplicationMode (2095.33s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 2101.590s ``` --- internal/service/mq/broker.go | 23 +++---- internal/service/mq/broker_test.go | 107 +++++++++++++---------------- 2 files changed, 54 insertions(+), 76 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index 9c2e102a850..14982fd3eab 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -487,7 +487,7 @@ func resourceBrokerRead(ctx context.Context, d *schema.ResourceData, meta interf d.Set("data_replication_mode", output.DataReplicationMode) d.Set("deployment_mode", output.DeploymentMode) d.Set("engine_type", output.EngineType) - d.Set(names.AttrEngineVersion, normalizeEngineVersion(output)) + d.Set(names.AttrEngineVersion, normalizeEngineVersion(string(output.EngineType), aws.ToString(output.EngineVersion), aws.ToBool(output.AutoMinorVersionUpgrade))) d.Set("host_instance_type", output.HostInstanceType) d.Set("instances", flattenBrokerInstances(output.BrokerInstances)) d.Set("pending_data_replication_mode", output.PendingDataReplicationMode) @@ -557,16 +557,15 @@ func resourceBrokerUpdate(ctx context.Context, d *schema.ResourceData, meta inte } if d.HasChanges(names.AttrConfiguration, "logs", names.AttrEngineVersion) { - output, errors := findBrokerByID(ctx, conn, d.Id()) - if errors != nil { - return sdkdiag.AppendErrorf(diags, "reading MQ Broker (%s): %s", d.Id(), errors) - } + engineType := d.Get("engine_type").(string) + engineVersion := d.Get("engine_version").(string) + autoMinorVersionUpgrade := d.Get("auto_minor_version_upgrade").(bool) input := &mq.UpdateBrokerInput{ BrokerId: aws.String(d.Id()), Configuration: expandConfigurationId(d.Get(names.AttrConfiguration).([]interface{})), - EngineVersion: aws.String(normalizeEngineVersion(output)), - Logs: expandLogs(d.Get("engine_type").(string), d.Get("logs").([]interface{})), + EngineVersion: aws.String(normalizeEngineVersion(engineType, engineVersion, autoMinorVersionUpgrade)), + Logs: expandLogs(engineType, d.Get("logs").([]interface{})), } _, err := conn.UpdateBroker(ctx, input) @@ -931,14 +930,8 @@ func DiffBrokerUsers(bId string, oldUsers, newUsers []interface{}) (cr []*mq.Cre // version is returned unmodified. // // Ref: https://docs.aws.amazon.com/amazon-mq/latest/developer-guide/upgrading-brokers.html -func normalizeEngineVersion(output *mq.DescribeBrokerOutput) string { - if output == nil { - return "" - } - - autoMinorVersionUpgrade := aws.ToBool(output.AutoMinorVersionUpgrade) - engineType := string(output.EngineType) - engineVersion := aws.ToString(output.EngineVersion) +// func normalizeEngineVersion(output *mq.DescribeBrokerOutput) string { +func normalizeEngineVersion(engineType string, engineVersion string, autoMinorVersionUpgrade bool) string { majorMinor := semver.MajorMinor("v" + engineVersion) // initial versions where `auto_minor_version_upgrade` triggers automatic diff --git a/internal/service/mq/broker_test.go b/internal/service/mq/broker_test.go index 5c0ef3fa9b2..d81ed1ef4c3 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -244,88 +244,73 @@ func TestNormalizeEngineVersion(t *testing.T) { t.Parallel() tests := []struct { - name string - output *mq.DescribeBrokerOutput - want string + name string + engineType string + engineVersion string + autoMinorVersionUpgrade bool + want string }{ - {"nil", nil, ""}, // should never happen in practice, but ensure no panic { - "RabbitMQ, no auto, pre 3.13", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeRabbitmq, - EngineVersion: aws.String("3.12.0"), - AutoMinorVersionUpgrade: aws.Bool(false), - }, - "3.12.0", + name: "RabbitMQ, no auto, pre 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.12.0", + autoMinorVersionUpgrade: false, + want: "3.12.0", }, { - "RabbitMQ, auto, pre 3.13", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeRabbitmq, - EngineVersion: aws.String("3.12.0"), - AutoMinorVersionUpgrade: aws.Bool(true), - }, - "3.12.0", + name: "RabbitMQ, auto, pre 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.12.0", + autoMinorVersionUpgrade: true, + want: "3.12.0", }, { - "RabbitMQ, no auto, post 3.13", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeRabbitmq, - EngineVersion: aws.String("3.13.2"), - AutoMinorVersionUpgrade: aws.Bool(false), - }, - "3.13.2", + name: "RabbitMQ, no auto, post 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.13.2", + autoMinorVersionUpgrade: false, + want: "3.13.2", }, { - "RabbitMQ, auto, post 3.13", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeRabbitmq, - EngineVersion: aws.String("3.13.2"), - AutoMinorVersionUpgrade: aws.Bool(true), - }, - "3.13", + name: "RabbitMQ, auto, post 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.13.2", + autoMinorVersionUpgrade: true, + want: "3.13", }, { - "ActiveMQ, no auto, pre 5.18", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeActivemq, - EngineVersion: aws.String("5.17.0"), - AutoMinorVersionUpgrade: aws.Bool(false), - }, - "5.17.0", + name: "ActiveMQ, no auto, pre 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.17.0", + autoMinorVersionUpgrade: false, + want: "5.17.0", }, { - "ActiveMQ, auto, pre 5.18", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeActivemq, - EngineVersion: aws.String("5.17.0"), - AutoMinorVersionUpgrade: aws.Bool(true), - }, - "5.17.0", + name: "ActiveMQ, auto, pre 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.17.0", + autoMinorVersionUpgrade: true, + want: "5.17.0", }, { - "ActiveMQ, no auto, post 5.18", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeActivemq, - EngineVersion: aws.String("5.18.2"), - AutoMinorVersionUpgrade: aws.Bool(false), - }, - "5.18.2", + name: "ActiveMQ, no auto, post 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.18.2", + autoMinorVersionUpgrade: false, + want: "5.18.2", }, { - "ActiveMQ, auto, post 5.18", - &mq.DescribeBrokerOutput{ - EngineType: types.EngineTypeActivemq, - EngineVersion: aws.String("5.18.2"), - AutoMinorVersionUpgrade: aws.Bool(true), - }, - "5.18", + name: "ActiveMQ, auto, post 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.18.2", + autoMinorVersionUpgrade: true, + want: "5.18", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := tfmq.NormalizeEngineVersion(tt.output); got != tt.want { + if got := tfmq.NormalizeEngineVersion(tt.engineType, tt.engineVersion, tt.autoMinorVersionUpgrade); got != tt.want { t.Errorf("NormalizeEngineVersion() = %v, want %v", got, tt.want) } }) From cda3a9f2cf80c8274b04b5d3ddd9dd02be3afade Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Thu, 12 Sep 2024 20:38:31 -0400 Subject: [PATCH 15/15] chore: make fix-constants --- internal/service/mq/broker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index 14982fd3eab..c2434c454af 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -558,8 +558,8 @@ func resourceBrokerUpdate(ctx context.Context, d *schema.ResourceData, meta inte if d.HasChanges(names.AttrConfiguration, "logs", names.AttrEngineVersion) { engineType := d.Get("engine_type").(string) - engineVersion := d.Get("engine_version").(string) - autoMinorVersionUpgrade := d.Get("auto_minor_version_upgrade").(bool) + engineVersion := d.Get(names.AttrEngineVersion).(string) + autoMinorVersionUpgrade := d.Get(names.AttrAutoMinorVersionUpgrade).(bool) input := &mq.UpdateBrokerInput{ BrokerId: aws.String(d.Id()),