Skip to content

Commit

Permalink
resource/aws_mq_broker: security group changes not require broker reb…
Browse files Browse the repository at this point in the history 
…oot or recreation (#10442)

Output from acceptance testing:

```
--- PASS: TestAccAWSMqBroker_basic (1059.92s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1100.47s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1152.79s)
--- PASS: TestAccAWSMqBroker_updateTags (1223.38s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1235.39s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1446.46s)
--- PASS: TestAccAWSMqBroker_updateUsers (1597.68s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1801.50s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1861.13s)
```
  • Loading branch information
@albertoal @bflad
albertoal authored and bflad committed Nov 2, 2019
1 parent 6586f4e commit d26e9b6
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 14 deletions.
50 changes: 38 additions & 12 deletions aws/resource_aws_mq_broker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ func resourceAwsMqBroker() *schema.Resource {
Type: schema.TypeSet,
Elem: &schema.Schema{Type: schema.TypeString},
Required: true,
ForceNew: true,
},
"subnet_ids": {
Type: schema.TypeSet,
Expand Down Expand Up @@ -382,32 +381,53 @@ func resourceAwsMqBrokerRead(d *schema.ResourceData, meta interface{}) error {
func resourceAwsMqBrokerUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).mqconn

requiresReboot := false

if d.HasChange("security_groups") {
_, err := conn.UpdateBroker(&mq.UpdateBrokerRequest{
BrokerId: aws.String(d.Id()),
SecurityGroups: expandStringSet(d.Get("security_groups").(*schema.Set)),
})
if err != nil {
return fmt.Errorf("error updating MQ Broker (%s) security groups: %s", d.Id(), err)
}
}

if d.HasChange("configuration") || d.HasChange("logs") {
_, err := conn.UpdateBroker(&mq.UpdateBrokerRequest{
BrokerId: aws.String(d.Id()),
Configuration: expandMqConfigurationId(d.Get("configuration").([]interface{})),
Logs: expandMqLogs(d.Get("logs").([]interface{})),
})
if err != nil {
return err
return fmt.Errorf("error updating MQ Broker (%s) configuration: %s", d.Id(), err)
}
requiresReboot = true
}

if d.HasChange("user") {
o, n := d.GetChange("user")
err := updateAwsMqBrokerUsers(conn, d.Id(),
var err error
// d.HasChange("user") always reports a change when running resourceAwsMqBrokerUpdate
// updateAwsMqBrokerUsers needs to be called to know if changes to user are actually made
var usersUpdated bool
usersUpdated, err = updateAwsMqBrokerUsers(conn, d.Id(),
o.(*schema.Set).List(), n.(*schema.Set).List())
if err != nil {
return err
return fmt.Errorf("error updating MQ Broker (%s) user: %s", d.Id(), err)
}

if usersUpdated {
requiresReboot = true
}
}

if d.Get("apply_immediately").(bool) {
if d.Get("apply_immediately").(bool) && requiresReboot {
_, err := conn.RebootBroker(&mq.RebootBrokerInput{
BrokerId: aws.String(d.Id()),
})
if err != nil {
return err
return fmt.Errorf("error rebooting MQ Broker (%s): %s", d.Id(), err)
}

stateConf := resource.StateChangeConf{
Expand Down Expand Up @@ -502,32 +522,38 @@ func waitForMqBrokerDeletion(conn *mq.MQ, id string) error {
return err
}

func updateAwsMqBrokerUsers(conn *mq.MQ, bId string, oldUsers, newUsers []interface{}) error {
func updateAwsMqBrokerUsers(conn *mq.MQ, bId string, oldUsers, newUsers []interface{}) (bool, error) {
// If there are any user creates/deletes/updates, updatedUsers will be set to true
updatedUsers := false

createL, deleteL, updateL, err := diffAwsMqBrokerUsers(bId, oldUsers, newUsers)
if err != nil {
return err
return updatedUsers, err
}

for _, c := range createL {
_, err := conn.CreateUser(c)
updatedUsers = true
if err != nil {
return err
return updatedUsers, err
}
}
for _, d := range deleteL {
_, err := conn.DeleteUser(d)
updatedUsers = true
if err != nil {
return err
return updatedUsers, err
}
}
for _, u := range updateL {
_, err := conn.UpdateUser(u)
updatedUsers = true
if err != nil {
return err
return updatedUsers, err
}
}

return nil
return updatedUsers, nil
}

func diffAwsMqBrokerUsers(bId string, oldUsers, newUsers []interface{}) (
Expand Down
103 changes: 101 additions & 2 deletions aws/resource_aws_mq_broker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ func TestAccAWSMqBroker_updateTags(t *testing.T) {
resource.TestCheckResourceAttr("aws_mq_broker.test", "tags.env", "test"),
),
},
// Adding new user + modify existing
// Modify existing tags
{
Config: testAccMqBrokerConfig_updateTags2(sgName, brokerName),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -654,7 +654,7 @@ func TestAccAWSMqBroker_updateTags(t *testing.T) {
resource.TestCheckResourceAttr("aws_mq_broker.test", "tags.role", "test-role"),
),
},
// Deleting user + modify existing
// Deleting tags
{
Config: testAccMqBrokerConfig_updateTags3(sgName, brokerName),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -667,6 +667,45 @@ func TestAccAWSMqBroker_updateTags(t *testing.T) {
})
}

func TestAccAWSMqBroker_updateSecurityGroup(t *testing.T) {
sgName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5))
brokerName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5))

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSMq(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsMqBrokerDestroy,
Steps: []resource.TestStep{
{
Config: testAccMqBrokerConfig(sgName, brokerName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsMqBrokerExists("aws_mq_broker.test"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "security_groups.#", "1"),
),
},
{
Config: testAccMqBrokerConfig_updateSecurityGroups(sgName, brokerName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsMqBrokerExists("aws_mq_broker.test"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "security_groups.#", "2"),
),
},
// Trigger a reboot and ensure the password change was applied
// User hashcode can be retrieved by calling resourceAwsMqUserHash
{
Config: testAccMqBrokerConfig_updateUsersSecurityGroups(sgName, brokerName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsMqBrokerExists("aws_mq_broker.test"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "security_groups.#", "1"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "user.#", "1"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "user.2209734970.username", "Test"),
resource.TestCheckResourceAttr("aws_mq_broker.test", "user.2209734970.password", "TestTest9999"),
),
},
},
})
}

func testAccCheckAwsMqBrokerDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).mqconn

Expand Down Expand Up @@ -1128,3 +1167,63 @@ resource "aws_mq_broker" "test" {
}
`, sgName, brokerName)
}

func testAccMqBrokerConfig_updateSecurityGroups(sgName, brokerName string) string {
return fmt.Sprintf(`
resource "aws_security_group" "test" {
name = "%s"
}
resource "aws_security_group" "test2" {
name = "%s-2"
}
resource "aws_mq_broker" "test" {
apply_immediately = true
broker_name = "%s"
engine_type = "ActiveMQ"
engine_version = "5.15.0"
host_instance_type = "mq.t2.micro"
security_groups = ["${aws_security_group.test.id}", "${aws_security_group.test2.id}"]
logs {
general = true
}
user {
username = "Test"
password = "TestTest1234"
}
}
`, sgName, sgName, brokerName)
}

func testAccMqBrokerConfig_updateUsersSecurityGroups(sgName, brokerName string) string {
return fmt.Sprintf(`
resource "aws_security_group" "test" {
name = "%s"
}
resource "aws_security_group" "test2" {
name = "%s-2"
}
resource "aws_mq_broker" "test" {
apply_immediately = true
broker_name = "%s"
engine_type = "ActiveMQ"
engine_version = "5.15.0"
host_instance_type = "mq.t2.micro"
security_groups = ["${aws_security_group.test2.id}"]
logs {
general = true
}
user {
username = "Test"
password = "TestTest9999"
}
}
`, sgName, sgName, brokerName)
}

0 comments on commit d26e9b6

Please sign in to comment.