From 7d502812559d8a35358a51f996e0826ddb8e0f8a Mon Sep 17 00:00:00 2001 From: James Peiris Date: Thu, 15 Aug 2019 14:00:57 +0100 Subject: [PATCH 1/3] Fixed remnant CloudWatch Dashboards on each dashboard name change. Added related test. --- aws/resource_aws_cloudwatch_dashboard.go | 36 +++++++++++-- aws/resource_aws_cloudwatch_dashboard_test.go | 52 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_cloudwatch_dashboard.go b/aws/resource_aws_cloudwatch_dashboard.go index 2a60e595fd1c..0745a81d77ce 100644 --- a/aws/resource_aws_cloudwatch_dashboard.go +++ b/aws/resource_aws_cloudwatch_dashboard.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/structure" "github.com/hashicorp/terraform/helper/validation" + "github.com/hashicorp/terraform/terraform" ) func resourceAwsCloudWatchDashboard() *schema.Resource { @@ -76,10 +77,20 @@ func resourceAwsCloudWatchDashboardRead(d *schema.ResourceData, meta interface{} } func resourceAwsCloudWatchDashboardPut(d *schema.ResourceData, meta interface{}) error { + var prevState *terraform.InstanceState = d.State() + var dashboardNameBefore string + + dashboardBody := d.Get("dashboard_body") + dashboardName := d.Get("dashboard_name") + + if prevState != nil { + dashboardNameBefore = (*d.State()).ID + } + conn := meta.(*AWSClient).cloudwatchconn params := cloudwatch.PutDashboardInput{ - DashboardBody: aws.String(d.Get("dashboard_body").(string)), - DashboardName: aws.String(d.Get("dashboard_name").(string)), + DashboardBody: aws.String(dashboardBody.(string)), + DashboardName: aws.String(dashboardName.(string)), } log.Printf("[DEBUG] Putting CloudWatch Dashboard: %#v", params) @@ -88,9 +99,28 @@ func resourceAwsCloudWatchDashboardPut(d *schema.ResourceData, meta interface{}) if err != nil { return fmt.Errorf("Putting dashboard failed: %s", err) } - d.SetId(d.Get("dashboard_name").(string)) + d.SetId(dashboardName.(string)) + log.Println("[INFO] CloudWatch Dashboard put finished") + if prevState != nil && dashboardNameBefore != dashboardName { + log.Printf("[INFO] Dashboard name changed, cleaning up previous dashboard: %s", dashboardNameBefore) + + newConn := meta.(*AWSClient).cloudwatchconn + params := cloudwatch.DeleteDashboardsInput{ + DashboardNames: []*string{aws.String(dashboardNameBefore)}, + } + + if _, err := newConn.DeleteDashboards(¶ms); err != nil { + if isCloudWatchDashboardNotFoundErr(err) { + return nil + } + return fmt.Errorf("Error deleting CloudWatch Dashboard: %s", dashboardNameBefore) + } + + log.Printf("[INFO] CloudWatch Dashboard %s deleted", dashboardNameBefore) + } + return resourceAwsCloudWatchDashboardRead(d, meta) } diff --git a/aws/resource_aws_cloudwatch_dashboard_test.go b/aws/resource_aws_cloudwatch_dashboard_test.go index 476829eafa53..9c44d7525369 100644 --- a/aws/resource_aws_cloudwatch_dashboard_test.go +++ b/aws/resource_aws_cloudwatch_dashboard_test.go @@ -82,6 +82,36 @@ func TestAccAWSCloudWatchDashboard_update(t *testing.T) { }) } +func TestAccAWSCloudWatchDashboard_updateName(t *testing.T) { + var dashboard cloudwatch.GetDashboardOutput + rInt := acctest.RandInt() + rInt2 := acctest.RandInt() + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSCloudWatchDashboardDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSCloudWatchDashboardConfig(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudWatchDashboardExists("aws_cloudwatch_dashboard.foobar", &dashboard), + testAccCloudWatchCheckDashboardBodyIsExpected("aws_cloudwatch_dashboard.foobar", basicWidget), + resource.TestCheckResourceAttr("aws_cloudwatch_dashboard.foobar", "dashboard_name", testAccAWSCloudWatchDashboardName(rInt)), + ), + }, + { + Config: testAccAWSCloudWatchDashboardConfig(rInt2), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudWatchDashboardExists("aws_cloudwatch_dashboard.foobar", &dashboard), + testAccCloudWatchCheckDashboardBodyIsExpected("aws_cloudwatch_dashboard.foobar", basicWidget), + resource.TestCheckResourceAttr("aws_cloudwatch_dashboard.foobar", "dashboard_name", testAccAWSCloudWatchDashboardName(rInt2)), + testAccCheckAWSCloudWatchDashboardDestroyPrevious(testAccAWSCloudWatchDashboardName(rInt)), + ), + }, + }, + }) +} + func testAccCheckCloudWatchDashboardExists(n string, dashboard *cloudwatch.GetDashboardOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -129,6 +159,28 @@ func testAccCheckAWSCloudWatchDashboardDestroy(s *terraform.State) error { return nil } +func testAccCheckAWSCloudWatchDashboardDestroyPrevious(dashboardName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).cloudwatchconn + + params := cloudwatch.GetDashboardInput{ + DashboardName: aws.String(dashboardName), + } + + _, err := conn.GetDashboard(¶ms) + + if err == nil { + return fmt.Errorf("Dashboard still exists: %s", dashboardName) + } + + if !isCloudWatchDashboardNotFoundErr(err) { + return err + } + + return nil + } +} + const ( basicWidget = `{ "widgets": [{ From fbd8fdcb2eb21498b42f4166beecec2c11ceb2ae Mon Sep 17 00:00:00 2001 From: James Peiris Date: Thu, 2 Jan 2020 11:11:05 +0000 Subject: [PATCH 2/3] Resetting implementation --- aws/resource_aws_cloudwatch_dashboard.go | 36 ++---------------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/aws/resource_aws_cloudwatch_dashboard.go b/aws/resource_aws_cloudwatch_dashboard.go index 670558161665..03b7c95df035 100644 --- a/aws/resource_aws_cloudwatch_dashboard.go +++ b/aws/resource_aws_cloudwatch_dashboard.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" - "github.com/hashicorp/terraform/terraform" // TODO: (JP) Is this reference still correct? ) func resourceAwsCloudWatchDashboard() *schema.Resource { @@ -77,20 +76,10 @@ func resourceAwsCloudWatchDashboardRead(d *schema.ResourceData, meta interface{} } func resourceAwsCloudWatchDashboardPut(d *schema.ResourceData, meta interface{}) error { - var prevState *terraform.InstanceState = d.State() - var dashboardNameBefore string - - dashboardBody := d.Get("dashboard_body") - dashboardName := d.Get("dashboard_name") - - if prevState != nil { - dashboardNameBefore = (*d.State()).ID - } - conn := meta.(*AWSClient).cloudwatchconn params := cloudwatch.PutDashboardInput{ - DashboardBody: aws.String(dashboardBody.(string)), - DashboardName: aws.String(dashboardName.(string)), + DashboardBody: aws.String(d.Get("dashboard_body").(string)), + DashboardName: aws.String(d.Get("dashboard_name").(string)), } log.Printf("[DEBUG] Putting CloudWatch Dashboard: %#v", params) @@ -99,28 +88,9 @@ func resourceAwsCloudWatchDashboardPut(d *schema.ResourceData, meta interface{}) if err != nil { return fmt.Errorf("Putting dashboard failed: %s", err) } - d.SetId(dashboardName.(string)) - + d.SetId(d.Get("dashboard_name").(string)) log.Println("[INFO] CloudWatch Dashboard put finished") - if prevState != nil && dashboardNameBefore != dashboardName { - log.Printf("[INFO] Dashboard name changed, cleaning up previous dashboard: %s", dashboardNameBefore) - - newConn := meta.(*AWSClient).cloudwatchconn - params := cloudwatch.DeleteDashboardsInput{ - DashboardNames: []*string{aws.String(dashboardNameBefore)}, - } - - if _, err := newConn.DeleteDashboards(¶ms); err != nil { - if isCloudWatchDashboardNotFoundErr(err) { - return nil - } - return fmt.Errorf("Error deleting CloudWatch Dashboard: %s", dashboardNameBefore) - } - - log.Printf("[INFO] CloudWatch Dashboard %s deleted", dashboardNameBefore) - } - return resourceAwsCloudWatchDashboardRead(d, meta) } From 5c1a19456137ded0274a053ebd2feef49362818f Mon Sep 17 00:00:00 2001 From: James Peiris Date: Thu, 2 Jan 2020 11:11:43 +0000 Subject: [PATCH 3/3] Set ForceNew true on dashboard name --- aws/resource_aws_cloudwatch_dashboard.go | 1 + 1 file changed, 1 insertion(+) diff --git a/aws/resource_aws_cloudwatch_dashboard.go b/aws/resource_aws_cloudwatch_dashboard.go index 03b7c95df035..a7ff5aac3122 100644 --- a/aws/resource_aws_cloudwatch_dashboard.go +++ b/aws/resource_aws_cloudwatch_dashboard.go @@ -44,6 +44,7 @@ func resourceAwsCloudWatchDashboard() *schema.Resource { "dashboard_name": { Type: schema.TypeString, Required: true, + ForceNew: true, ValidateFunc: validateCloudWatchDashboardName, }, },