From d65c8a421a957b9d157350a608bec9c0f9ba44b1 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Fri, 5 Jun 2015 10:00:39 -0500 Subject: [PATCH] refactor the internals of this --- .../resource_aws_autoscaling_notification.go | 88 +++++++------------ ...ource_aws_autoscaling_notification_test.go | 86 +++++++++--------- 2 files changed, 73 insertions(+), 101 deletions(-) diff --git a/builtin/providers/aws/resource_aws_autoscaling_notification.go b/builtin/providers/aws/resource_aws_autoscaling_notification.go index 1eedeef75..07adaa676 100644 --- a/builtin/providers/aws/resource_aws_autoscaling_notification.go +++ b/builtin/providers/aws/resource_aws_autoscaling_notification.go @@ -2,8 +2,6 @@ package aws import ( "fmt" - "sort" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -33,7 +31,7 @@ func resourceAwsAutoscalingNotification() *schema.Resource { }, "notifications": &schema.Schema{ - Type: schema.TypeList, + Type: schema.TypeSet, Required: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, @@ -44,16 +42,11 @@ func resourceAwsAutoscalingNotification() *schema.Resource { func resourceAwsAutoscalingNotificationCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn - gl := d.Get("group_names").(*schema.Set).List() - var groups []interface{} - for _, g := range gl { - groups = append(groups, g) - } - - nl := getNofiticationList(d.Get("notifications").([]interface{})) + gl := convertSetToList(d.Get("group_names").(*schema.Set)) + nl := convertSetToList(d.Get("notifications").(*schema.Set)) topic := d.Get("topic_arn").(string) - if err := addNotificationConfigToGroupsWithTopic(conn, groups, nl, topic); err != nil { + if err := addNotificationConfigToGroupsWithTopic(conn, gl, nl, topic); err != nil { return err } @@ -65,14 +58,10 @@ func resourceAwsAutoscalingNotificationCreate(d *schema.ResourceData, meta inter func resourceAwsAutoscalingNotificationRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn - gl := d.Get("group_names").(*schema.Set).List() - var groups []*string - for _, g := range gl { - groups = append(groups, aws.String(g.(string))) - } + gl := convertSetToList(d.Get("group_names").(*schema.Set)) opts := &autoscaling.DescribeNotificationConfigurationsInput{ - AutoScalingGroupNames: groups, + AutoScalingGroupNames: gl, } resp, err := conn.DescribeNotificationConfigurations(opts) @@ -80,10 +69,12 @@ func resourceAwsAutoscalingNotificationRead(d *schema.ResourceData, meta interfa return fmt.Errorf("Error describing notifications") } - // grab all applicable notifcation configurations for this Topic + topic := d.Get("topic_arn").(string) + // Grab all applicable notifcation configurations for this Topic. + // Each NotificationType will have a record, so 1 Group with 3 Types results + // in 3 records, all with the same Group name gRaw := make(map[string]bool) nRaw := make(map[string]bool) - topic := d.Get("topic_arn").(string) for _, n := range resp.NotificationConfigurations { if *n.TopicARN == topic { gRaw[*n.AutoScalingGroupName] = true @@ -91,18 +82,18 @@ func resourceAwsAutoscalingNotificationRead(d *schema.ResourceData, meta interfa } } + // Grab the keys here as the list of Groups var gList []string for k, _ := range gRaw { gList = append(gList, k) } + + // Grab the keys here as the list of Types var nList []string for k, _ := range nRaw { nList = append(nList, k) } - sort.Strings(gList) - sort.Strings(nList) - if err := d.Set("group_names", gList); err != nil { return err } @@ -116,7 +107,9 @@ func resourceAwsAutoscalingNotificationRead(d *schema.ResourceData, meta interfa func resourceAwsAutoscalingNotificationUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn - nl := getNofiticationList(d.Get("notifications").([]interface{})) + // Notifications API call is a PUT, so we don't need to diff the list, just + // push whatever it is and AWS sorts it out + nl := convertSetToList(d.Get("notifications").(*schema.Set)) o, n := d.GetChange("group_names") if o == nil { @@ -128,8 +121,8 @@ func resourceAwsAutoscalingNotificationUpdate(d *schema.ResourceData, meta inter os := o.(*schema.Set) ns := n.(*schema.Set) - remove := os.Difference(ns).List() - add := ns.Difference(os).List() + remove := convertSetToList(os.Difference(ns)) + add := convertSetToList(ns.Difference(os)) topic := d.Get("topic_arn").(string) @@ -137,11 +130,9 @@ func resourceAwsAutoscalingNotificationUpdate(d *schema.ResourceData, meta inter return err } - var update []interface{} + var update []*string if d.HasChange("notifications") { - for _, g := range d.Get("group_names").(*schema.Set).List() { - update = append(update, g) - } + update = convertSetToList(d.Get("group_names").(*schema.Set)) } else { update = add } @@ -153,10 +144,10 @@ func resourceAwsAutoscalingNotificationUpdate(d *schema.ResourceData, meta inter return resourceAwsAutoscalingNotificationRead(d, meta) } -func addNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, groups []interface{}, nl []*string, topic string) error { +func addNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, groups []*string, nl []*string, topic string) error { for _, a := range groups { opts := &autoscaling.PutNotificationConfigurationInput{ - AutoScalingGroupName: aws.String(a.(string)), + AutoScalingGroupName: a, NotificationTypes: nl, TopicARN: aws.String(topic), } @@ -164,7 +155,7 @@ func addNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, group _, err := conn.PutNotificationConfiguration(opts) if err != nil { if awsErr, ok := err.(awserr.Error); ok { - return fmt.Errorf("[WARN] Error creating Autoscaling Group Notification for Group %s, error: \"%s\", code: \"%s\"", a.(string), awsErr.Message(), awsErr.Code()) + return fmt.Errorf("[WARN] Error creating Autoscaling Group Notification for Group %s, error: \"%s\", code: \"%s\"", *a, awsErr.Message(), awsErr.Code()) } return err } @@ -172,16 +163,16 @@ func addNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, group return nil } -func removeNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, groups []interface{}, topic string) error { +func removeNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, groups []*string, topic string) error { for _, r := range groups { opts := &autoscaling.DeleteNotificationConfigurationInput{ - AutoScalingGroupName: aws.String(r.(string)), + AutoScalingGroupName: r, TopicARN: aws.String(topic), } _, err := conn.DeleteNotificationConfiguration(opts) if err != nil { - return fmt.Errorf("[WARN] Error deleting notification configuration for ASG \"%s\", Topic ARN \"%s\"", r.(string), topic) + return fmt.Errorf("[WARN] Error deleting notification configuration for ASG \"%s\", Topic ARN \"%s\"", *r, topic) } } return nil @@ -189,36 +180,21 @@ func removeNotificationConfigToGroupsWithTopic(conn *autoscaling.AutoScaling, gr func resourceAwsAutoscalingNotificationDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn - gl := d.Get("group_names").(*schema.Set).List() - var groups []interface{} - for _, g := range gl { - groups = append(groups, g) - } + gl := convertSetToList(d.Get("group_names").(*schema.Set)) topic := d.Get("topic_arn").(string) - if err := removeNotificationConfigToGroupsWithTopic(conn, groups, topic); err != nil { + if err := removeNotificationConfigToGroupsWithTopic(conn, gl, topic); err != nil { return err } return nil } -func buildNotificationTypesSlice(l []string) (nl []*string) { +func convertSetToList(s *schema.Set) (nl []*string) { + l := s.List() for _, n := range l { - if !strings.HasPrefix(n, "autoscaling:") { - nl = append(nl, aws.String("autoscaling:"+n)) - } else { - nl = append(nl, aws.String(n)) - } + nl = append(nl, aws.String(n.(string))) } + return nl } - -func getNofiticationList(l []interface{}) (nl []*string) { - var notifications []string - for _, n := range l { - notifications = append(notifications, n.(string)) - } - - return buildNotificationTypesSlice(notifications) -} diff --git a/builtin/providers/aws/resource_aws_autoscaling_notification_test.go b/builtin/providers/aws/resource_aws_autoscaling_notification_test.go index 4b2ceed56..a9b078a6e 100644 --- a/builtin/providers/aws/resource_aws_autoscaling_notification_test.go +++ b/builtin/providers/aws/resource_aws_autoscaling_notification_test.go @@ -11,28 +11,6 @@ import ( "github.com/hashicorp/terraform/terraform" ) -func TestBuildNotificationSlice(t *testing.T) { - a := "autoscaling:one" - b := "autoscaling:two" - - cases := []struct { - Input []string - Output []*string - }{ - {[]string{"one", "two"}, []*string{&a, &b}}, - {[]string{"autoscaling:one", "two"}, []*string{&a, &b}}, - } - - for _, tc := range cases { - actual := buildNotificationTypesSlice(tc.Input) - for i, a := range actual { - if *tc.Output[i] != *a { - t.Fatalf("bad converstion:\n\tinput: %s\n\toutput: %s", *tc.Output[i], *a) - } - } - } -} - func TestAccASGNotification_basic(t *testing.T) { var asgn autoscaling.DescribeNotificationConfigurationsOutput @@ -44,7 +22,7 @@ func TestAccASGNotification_basic(t *testing.T) { resource.TestStep{ Config: testAccASGNotificationConfig_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckASGNotificationExists("aws_autoscaling_notification.example", &asgn), + testAccCheckASGNotificationExists("aws_autoscaling_notification.example", []string{"foobar1-terraform-test"}, &asgn), testAccCheckAWSASGNotificationAttributes("aws_autoscaling_notification.example", &asgn), ), }, @@ -63,7 +41,7 @@ func TestAccASGNotification_update(t *testing.T) { resource.TestStep{ Config: testAccASGNotificationConfig_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckASGNotificationExists("aws_autoscaling_notification.example", &asgn), + testAccCheckASGNotificationExists("aws_autoscaling_notification.example", []string{"foobar1-terraform-test"}, &asgn), testAccCheckAWSASGNotificationAttributes("aws_autoscaling_notification.example", &asgn), ), }, @@ -71,7 +49,7 @@ func TestAccASGNotification_update(t *testing.T) { resource.TestStep{ Config: testAccASGNotificationConfig_update, Check: resource.ComposeTestCheckFunc( - testAccCheckASGNotificationExists("aws_autoscaling_notification.example", &asgn), + testAccCheckASGNotificationExists("aws_autoscaling_notification.example", []string{"foobar1-terraform-test", "barfoo-terraform-test"}, &asgn), testAccCheckAWSASGNotificationAttributes("aws_autoscaling_notification.example", &asgn), ), }, @@ -79,7 +57,7 @@ func TestAccASGNotification_update(t *testing.T) { }) } -func testAccCheckASGNotificationExists(n string, asgn *autoscaling.DescribeNotificationConfigurationsOutput) resource.TestCheckFunc { +func testAccCheckASGNotificationExists(n string, groups []string, asgn *autoscaling.DescribeNotificationConfigurationsOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -90,17 +68,14 @@ func testAccCheckASGNotificationExists(n string, asgn *autoscaling.DescribeNotif return fmt.Errorf("No ASG Notification ID is set") } - // var groups []*string - // groupCount, _ := strconv.Atoi(rs.Primary.Attributes["group_names.#"]) - // for i := 0; i < groupCount; i++ { - // key := fmt.Sprintf("group_names.%d", i) - // groups = append(groups, aws.String(rs.Primary.Attributes[key])) - // } - groups := []*string{aws.String("foobar1-terraform-test")} + var gl []*string + for _, g := range groups { + gl = append(gl, aws.String(g)) + } conn := testAccProvider.Meta().(*AWSClient).autoscalingconn opts := &autoscaling.DescribeNotificationConfigurationsInput{ - AutoScalingGroupNames: groups, + AutoScalingGroupNames: gl, } resp, err := conn.DescribeNotificationConfigurations(opts) @@ -154,17 +129,38 @@ func testAccCheckAWSASGNotificationAttributes(n string, asgn *autoscaling.Descri return fmt.Errorf("Error: no ASG Notifications found") } - var notifications []*autoscaling.NotificationConfiguration + // build a unique list of groups, notification types + gRaw := make(map[string]bool) + nRaw := make(map[string]bool) for _, n := range asgn.NotificationConfigurations { if *n.TopicARN == rs.Primary.Attributes["topic_arn"] { - notifications = append(notifications, n) + gRaw[*n.AutoScalingGroupName] = true + nRaw[*n.NotificationType] = true } } + // Grab the keys here as the list of Groups + var gList []string + for k, _ := range gRaw { + gList = append(gList, k) + } + + // Grab the keys here as the list of Types + var nList []string + for k, _ := range nRaw { + nList = append(nList, k) + } + typeCount, _ := strconv.Atoi(rs.Primary.Attributes["notifications.#"]) - if len(notifications) != typeCount { - return fmt.Errorf("Error: Bad ASG Notification count, expected (%d), got (%d)", typeCount, len(notifications)) + if len(nList) != typeCount { + return fmt.Errorf("Error: Bad ASG Notification count, expected (%d), got (%d)", typeCount, len(nList)) + } + + groupCount, _ := strconv.Atoi(rs.Primary.Attributes["group_names.#"]) + + if len(gList) != groupCount { + return fmt.Errorf("Error: Bad ASG Group count, expected (%d), got (%d)", typeCount, len(gList)) } return nil @@ -243,14 +239,14 @@ resource "aws_autoscaling_group" "foo" { } resource "aws_autoscaling_notification" "example" { - group_names = [ + group_names = [ "${aws_autoscaling_group.bar.name}", "${aws_autoscaling_group.foo.name}", ] - notifications = [ - "EC2_INSTANCE_LAUNCH", - "EC2_INSTANCE_TERMINATE", - "EC2_INSTANCE_LAUNCH_ERROR" - ] - topic_arn = "${aws_sns_topic.user_updates.arn}" + notifications = [ + "autoscaling:EC2_INSTANCE_LAUNCH", + "autoscaling:EC2_INSTANCE_TERMINATE", + "autoscaling:EC2_INSTANCE_LAUNCH_ERROR" + ] + topic_arn = "${aws_sns_topic.user_updates.arn}" }`