From 6edf1b497299953fe2fe9a79850670bcd72da7ce Mon Sep 17 00:00:00 2001 From: Clint Date: Sun, 7 Aug 2016 02:34:58 -0500 Subject: [PATCH] provider/aws: Fix issue updating ElasticBeanstalk Environment Settings (#7777) * provider/aws: Fix issue updating ElasticBeanstalk Environment Settings Fixes the logic that updated settings for Elastic Beanstalk Environments. Because the update is done in the same API call, we need to split removals / additions. Fixes #6890 * add acc test that fails on master --- ...ource_aws_elastic_beanstalk_environment.go | 46 +++++- ..._aws_elastic_beanstalk_environment_test.go | 154 ++++++++++++++++++ 2 files changed, 198 insertions(+), 2 deletions(-) diff --git a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go index cf1748733f..4d98189f99 100644 --- a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go +++ b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment.go @@ -97,7 +97,6 @@ func resourceAwsElasticBeanstalkEnvironment() *schema.Resource { "setting": &schema.Schema{ Type: schema.TypeSet, Optional: true, - Computed: true, Elem: resourceAwsElasticBeanstalkOptionSetting(), Set: optionSettingValueHash, }, @@ -329,7 +328,50 @@ func resourceAwsElasticBeanstalkEnvironmentUpdate(d *schema.ResourceData, meta i os := o.(*schema.Set) ns := n.(*schema.Set) - updateOpts.OptionSettings = extractOptionSettings(ns.Difference(os)) + rm := extractOptionSettings(os.Difference(ns)) + add := extractOptionSettings(ns.Difference(os)) + + // Additions and removals of options are done in a single API call, so we + // can't do our normal "remove these" and then later "add these", re-adding + // any updated settings. + // Because of this, we need to remove any settings in the "removable" + // settings that are also found in the "add" settings, otherwise they + // conflict. Here we loop through all the initial removables from the set + // difference, and we build up a slice of settings not found in the "add" + // set + var remove []*elasticbeanstalk.ConfigurationOptionSetting + if len(add) > 0 { + for _, r := range rm { + for _, a := range add { + // ResourceNames are optional. Some defaults come with it, some do + // not. We need to guard against nil/empty in state as well as + // nil/empty from the API + if a.ResourceName != nil { + if r.ResourceName == nil { + continue + } + if *r.ResourceName != *a.ResourceName { + continue + } + } + if *r.Namespace == *a.Namespace && *r.OptionName == *a.OptionName { + continue + } + remove = append(remove, r) + } + } + } else { + remove = rm + } + + for _, elem := range remove { + updateOpts.OptionsToRemove = append(updateOpts.OptionsToRemove, &elasticbeanstalk.OptionSpecification{ + Namespace: elem.Namespace, + OptionName: elem.OptionName, + }) + } + + updateOpts.OptionSettings = add } if d.HasChange("template_name") { diff --git a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment_test.go b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment_test.go index ab47443513..9f23b0fae9 100644 --- a/builtin/providers/aws/resource_aws_elastic_beanstalk_environment_test.go +++ b/builtin/providers/aws/resource_aws_elastic_beanstalk_environment_test.go @@ -3,7 +3,9 @@ package aws import ( "fmt" "log" + "reflect" "regexp" + "sort" "testing" "github.com/aws/aws-sdk-go/aws" @@ -212,6 +214,93 @@ func TestAccAWSBeanstalkEnv_template_change(t *testing.T) { }) } +func TestAccAWSBeanstalkEnv_basic_settings_update(t *testing.T) { + var app elasticbeanstalk.EnvironmentDescription + + rInt := acctest.RandInt() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckBeanstalkEnvDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccBeanstalkEnvConfig_empty_settings(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), + testAccVerifyBeanstalkConfig(&app, []string{}), + ), + }, + resource.TestStep{ + Config: testAccBeanstalkEnvConfig_settings(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), + testAccVerifyBeanstalkConfig(&app, []string{"TF_LOG", "TF_SOME_VAR"}), + ), + }, + resource.TestStep{ + Config: testAccBeanstalkEnvConfig_empty_settings(rInt), + Check: resource.ComposeTestCheckFunc( + testAccCheckBeanstalkEnvExists("aws_elastic_beanstalk_environment.tfenvtest", &app), + testAccVerifyBeanstalkConfig(&app, []string{}), + ), + }, + }, + }) +} + +func testAccVerifyBeanstalkConfig(env *elasticbeanstalk.EnvironmentDescription, expected []string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if env == nil { + return fmt.Errorf("Nil environment in testAccVerifyBeanstalkConfig") + } + conn := testAccProvider.Meta().(*AWSClient).elasticbeanstalkconn + + resp, err := conn.DescribeConfigurationSettings(&elasticbeanstalk.DescribeConfigurationSettingsInput{ + ApplicationName: env.ApplicationName, + EnvironmentName: env.EnvironmentName, + }) + + if err != nil { + return fmt.Errorf("Error describing config settings in testAccVerifyBeanstalkConfig: %s", err) + } + + // should only be 1 environment + if len(resp.ConfigurationSettings) != 1 { + return fmt.Errorf("Expected only 1 set of Configuration Settings in testAccVerifyBeanstalkConfig, got (%d)", len(resp.ConfigurationSettings)) + } + + cs := resp.ConfigurationSettings[0] + + var foundEnvs []string + testStrings := []string{"TF_LOG", "TF_SOME_VAR"} + for _, os := range cs.OptionSettings { + for _, k := range testStrings { + if *os.OptionName == k { + foundEnvs = append(foundEnvs, k) + } + } + } + + // if expected is zero, then we should not have found any of the predefined + // env vars + if len(expected) == 0 { + if len(foundEnvs) > 0 { + return fmt.Errorf("Found configs we should not have: %#v", foundEnvs) + } + return nil + } + + sort.Strings(testStrings) + sort.Strings(expected) + if !reflect.DeepEqual(testStrings, expected) { + return fmt.Errorf("Error matching strings, expected:\n\n%#v\n\ngot:\n\n%#v\n", testStrings, foundEnvs) + } + + return nil + } +} + func testAccCheckBeanstalkEnvDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).elasticbeanstalkconn @@ -376,6 +465,71 @@ resource "aws_elastic_beanstalk_environment" "tfenvtest" { } ` +func testAccBeanstalkEnvConfig_empty_settings(r int) string { + return fmt.Sprintf(` +resource "aws_elastic_beanstalk_application" "tftest" { + name = "tf-test-name-%d" + description = "tf-test-desc" +} + +resource "aws_elastic_beanstalk_environment" "tfenvtest" { + name = "tf-test-name-%d" + application = "${aws_elastic_beanstalk_application.tftest.name}" + solution_stack_name = "64bit Amazon Linux running Python" + + wait_for_ready_timeout = "15m" +}`, r, r) +} + +func testAccBeanstalkEnvConfig_settings(r int) string { + return fmt.Sprintf(` +resource "aws_elastic_beanstalk_application" "tftest" { + name = "tf-test-name-%d" + description = "tf-test-desc" +} + +resource "aws_elastic_beanstalk_environment" "tfenvtest" { + name = "tf-test-name-%d" + application = "${aws_elastic_beanstalk_application.tftest.name}" + solution_stack_name = "64bit Amazon Linux running Python" + + wait_for_ready_timeout = "15m" + + setting { + namespace = "aws:elasticbeanstalk:application:environment" + name = "TF_LOG" + value = "true" + } + + setting { + namespace = "aws:elasticbeanstalk:application:environment" + name = "TF_SOME_VAR" + value = "true" + } + + setting { + namespace = "aws:autoscaling:scheduledaction" + resource = "ScheduledAction01" + name = "MinSize" + value = 2 + } + + setting { + namespace = "aws:autoscaling:scheduledaction" + resource = "ScheduledAction01" + name = "MaxSize" + value = 3 + } + + setting { + namespace = "aws:autoscaling:scheduledaction" + resource = "ScheduledAction01" + name = "StartTime" + value = "2016-07-28T04:07:02Z" + } +}`, r, r) +} + const testAccBeanstalkWorkerEnvConfig = ` resource "aws_iam_instance_profile" "tftest" { name = "tftest_profile"