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 +``` 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 diff --git a/internal/service/mq/broker.go b/internal/service/mq/broker.go index cae69098a26..c2434c454af 100644 --- a/internal/service/mq/broker.go +++ b/internal/service/mq/broker.go @@ -36,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") @@ -486,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, output.EngineVersion) + 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) @@ -556,11 +557,15 @@ 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(names.AttrEngineVersion).(string) + autoMinorVersionUpgrade := d.Get(names.AttrAutoMinorVersionUpgrade).(bool) + input := &mq.UpdateBrokerInput{ BrokerId: aws.String(d.Id()), Configuration: expandConfigurationId(d.Get(names.AttrConfiguration).([]interface{})), - EngineVersion: aws.String(d.Get(names.AttrEngineVersion).(string)), - 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) @@ -912,6 +917,40 @@ 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 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. +// +// 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 { +func normalizeEngineVersion(engineType string, engineVersion string, autoMinorVersionUpgrade bool) string { + 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 + minRabbit := "v3.13" + minActive := "v5.18" + + if !autoMinorVersionUpgrade { + return engineVersion + } + + 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 +} + 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 93ab9f530e5..d81ed1ef4c3 100644 --- a/internal/service/mq/broker_test.go +++ b/internal/service/mq/broker_test.go @@ -240,10 +240,90 @@ func TestDiffUsers(t *testing.T) { } } +func TestNormalizeEngineVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + engineType string + engineVersion string + autoMinorVersionUpgrade bool + want string + }{ + { + name: "RabbitMQ, no auto, pre 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.12.0", + autoMinorVersionUpgrade: false, + want: "3.12.0", + }, + { + name: "RabbitMQ, auto, pre 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.12.0", + autoMinorVersionUpgrade: true, + want: "3.12.0", + }, + { + name: "RabbitMQ, no auto, post 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.13.2", + autoMinorVersionUpgrade: false, + want: "3.13.2", + }, + { + name: "RabbitMQ, auto, post 3.13", + engineType: string(types.EngineTypeRabbitmq), + engineVersion: "3.13.2", + autoMinorVersionUpgrade: true, + want: "3.13", + }, + { + name: "ActiveMQ, no auto, pre 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.17.0", + autoMinorVersionUpgrade: false, + want: "5.17.0", + }, + { + name: "ActiveMQ, auto, pre 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.17.0", + autoMinorVersionUpgrade: true, + want: "5.17.0", + }, + { + name: "ActiveMQ, no auto, post 5.18", + engineType: string(types.EngineTypeActivemq), + engineVersion: "5.18.2", + autoMinorVersionUpgrade: false, + want: "5.18.2", + }, + { + 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.engineType, tt.engineVersion, tt.autoMinorVersionUpgrade); 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 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) { @@ -325,6 +405,48 @@ func TestAccMQBroker_basic(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") + } + + 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_autoMinorVersionUpgrade(rName, testAccActiveVersionNormalized5_18, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBrokerExists(ctx, resourceName, &broker), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "engine_type", "ActiveMQ"), + // 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), + ), + }, + { + 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 +1248,95 @@ func TestAccMQBroker_RabbitMQ_basic(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") + } + + 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_rabbitAutoMinorVersionUpgrade(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), + 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_normalizedEngineVersion(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_rabbitAutoMinorVersionUpgrade(rName, testAccRabbitVersionNormalized3_13, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckBrokerExists(ctx, resourceName, &broker), + resource.TestCheckResourceAttr(resourceName, names.AttrAutoMinorVersionUpgrade, acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "engine_type", "RabbitMQ"), + // 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), + ), + }, + { + 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 +1813,38 @@ resource "aws_mq_broker" "test" { `, rName, version) } +func testAccBrokerConfig_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 +2389,32 @@ resource "aws_mq_broker" "test" { `, rName, version) } +func testAccBrokerConfig_rabbitAutoMinorVersionUpgrade(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" { 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 )