Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #3004: adds support for modifying Beanstalk tags #3513

Merged
merged 4 commits into from
Mar 1, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions aws/resource_aws_elastic_beanstalk_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ func resourceAwsElasticBeanstalkEnvironment() *schema.Resource {
MigrateState: resourceAwsElasticBeanstalkEnvironmentMigrateState,

Schema: map[string]*schema.Schema{
"arn": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ensure this new attribute has an associated test of some sort in TestAccAWSBeanstalkEnv_basic:

resource.TestCheckResourceAttrSet("aws_elastic_beanstalk_environment.tfenvtest", "arn"),
# or preferably
resource.TestMatchResourceAttr("aws_elastic_beanstalk_environment.tfenvtest", "arn", regexp.MustCompile(fmt.Sprintf("^arn:[^:]+:elasticbeanstalk:[^:]+:[^:]+:environment/%s/%s$", appName, envName)),

Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Required: true,
Expand Down Expand Up @@ -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") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move all the tag update logic inside this conditional here? We do not need to make this more complicated by spreading the logic everywhere like the other update call (which needs to build up arguments).

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))
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -496,6 +562,10 @@ func resourceAwsElasticBeanstalkEnvironmentRead(d *schema.ResourceData, meta int
return err
}

if err := d.Set("arn", env.EnvironmentArn); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Unfortunately the code around it does not follow the convention, but for simple string attributes, we do not need the err handling logic.

d.Set("arn", env.EnvironmentArn)

return err
}

if err := d.Set("name", env.EnvironmentName); err != nil {
return err
}
Expand Down Expand Up @@ -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)
}

Expand Down
11 changes: 5 additions & 6 deletions aws/tagsBeanstalk.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,19 @@ 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 {
create[*t.Key] = *t.Value
}

// 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)
}
}

Expand Down Expand Up @@ -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 {
Expand Down
22 changes: 12 additions & 10 deletions aws/tagsBeanstalk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -27,8 +28,8 @@ func TestDiffBeanstalkTags(t *testing.T) {
Create: map[string]string{
"bar": "baz",
},
Remove: map[string]string{
"foo": "bar",
Remove: []string{
"foo",
},
},

Expand All @@ -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)
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions website/docs/r/elastic_beanstalk_environment.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down