From 13b7b88c61a7bda0e4530753e930e02a06d35820 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Wed, 7 Feb 2018 20:12:00 -0500 Subject: [PATCH 1/4] Closes #3004: adds support for modifying Beanstalk tags The Elastic Beanstalk environment resource did not support updating tags on an existing environment, forcing the use of other tools to manage environment tags. The Elastic Beanstalk API now supports updating tags via a separate `UpdateTagsForResource` call. This updates the terraform resource to make an `UpdateTagsForResource` call whenever there are changes to the `tags` attribute. --- ...ource_aws_elastic_beanstalk_environment.go | 82 +++++++++++++++++++ aws/tagsBeanstalk.go | 11 ++- aws/tagsBeanstalk_test.go | 22 ++--- ...lastic_beanstalk_environment.html.markdown | 4 +- 4 files changed, 100 insertions(+), 19 deletions(-) diff --git a/aws/resource_aws_elastic_beanstalk_environment.go b/aws/resource_aws_elastic_beanstalk_environment.go index 156b1809fa4..2b71022dfcf 100644 --- a/aws/resource_aws_elastic_beanstalk_environment.go +++ b/aws/resource_aws_elastic_beanstalk_environment.go @@ -56,6 +56,10 @@ func resourceAwsElasticBeanstalkEnvironment() *schema.Resource { MigrateState: resourceAwsElasticBeanstalkEnvironmentMigrateState, Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, "name": { Type: schema.TypeString, Required: true, @@ -313,11 +317,28 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i envId := d.Id() var hasChange bool + var hasTagChange bool updateOpts := elasticbeanstalk.UpdateEnvironmentInput{ EnvironmentId: aws.String(envId), } + updateTags := elasticbeanstalk.UpdateTagsForResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + } + + if d.HasChange("tags") { + hasTagChange = true + o, n := d.GetChange("tags") + oldTags := tagsFromMapBeanstalk(o.(map[string]interface{})) + newTags := tagsFromMapBeanstalk(n.(map[string]interface{})) + + tagsToAdd, tagNamesToRemove := diffTagsBeanstalk(oldTags, newTags) + + updateTags.TagsToAdd = tagsToAdd + updateTags.TagsToRemove = tagNamesToRemove + } + if d.HasChange("description") { hasChange = true updateOpts.Description = aws.String(d.Get("description").(string)) @@ -452,6 +473,51 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i } } + if hasTagChange { + // Get the current time to filter getBeanstalkEnvironmentErrors messages + t := time.Now() + log.Printf("[DEBUG] Elastic Beanstalk Environment update tags: %s", updateTags) + _, err := conn.UpdateTagsForResource(&updateTags) + if err != nil { + return err + } + + waitForReadyTimeOut, err := time.ParseDuration(d.Get("wait_for_ready_timeout").(string)) + if err != nil { + return err + } + pollInterval, err := time.ParseDuration(d.Get("poll_interval").(string)) + if err != nil { + pollInterval = 0 + log.Printf("[WARN] Error parsing poll_interval, using default backoff") + } + + stateConf := &resource.StateChangeConf{ + Pending: []string{"Launching", "Updating"}, + Target: []string{"Ready"}, + Refresh: environmentStateRefreshFunc(conn, d.Id(), t), + Timeout: waitForReadyTimeOut, + Delay: 10 * time.Second, + PollInterval: pollInterval, + MinTimeout: 3 * time.Second, + } + + _, err = stateConf.WaitForState() + if err != nil { + return fmt.Errorf( + "Error waiting for Elastic Beanstalk Environment (%s) to become ready: %s", + d.Id(), err) + } + + envErrors, err := getBeanstalkEnvironmentErrors(conn, d.Id(), t) + if err != nil { + return err + } + if envErrors != nil { + return envErrors + } + } + return resourceAwsElasticBeanstalkEnvironmentRead(d, meta) } @@ -496,6 +562,10 @@ func resourceAwsElasticBeanstalkEnvironmentRead(d *schema.ResourceData, meta int return err } + if err := d.Set("arn", env.EnvironmentArn); err != nil { + return err + } + if err := d.Set("name", env.EnvironmentName); err != nil { return err } @@ -564,6 +634,18 @@ func resourceAwsElasticBeanstalkEnvironmentRead(d *schema.ResourceData, meta int return err } + tags, err := conn.ListTagsForResource(&elasticbeanstalk.ListTagsForResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + }) + + if err != nil { + return err + } + + if err := d.Set("tags", tagsToMapBeanstalk(tags.ResourceTags)); err != nil { + return err + } + return resourceAwsElasticBeanstalkEnvironmentSettingsRead(d, meta) } diff --git a/aws/tagsBeanstalk.go b/aws/tagsBeanstalk.go index 7b85d611603..29158b10915 100644 --- a/aws/tagsBeanstalk.go +++ b/aws/tagsBeanstalk.go @@ -11,7 +11,7 @@ import ( // diffTags takes our tags locally and the ones remotely and returns // the set of tags that must be created, and the set of tags that must // be destroyed. -func diffTagsBeanstalk(oldTags, newTags []*elasticbeanstalk.Tag) ([]*elasticbeanstalk.Tag, []*elasticbeanstalk.Tag) { +func diffTagsBeanstalk(oldTags, newTags []*elasticbeanstalk.Tag) ([]*elasticbeanstalk.Tag, []*string) { // First, we're creating everything we have create := make(map[string]interface{}) for _, t := range newTags { @@ -19,12 +19,11 @@ func diffTagsBeanstalk(oldTags, newTags []*elasticbeanstalk.Tag) ([]*elasticbean } // Build the list of what to remove - var remove []*elasticbeanstalk.Tag + var remove []*string for _, t := range oldTags { - old, ok := create[*t.Key] - if !ok || old != *t.Value { + if _, ok := create[*t.Key]; !ok { // Delete it! - remove = append(remove, t) + remove = append(remove, t.Key) } } @@ -62,7 +61,7 @@ func tagsToMapBeanstalk(ts []*elasticbeanstalk.Tag) map[string]string { // compare a tag against a list of strings and checks if it should // be ignored or not func tagIgnoredBeanstalk(t *elasticbeanstalk.Tag) bool { - filter := []string{"^aws:"} + filter := []string{"^aws:", "^elasticbeanstalk:", "Name"} for _, v := range filter { log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key) if r, _ := regexp.MatchString(v, *t.Key); r == true { diff --git a/aws/tagsBeanstalk_test.go b/aws/tagsBeanstalk_test.go index 0c92338630a..e6dbb09829d 100644 --- a/aws/tagsBeanstalk_test.go +++ b/aws/tagsBeanstalk_test.go @@ -13,8 +13,9 @@ import ( func TestDiffBeanstalkTags(t *testing.T) { cases := []struct { - Old, New map[string]interface{} - Create, Remove map[string]string + Old, New map[string]interface{} + Create map[string]string + Remove []string }{ // Basic add/remove { @@ -27,8 +28,8 @@ func TestDiffBeanstalkTags(t *testing.T) { Create: map[string]string{ "bar": "baz", }, - Remove: map[string]string{ - "foo": "bar", + Remove: []string{ + "foo", }, }, @@ -43,21 +44,22 @@ func TestDiffBeanstalkTags(t *testing.T) { Create: map[string]string{ "foo": "baz", }, - Remove: map[string]string{ - "foo": "bar", - }, + Remove: []string{}, }, } for i, tc := range cases { c, r := diffTagsBeanstalk(tagsFromMapBeanstalk(tc.Old), tagsFromMapBeanstalk(tc.New)) cm := tagsToMapBeanstalk(c) - rm := tagsToMapBeanstalk(r) + rl := []string{} + for _, tagName := range r { + rl = append(rl, *tagName) + } if !reflect.DeepEqual(cm, tc.Create) { t.Fatalf("%d: bad create: %#v", i, cm) } - if !reflect.DeepEqual(rm, tc.Remove) { - t.Fatalf("%d: bad remove: %#v", i, rm) + if !reflect.DeepEqual(rl, tc.Remove) { + t.Fatalf("%d: bad remove: %#v", i, rl) } } } diff --git a/website/docs/r/elastic_beanstalk_environment.html.markdown b/website/docs/r/elastic_beanstalk_environment.html.markdown index cfa004c4bea..090782017c2 100644 --- a/website/docs/r/elastic_beanstalk_environment.html.markdown +++ b/website/docs/r/elastic_beanstalk_environment.html.markdown @@ -60,9 +60,7 @@ for any `create` or `update` action. Minimum `10s`, maximum `180s`. Omit this to use the default behavior, which is an exponential backoff * `version_label` - (Optional) The name of the Elastic Beanstalk Application Version to use in deployment. -* `tags` – (Optional) A set of tags to apply to the Environment. **Note:** at -this time the Elastic Beanstalk API does not provide a programatic way of -changing these tags after initial application +* `tags` – (Optional) A set of tags to apply to the Environment. ## Option Settings From 652531c0857ad84010dc17ec79801e68b543d711 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 26 Feb 2018 12:49:37 -0500 Subject: [PATCH 2/4] Syntax updates for PR review --- ...ource_aws_elastic_beanstalk_environment.go | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/aws/resource_aws_elastic_beanstalk_environment.go b/aws/resource_aws_elastic_beanstalk_environment.go index 2b71022dfcf..48f20587c78 100644 --- a/aws/resource_aws_elastic_beanstalk_environment.go +++ b/aws/resource_aws_elastic_beanstalk_environment.go @@ -317,28 +317,11 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i envId := d.Id() var hasChange bool - var hasTagChange bool updateOpts := elasticbeanstalk.UpdateEnvironmentInput{ EnvironmentId: aws.String(envId), } - updateTags := elasticbeanstalk.UpdateTagsForResourceInput{ - ResourceArn: aws.String(d.Get("arn").(string)), - } - - if d.HasChange("tags") { - hasTagChange = true - o, n := d.GetChange("tags") - oldTags := tagsFromMapBeanstalk(o.(map[string]interface{})) - newTags := tagsFromMapBeanstalk(n.(map[string]interface{})) - - tagsToAdd, tagNamesToRemove := diffTagsBeanstalk(oldTags, newTags) - - updateTags.TagsToAdd = tagsToAdd - updateTags.TagsToRemove = tagNamesToRemove - } - if d.HasChange("description") { hasChange = true updateOpts.Description = aws.String(d.Get("description").(string)) @@ -473,7 +456,19 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i } } - if hasTagChange { + if d.HasChange("tags") { + o, n := d.GetChange("tags") + oldTags := tagsFromMapBeanstalk(o.(map[string]interface{})) + newTags := tagsFromMapBeanstalk(n.(map[string]interface{})) + + tagsToAdd, tagNamesToRemove := diffTagsBeanstalk(oldTags, newTags) + + updateTags := elasticbeanstalk.UpdateTagsForResourceInput{ + ResourceArn: aws.String(d.Get("arn").(string)), + TagsToAdd: tagsToAdd, + TagsToRemove: tagNamesToRemove, + } + // Get the current time to filter getBeanstalkEnvironmentErrors messages t := time.Now() log.Printf("[DEBUG] Elastic Beanstalk Environment update tags: %s", updateTags) @@ -562,9 +557,7 @@ func resourceAwsElasticBeanstalkEnvironmentRead(d *schema.ResourceData, meta int return err } - if err := d.Set("arn", env.EnvironmentArn); err != nil { - return err - } + d.Set("arn", env.EnvironmentArn) if err := d.Set("name", env.EnvironmentName); err != nil { return err From 6c194ca428a9d27865ef7549e0c383d943d83c9f Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 26 Feb 2018 12:50:34 -0500 Subject: [PATCH 3/4] Adds acceptance tests for EB environment ARN and tags --- ..._aws_elastic_beanstalk_environment_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/aws/resource_aws_elastic_beanstalk_environment_test.go b/aws/resource_aws_elastic_beanstalk_environment_test.go index 8b26da44463..6ef65629323 100644 --- a/aws/resource_aws_elastic_beanstalk_environment_test.go +++ b/aws/resource_aws_elastic_beanstalk_environment_test.go @@ -125,10 +125,14 @@ func TestAccAWSBeanstalkEnv_basic(t *testing.T) { Config: testAccBeanstalkEnvConfig(appName, envName), Check: resource.ComposeTestCheckFunc( testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), + resource.TestMatchResourceAttr( + "aws_elastic_beanstalk_environment.tfenvtest", "arn", + regexp.MustCompile(fmt.Sprintf("^arn:[^:]+:elasticbeanstalk:[^:]+:[^:]+:environment/%s/%s$", appName, envName))), ), }, }, }) + } func TestAccAWSBeanstalkEnv_tier(t *testing.T) { @@ -283,6 +287,49 @@ func TestAccAWSBeanstalkEnv_resource(t *testing.T) { }) } +func TestAccAWSBeanstalkEnv_tags(t *testing.T) { + var app elasticbeanstalk.EnvironmentDescription + + rString := acctest.RandString(8) + appName := fmt.Sprintf("tf_acc_app_env_resource_%s", rString) + envName := fmt.Sprintf("tf-acc-env-resource-%s", rString) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckBeanstalkEnvDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBeanstalkEnvConfig_empty_settings(appName, envName), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{}), + ), + }, + + { + Config: testAccBeanstalkTagsTemplate(appName, envName, "test1", "test2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{"firstTag": "test1", "secondTag": "test2"}), + ), + }, + + { + Config: testAccBeanstalkTagsTemplate(appName, envName, "test2", "test1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{"firstTag": "test2", "secondTag": "test1"}), + ), + }, + + { + Config: testAccBeanstalkEnvConfig_empty_settings(appName, envName), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{}), + ), + }, + }, + }) +} + func TestAccAWSBeanstalkEnv_vpc(t *testing.T) { var app elasticbeanstalk.EnvironmentDescription @@ -629,6 +676,32 @@ func testAccCheckBeanstalkEnvConfigValue(n string, expectedValue string) resourc } } +func testAccCheckBeanstalkEnvTagsMatch(env *elasticbeanstalk.EnvironmentDescription, expectedValue map[string]string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if env == nil { + return fmt.Errorf("Nil environment in testAccCheckBeanstalkEnvTagsMatch") + } + + conn := testAccProvider.Meta().(*AWSClient).elasticbeanstalkconn + + tags, err := conn.ListTagsForResource(&elasticbeanstalk.ListTagsForResourceInput{ + ResourceArn: env.EnvironmentArn, + }) + + if err != nil { + return err + } + + foundTags := tagsToMapBeanstalk(tags.ResourceTags) + + if !reflect.DeepEqual(foundTags, expectedValue) { + return fmt.Errorf("Tag value: %s. Expected %s", foundTags, expectedValue) + } + + return nil + } +} + func testAccCheckBeanstalkApplicationVersionDeployed(n string, app *elasticbeanstalk.EnvironmentDescription) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -933,6 +1006,27 @@ resource "aws_elastic_beanstalk_environment" "tfenvtest" { }`, appName, envName) } +func testAccBeanstalkTagsTemplate(appName, envName, firstTag, secondTag string) string { + return fmt.Sprintf(` +resource "aws_elastic_beanstalk_application" "tftest" { + name = "%s" + description = "tf-test-desc" +} + +resource "aws_elastic_beanstalk_environment" "tfenvtest" { + name = "%s" + application = "${aws_elastic_beanstalk_application.tftest.name}" + solution_stack_name = "64bit Amazon Linux running Python" + + wait_for_ready_timeout = "15m" + + tags { + firstTag = "%s" + secondTag = "%s" + } +}`, appName, envName, firstTag, secondTag) +} + func testAccBeanstalkEnv_VPC(sgName, appName, envName string) string { return fmt.Sprintf(` resource "aws_vpc" "tf_b_test" { From 5659931a0c78622efd7c4236bd075a44f75e3f76 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Thu, 1 Mar 2018 13:41:12 -0500 Subject: [PATCH 4/4] Adds env check so that acceptance tests pass --- aws/resource_aws_elastic_beanstalk_environment_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/aws/resource_aws_elastic_beanstalk_environment_test.go b/aws/resource_aws_elastic_beanstalk_environment_test.go index 6ef65629323..45095e84c03 100644 --- a/aws/resource_aws_elastic_beanstalk_environment_test.go +++ b/aws/resource_aws_elastic_beanstalk_environment_test.go @@ -302,6 +302,7 @@ func TestAccAWSBeanstalkEnv_tags(t *testing.T) { { Config: testAccBeanstalkEnvConfig_empty_settings(appName, envName), Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{}), ), }, @@ -309,6 +310,7 @@ func TestAccAWSBeanstalkEnv_tags(t *testing.T) { { Config: testAccBeanstalkTagsTemplate(appName, envName, "test1", "test2"), Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{"firstTag": "test1", "secondTag": "test2"}), ), }, @@ -316,6 +318,7 @@ func TestAccAWSBeanstalkEnv_tags(t *testing.T) { { Config: testAccBeanstalkTagsTemplate(appName, envName, "test2", "test1"), Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{"firstTag": "test2", "secondTag": "test1"}), ), }, @@ -323,6 +326,7 @@ func TestAccAWSBeanstalkEnv_tags(t *testing.T) { { Config: testAccBeanstalkEnvConfig_empty_settings(appName, envName), Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), testAccCheckBeanstalkEnvTagsMatch(&app, map[string]string{}), ), },