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.

Sign up for GitHub

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

Jump to bottom

fix: RabbitMQ 3.13 and ActiveMQ 5.18 Version Mismatch #39024

Merged
merged 17 commits into from
Sep 13, 2024
Merged

fix: RabbitMQ 3.13 and ActiveMQ 5.18 Version Mismatch #39024

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
73fcefe
fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.1…
architec Aug 22, 2024
635d825
fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.1…
architec Aug 22, 2024
3e775e7
fix: fixed bug for aws_mq_broker that simplified version RabbitMQ 3.1…
architec Aug 22, 2024
4038f6c
Merge branch 'main' into b-mq-simplified-version-auto-minor-version-u…
jar-b Sep 11, 2024
8331fda
chore: make testacc-lint-fix
jar-b Sep 11, 2024
9501238
r/aws_mq_broker: fix import lints
jar-b Sep 11, 2024
73df9e2
r/aws_mq_broker(test): tidy normalized engine version tests
jar-b Sep 11, 2024
16b4fb5
r/aws_mq_broker(test): fix test config name
jar-b Sep 11, 2024
51e25a3
r/aws_mq_broker: adjust normalization function args, unit test
jar-b Sep 11, 2024
391e82f
r/aws_mq_broker(test): parallel unit test lint
jar-b Sep 12, 2024
567daf6
r/aws_mq_broker: compare engine versions with greater or equal
jar-b Sep 12, 2024
ca45ed9
chore: changelog
jar-b Sep 12, 2024
01ab5f8
r/aws_mq_broker: tidy normalizeEngineVersion
jar-b Sep 12, 2024
d9c4feb
Merge branch 'main' into b-mq-simplified-version-auto-minor-version-u…
jar-b Sep 12, 2024
b189df9
chore: make clean-tidy
jar-b Sep 12, 2024
7ce1385
r/aws_mq_broker: fix engine_version update logic
jar-b Sep 12, 2024
cda3a9f
chore: make fix-constants
jar-b Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions internal/service/mq/broker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) HashiCorp, Inc.

Check failure on line 1 in internal/service/mq/broker.go

View workflow job for this annotation

GitHub Actions / import-lint 

Imports of different types are not allowed in the same group (0): 	"bytes" != 	"golang.org/x/mod/semver"
// SPDX-License-Identifier: MPL-2.0

package mq
Expand All @@ -8,6 +8,7 @@
"context"
"errors"
"fmt"
"golang.org/x/mod/semver"
"log"
"reflect"
"strconv"
Expand Down Expand Up @@ -462,6 +463,20 @@
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"
Copy link

Choose a reason for hiding this comment

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

Thanks for picking this up @architec as I was looking at this issue as well. With this setup don't we have to update the code when AWS releases a new version? Also I was looking at how the RDS service had it working and setting up an actual_engine_version attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, this PR will necessitate code updates when Amazon MQ releases new versions. However, I can submit a subsequent pull request to modify the change, making it applicable to all versions >= 3.13 for RabbitMQ and versions >= 5.18 for ActiveMQ.

With the subsequent PR, adding an attribute such as actual_engine_version is unnecessary.

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

Expand All @@ -486,7 +501,7 @@
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)
Expand Down Expand Up @@ -556,10 +571,14 @@
}

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{})),
}

Expand Down
238 changes: 235 additions & 3 deletions internal/service/mq/broker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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.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",
}),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrApplyImmediately, "user"},
},
},
})
}

func TestAccMQBroker_disappears(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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" {
Expand Down
Loading