From 80ea2ae6d2d48dfa9419cbb8974c5233a0dce0a1 Mon Sep 17 00:00:00 2001 From: Jocelyn Giroux Date: Mon, 28 Aug 2017 15:06:24 -0400 Subject: [PATCH 1/5] Add support for description, allowed_pattern and tags for ssm parameters Allow detection of key_id change Fixing problems with ssm-parameter. The resource should not return an error if the parameter no longer exist. If a user manually delete a parameter, Terraform is no longer able to recreate it or destroy it. The defautt value for overwrite should be true. Otherwise, this is causing a breaking change with previously written code where overwrite is not set and the behaviour was to overwrite the parameter by default. Moreover, terraform already provides a mechanism to handle the lifecycle of the values. --- aws/resource_aws_ssm_parameter.go | 99 +++++++++++------ aws/resource_aws_ssm_parameter_test.go | 9 +- aws/tagsSSM.go | 117 +++++++++++++++++++++ aws/tagsSSM_test.go | 105 ++++++++++++++++++ website/docs/r/ssm_parameter.html.markdown | 11 +- 5 files changed, 305 insertions(+), 36 deletions(-) create mode 100644 aws/tagsSSM.go create mode 100644 aws/tagsSSM_test.go diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index 72669bfc72c..5c1e5bf9534 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -23,6 +23,11 @@ func resourceAwsSsmParameter() *schema.Resource { Required: true, ForceNew: true, }, + "description": { + Type: schema.TypeString, + Optional: true, + Default: "", + }, "type": { Type: schema.TypeString, Required: true, @@ -42,84 +47,114 @@ func resourceAwsSsmParameter() *schema.Resource { "overwrite": { Type: schema.TypeBool, Optional: true, - Default: false, + // The default should be set to true, terraform lifecycle should take care of not overriding the value if it is manually set by the user. + // Otherwise, it is causing a breaking change because the first version did not allow overwrite parameter and overwrite was allowed. + Default: true, + }, + "allowed_pattern": { + Type: schema.TypeString, + Optional: true, + Default: "", }, + "tags": tagsSchema(), }, } } func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error { - ssmconn := meta.(*AWSClient).ssmconn + conn := meta.(*AWSClient).ssmconn log.Printf("[DEBUG] Reading SSM Parameter: %s", d.Id()) - paramInput := &ssm.GetParametersInput{ - Names: []*string{ - aws.String(d.Get("name").(string)), - }, + if resp, err := conn.GetParameters(&ssm.GetParametersInput{ + Names: []*string{aws.String(d.Get("name").(string))}, WithDecryption: aws.Bool(true), + }); err != nil { + return errwrap.Wrapf("[ERROR] Error getting SSM parameter: {{err}}", err) + } else { + if len(resp.InvalidParameters) > 0 { + log.Print("[INFO] The resource no longer exists, marking it for recreation:", d.Id()) + d.SetId("") + return nil + } + param := resp.Parameters[0] + d.Set("name", param.Name) + d.Set("type", param.Type) + d.Set("value", param.Value) } - resp, err := ssmconn.GetParameters(paramInput) - - if err != nil { + if resp, err := conn.DescribeParameters(&ssm.DescribeParametersInput{ + Filters: []*ssm.ParametersFilter{ + &ssm.ParametersFilter{ + Key: aws.String("Name"), + Values: []*string{aws.String(d.Get("name").(string))}, + }, + }, + }); err != nil { return errwrap.Wrapf("[ERROR] Error describing SSM parameter: {{err}}", err) + } else { + param := resp.Parameters[0] + d.Set("key_id", param.KeyId) + d.Set("description", param.Description) + d.Set("allowed_pattern", param.AllowedPattern) } - if len(resp.InvalidParameters) > 0 { - return fmt.Errorf("[ERROR] SSM Parameter %s is invalid", d.Id()) + if tagList, err := conn.ListTagsForResource(&ssm.ListTagsForResourceInput{ + ResourceId: aws.String(d.Get("name").(string)), + ResourceType: aws.String("Parameter"), + }); err != nil { + return fmt.Errorf("Failed to get SSM parameter tags for %s: %s", d.Get("name"), err) + } else { + d.Set("tags", tagsToMapSSM(tagList.TagList)) } - param := resp.Parameters[0] - d.Set("name", param.Name) - d.Set("type", param.Type) - d.Set("value", param.Value) - return nil } func resourceAwsSsmParameterDelete(d *schema.ResourceData, meta interface{}) error { - ssmconn := meta.(*AWSClient).ssmconn + conn := meta.(*AWSClient).ssmconn log.Printf("[INFO] Deleting SSM Parameter: %s", d.Id()) - paramInput := &ssm.DeleteParameterInput{ + _, err := conn.DeleteParameter(&ssm.DeleteParameterInput{ Name: aws.String(d.Get("name").(string)), - } - - _, err := ssmconn.DeleteParameter(paramInput) + }) if err != nil { return err } - d.SetId("") return nil } func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error { - ssmconn := meta.(*AWSClient).ssmconn + conn := meta.(*AWSClient).ssmconn log.Printf("[INFO] Creating SSM Parameter: %s", d.Get("name").(string)) paramInput := &ssm.PutParameterInput{ - Name: aws.String(d.Get("name").(string)), - Type: aws.String(d.Get("type").(string)), - Value: aws.String(d.Get("value").(string)), - Overwrite: aws.Bool(d.Get("overwrite").(bool)), + Name: aws.String(d.Get("name").(string)), + Description: aws.String(d.Get("description").(string)), + Type: aws.String(d.Get("type").(string)), + Value: aws.String(d.Get("value").(string)), + Overwrite: aws.Bool(d.Get("overwrite").(bool)), + AllowedPattern: aws.String(d.Get("allowed_pattern").(string)), } + if keyID, ok := d.GetOk("key_id"); ok { - log.Printf("[DEBUG] Setting key_id for SSM Parameter %s: %s", d.Get("name").(string), keyID.(string)) + log.Printf("[DEBUG] Setting key_id for SSM Parameter %v: %s", d.Get("name"), keyID) paramInput.SetKeyId(keyID.(string)) } - log.Printf("[DEBUG] Waiting for SSM Parameter %q to be updated", d.Get("name").(string)) - _, err := ssmconn.PutParameter(paramInput) - - if err != nil { + log.Printf("[DEBUG] Waiting for SSM Parameter %v to be updated", d.Get("name")) + if _, err := conn.PutParameter(paramInput); err != nil { return errwrap.Wrapf("[ERROR] Error creating SSM parameter: {{err}}", err) } + if err := setTagsSSM(conn, d, d.Get("name").(string), "Parameter"); err != nil { + return errwrap.Wrapf("[ERROR] Error creating SSM parameter tags: {{err}}", err) + } + d.SetId(d.Get("name").(string)) return resourceAwsSsmParameterRead(d, meta) diff --git a/aws/resource_aws_ssm_parameter_test.go b/aws/resource_aws_ssm_parameter_test.go index 7ed5d223738..df03c61a6ac 100644 --- a/aws/resource_aws_ssm_parameter_test.go +++ b/aws/resource_aws_ssm_parameter_test.go @@ -231,27 +231,30 @@ func testAccAWSSSMParameterBasicConfigOverwrite(rName string, value string) stri return fmt.Sprintf(` resource "aws_ssm_parameter" "foo" { name = "test_parameter-%s" + description = "description for parameter %s" type = "String" value = "%s" overwrite = true } -`, rName, value) +`, rName, rName, value) } func testAccAWSSSMParameterSecureConfig(rName string, value string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "secret_foo" { name = "test_secure_parameter-%s" + description = "description for parameter %s" type = "SecureString" value = "%s" } -`, rName, value) +`, rName, rName, value) } func testAccAWSSSMParameterSecureConfigWithKey(rName string, value string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "secret_foo" { name = "test_secure_parameter-%s" + description = "description for parameter %s" type = "SecureString" value = "%s" key_id = "${aws_kms_key.test_key.id}" @@ -261,5 +264,5 @@ resource "aws_kms_key" "test_key" { description = "KMS key 1" deletion_window_in_days = 7 } -`, rName, value) +`, rName, rName, value) } diff --git a/aws/tagsSSM.go b/aws/tagsSSM.go new file mode 100644 index 00000000000..c26eafc5308 --- /dev/null +++ b/aws/tagsSSM.go @@ -0,0 +1,117 @@ +package aws + +import ( + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ssm" + "github.com/hashicorp/terraform/helper/schema" +) + +// setTags is a helper to set the tags for a resource. It expects the +// tags field to be named "tags" +func setTagsSSM(conn *ssm.SSM, d *schema.ResourceData, id, resourceType string) error { + if d.HasChange("tags") { + oraw, nraw := d.GetChange("tags") + o := oraw.(map[string]interface{}) + n := nraw.(map[string]interface{}) + create, remove := diffTagsSSM(tagsFromMapSSM(o), tagsFromMapSSM(n)) + + // Set tags + if len(remove) > 0 { + log.Printf("[DEBUG] Removing tags: %#v", remove) + k := make([]*string, len(remove), len(remove)) + for i, t := range remove { + k[i] = t.Key + } + + _, err := conn.RemoveTagsFromResource(&ssm.RemoveTagsFromResourceInput{ + ResourceId: aws.String(id), + ResourceType: aws.String(resourceType), + TagKeys: k, + }) + if err != nil { + return err + } + } + if len(create) > 0 { + log.Printf("[DEBUG] Creating tags: %#v", create) + _, err := conn.AddTagsToResource(&ssm.AddTagsToResourceInput{ + ResourceId: aws.String(id), + ResourceType: aws.String(resourceType), + Tags: create, + }) + if err != nil { + return err + } + } + } + + return nil +} + +// 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 diffTagsSSM(oldTags, newTags []*ssm.Tag) ([]*ssm.Tag, []*ssm.Tag) { + // 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 []*ssm.Tag + for _, t := range oldTags { + old, ok := create[*t.Key] + if !ok || old != *t.Value { + // Delete it! + remove = append(remove, t) + } + } + + return tagsFromMapSSM(create), remove +} + +// tagsFromMap returns the tags for the given map of data. +func tagsFromMapSSM(m map[string]interface{}) []*ssm.Tag { + result := make([]*ssm.Tag, 0, len(m)) + for k, v := range m { + t := &ssm.Tag{ + Key: aws.String(k), + Value: aws.String(v.(string)), + } + if !tagIgnoredSSM(t) { + result = append(result, t) + } + } + + return result +} + +// tagsToMap turns the list of tags into a map. +func tagsToMapSSM(ts []*ssm.Tag) map[string]string { + result := make(map[string]string) + for _, t := range ts { + if !tagIgnoredSSM(t) { + result[*t.Key] = *t.Value + } + } + + return result +} + +// compare a tag against a list of strings and checks if it should +// be ignored or not +func tagIgnoredSSM(t *ssm.Tag) bool { + filter := []string{"^aws:"} + for _, v := range filter { + log.Printf("[DEBUG] Matching %v with %v\n", v, *t.Key) + if r, _ := regexp.MatchString(v, *t.Key); r == true { + log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value) + return true + } + } + return false +} diff --git a/aws/tagsSSM_test.go b/aws/tagsSSM_test.go new file mode 100644 index 00000000000..33792ae6985 --- /dev/null +++ b/aws/tagsSSM_test.go @@ -0,0 +1,105 @@ +package aws + +import ( + "fmt" + "reflect" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ssm" + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" +) + +// go test -v -run="TestDiffSSMTags" +func TestDiffSSMTags(t *testing.T) { + cases := []struct { + Old, New map[string]interface{} + Create, Remove map[string]string + }{ + // Basic add/remove + { + Old: map[string]interface{}{ + "foo": "bar", + }, + New: map[string]interface{}{ + "bar": "baz", + }, + Create: map[string]string{ + "bar": "baz", + }, + Remove: map[string]string{ + "foo": "bar", + }, + }, + + // Modify + { + Old: map[string]interface{}{ + "foo": "bar", + }, + New: map[string]interface{}{ + "foo": "baz", + }, + Create: map[string]string{ + "foo": "baz", + }, + Remove: map[string]string{ + "foo": "bar", + }, + }, + } + + for i, tc := range cases { + c, r := diffTagsSSM(tagsFromMapSSM(tc.Old), tagsFromMapSSM(tc.New)) + cm := tagsToMapSSM(c) + rm := tagsToMapSSM(r) + 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) + } + } +} + +// go test -v -run="TestIgnoringTagsSSM" +func TestIgnoringTagsSSM(t *testing.T) { + var ignoredTags []*ssm.Tag + ignoredTags = append(ignoredTags, &ssm.Tag{ + Key: aws.String("aws:cloudformation:logical-id"), + Value: aws.String("foo"), + }) + ignoredTags = append(ignoredTags, &ssm.Tag{ + Key: aws.String("aws:foo:bar"), + Value: aws.String("baz"), + }) + for _, tag := range ignoredTags { + if !tagIgnoredSSM(tag) { + t.Fatalf("Tag %v with value %v not ignored, but should be!", *tag.Key, *tag.Value) + } + } +} + +// testAccCheckTags can be used to check the tags on a resource. +func testAccCheckSSMTags( + ts []*ssm.Tag, key string, value string) resource.TestCheckFunc { + return func(s *terraform.State) error { + m := tagsToMapSSM(ts) + v, ok := m[key] + if value != "" && !ok { + return fmt.Errorf("Missing tag: %s", key) + } else if value == "" && ok { + return fmt.Errorf("Extra tag: %s", key) + } + if value == "" { + return nil + } + + if v != value { + return fmt.Errorf("%s: bad value: %s", key, v) + } + + return nil + } +} diff --git a/website/docs/r/ssm_parameter.html.markdown b/website/docs/r/ssm_parameter.html.markdown index b0b991e844d..799c119233e 100644 --- a/website/docs/r/ssm_parameter.html.markdown +++ b/website/docs/r/ssm_parameter.html.markdown @@ -40,8 +40,13 @@ resource "aws_db_instance" "default" { resource "aws_ssm_parameter" "secret" { name = "${var.environment}/database/password/master" + description = "The parameter description" type = "SecureString" value = "${var.database_master_password}" + + tags { + environment = "${var.environment}" + } } ``` @@ -55,13 +60,17 @@ The following arguments are supported: * `name` - (Required) The name of the parameter. * `type` - (Required) The type of the parameter. Valid types are `String`, `StringList` and `SecureString`. * `value` - (Required) The value of the parameter. +* `description` - (Optional) The description of the parameter. * `key_id` - (Optional) The KMS key id or arn for encrypting a SecureString. -* `overwrite` - (Optional) Overwrite an existing parameter. If not specified, will default to `false`. +* `overwrite` - (Optional) Overwrite an existing parameter. If not specified, will default to `true`. +* `allowed_pattern` - (Optional) A regular expression used to validate the parameter value. +* `tags` - (Optional) A mapping of tags to assign to the object. ## Attributes Reference The following attributes are exported: * `name` - (Required) The name of the parameter. +* `description` - (Required) The description of the parameter. * `type` - (Required) The type of the parameter. Valid types are `String`, `StringList` and `SecureString`. * `value` - (Required) The value of the parameter. From 62254c3e3be1a5296167254b789ff4a844d6f39e Mon Sep 17 00:00:00 2001 From: Jocelyn Giroux Date: Mon, 25 Sep 2017 17:07:46 -0400 Subject: [PATCH 2/5] Fix many problem with the SSM parameter - The type and the key_id should not force new resource (that may result in data lost especially if we add a lifecycle rule to protect the value) - Implement the exist function to check if the resource already exist - Handle a bug in AWS that avoid removing a description after it has been set - Handle the key_id properly (if it is not specified by the user, should default to alias/aws/ssm) - Implemented the logic developed by @pmorton in PR #1556 (to better handle the overwrite parameter) --- aws/resource_aws_ssm_parameter.go | 88 +++++++++++++++------- aws/resource_aws_ssm_parameter_test.go | 32 ++++++++ website/docs/r/ssm_parameter.html.markdown | 2 +- 3 files changed, 95 insertions(+), 27 deletions(-) diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index 5c1e5bf9534..44fdf51dd94 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ssm" @@ -16,6 +17,7 @@ func resourceAwsSsmParameter() *schema.Resource { Read: resourceAwsSsmParameterRead, Update: resourceAwsSsmParameterPut, Delete: resourceAwsSsmParameterDelete, + Exists: resourceAwsSmmParameterExists, Schema: map[string]*schema.Schema{ "name": { @@ -31,7 +33,6 @@ func resourceAwsSsmParameter() *schema.Resource { "type": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validateSsmParameterType, }, "value": { @@ -42,14 +43,10 @@ func resourceAwsSsmParameter() *schema.Resource { "key_id": { Type: schema.TypeString, Optional: true, - ForceNew: true, }, "overwrite": { Type: schema.TypeBool, Optional: true, - // The default should be set to true, terraform lifecycle should take care of not overriding the value if it is manually set by the user. - // Otherwise, it is causing a breaking change because the first version did not allow overwrite parameter and overwrite was allowed. - Default: true, }, "allowed_pattern": { Type: schema.TypeString, @@ -61,44 +58,65 @@ func resourceAwsSsmParameter() *schema.Resource { } } +func resourceAwsSmmParameterExists(d *schema.ResourceData, meta interface{}) (bool, error) { + conn := meta.(*AWSClient).ssmconn + + resp, err := conn.GetParameters(&ssm.GetParametersInput{ + Names: []*string{aws.String(d.Get("name").(string))}, + WithDecryption: aws.Bool(true), + }) + + if err != nil { + return false, err + } + return len(resp.InvalidParameters) == 0, nil +} + func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ssmconn log.Printf("[DEBUG] Reading SSM Parameter: %s", d.Id()) - if resp, err := conn.GetParameters(&ssm.GetParametersInput{ + resp, err := conn.GetParameters(&ssm.GetParametersInput{ Names: []*string{aws.String(d.Get("name").(string))}, WithDecryption: aws.Bool(true), - }); err != nil { + }) + if err != nil { return errwrap.Wrapf("[ERROR] Error getting SSM parameter: {{err}}", err) - } else { - if len(resp.InvalidParameters) > 0 { - log.Print("[INFO] The resource no longer exists, marking it for recreation:", d.Id()) - d.SetId("") - return nil - } - param := resp.Parameters[0] - d.Set("name", param.Name) - d.Set("type", param.Type) - d.Set("value", param.Value) } - if resp, err := conn.DescribeParameters(&ssm.DescribeParametersInput{ + param := resp.Parameters[0] + d.Set("name", param.Name) + d.Set("type", param.Type) + d.Set("value", param.Value) + + respDetailed, err := conn.DescribeParameters(&ssm.DescribeParametersInput{ Filters: []*ssm.ParametersFilter{ &ssm.ParametersFilter{ Key: aws.String("Name"), Values: []*string{aws.String(d.Get("name").(string))}, }, }, - }); err != nil { + }) + if err != nil { return errwrap.Wrapf("[ERROR] Error describing SSM parameter: {{err}}", err) - } else { - param := resp.Parameters[0] - d.Set("key_id", param.KeyId) - d.Set("description", param.Description) - d.Set("allowed_pattern", param.AllowedPattern) } + detail := respDetailed.Parameters[0] + if detail.Description != nil { + // Trailing spaces are not considered as a difference + *detail.Description = strings.TrimSpace(*detail.Description) + } + if _, ok := d.GetOk("key_id"); !ok && detail.KeyId != nil && *detail.KeyId == "alias/aws/ssm" { + // If the key_id is not specified and the actual key is set to the AWS default key, we set + // the key to nil to ensure that terraform does not consider that the actual key has changed. + detail.KeyId = nil + } + + d.Set("key_id", detail.KeyId) + d.Set("description", detail.Description) + d.Set("allowed_pattern", detail.AllowedPattern) + if tagList, err := conn.ListTagsForResource(&ssm.ListTagsForResourceInput{ ResourceId: aws.String(d.Get("name").(string)), ResourceType: aws.String("Parameter"), @@ -134,13 +152,20 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error paramInput := &ssm.PutParameterInput{ Name: aws.String(d.Get("name").(string)), - Description: aws.String(d.Get("description").(string)), Type: aws.String(d.Get("type").(string)), Value: aws.String(d.Get("value").(string)), - Overwrite: aws.Bool(d.Get("overwrite").(bool)), + Overwrite: aws.Bool(shouldUpdateSsmParameter(d)), AllowedPattern: aws.String(d.Get("allowed_pattern").(string)), } + if description, ok := d.GetOk("description"); ok { + paramInput.SetDescription(description.(string)) + } else if d.HasChange("description") { + // There is a "bug" in the AWS API and is it not possible to unset a description once + // it has been initially set + paramInput.SetDescription(" ") + } + if keyID, ok := d.GetOk("key_id"); ok { log.Printf("[DEBUG] Setting key_id for SSM Parameter %v: %s", d.Get("name"), keyID) paramInput.SetKeyId(keyID.(string)) @@ -159,3 +184,14 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error return resourceAwsSsmParameterRead(d, meta) } + +func shouldUpdateSsmParameter(d *schema.ResourceData) bool { + // If the user has specified a preference, return their preference + if value, ok := d.GetOkExists("overwrite"); ok { + return value.(bool) + } + + // Since the user has not specified a preference, obey lifecycle rules + // if it is not a new resource, otherwise overwrite should be set to false. + return !d.IsNewResource() +} diff --git a/aws/resource_aws_ssm_parameter_test.go b/aws/resource_aws_ssm_parameter_test.go index df03c61a6ac..d1a53e2caf8 100644 --- a/aws/resource_aws_ssm_parameter_test.go +++ b/aws/resource_aws_ssm_parameter_test.go @@ -266,3 +266,35 @@ resource "aws_kms_key" "test_key" { } `, rName, rName, value) } + +func TestAWSSSMParameterShouldUpdate(t *testing.T) { + data := resourceAwsSsmParameter().TestResourceData() + failure := false + + if !shouldUpdateSsmParameter(data) { + t.Logf("Existing resources should be overwritten if the values don't match!") + failure = true + } + + data.MarkNewResource() + if shouldUpdateSsmParameter(data) { + t.Logf("New resources must never be overwritten, this will overwrite parameters created outside of the system") + failure = true + } + + data = resourceAwsSsmParameter().TestResourceData() + data.Set("overwrite", true) + if !shouldUpdateSsmParameter(data) { + t.Logf("Resources should always be overwritten if the user requests it") + failure = true + } + + data.Set("overwrite", false) + if shouldUpdateSsmParameter(data) { + t.Logf("Resources should never be overwritten if the user requests it") + failure = true + } + if failure { + t.Fail() + } +} diff --git a/website/docs/r/ssm_parameter.html.markdown b/website/docs/r/ssm_parameter.html.markdown index 799c119233e..2c436fd9b5d 100644 --- a/website/docs/r/ssm_parameter.html.markdown +++ b/website/docs/r/ssm_parameter.html.markdown @@ -62,7 +62,7 @@ The following arguments are supported: * `value` - (Required) The value of the parameter. * `description` - (Optional) The description of the parameter. * `key_id` - (Optional) The KMS key id or arn for encrypting a SecureString. -* `overwrite` - (Optional) Overwrite an existing parameter. If not specified, will default to `true`. +* `overwrite` - (Optional) Overwrite an existing parameter. If not specified, will default to `false` if the resource has not been created by terraform to avoid overwrite of existing resource and will default to `true` otherwise (terraform lifecycle rules should then be used to manage the update behavior). * `allowed_pattern` - (Optional) A regular expression used to validate the parameter value. * `tags` - (Optional) A mapping of tags to assign to the object. From 6da6a49d502f028e9234167dbcc54a094b74797b Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Thu, 9 Nov 2017 11:29:15 -0500 Subject: [PATCH 3/5] Include the import for ssm_parameter as suggested by jduchesne --- aws/import_aws_ssm_parameter_test.go | 32 ++++++++++++++++++++++ aws/resource_aws_ssm_parameter.go | 30 +++++++++++--------- website/docs/r/ssm_parameter.html.markdown | 8 ++++++ 3 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 aws/import_aws_ssm_parameter_test.go diff --git a/aws/import_aws_ssm_parameter_test.go b/aws/import_aws_ssm_parameter_test.go new file mode 100644 index 00000000000..fc1a2c79f7a --- /dev/null +++ b/aws/import_aws_ssm_parameter_test.go @@ -0,0 +1,32 @@ +package aws + +import ( + "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" +) + +func TestAccAWSSSMParameter_importBasic(t *testing.T) { + resourceName := "aws_ssm_parameter.foo" + randName := acctest.RandString(5) + randValue := acctest.RandString(5) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSSMParameterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSSMParameterBasicConfig(randName, randValue), + }, + + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"overwrite"}, + }, + }, + }) +} diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index 9348df48a0d..b289799745a 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -1,6 +1,7 @@ package aws import ( + "fmt" "log" "strings" @@ -17,6 +18,9 @@ func resourceAwsSsmParameter() *schema.Resource { Update: resourceAwsSsmParameterPut, Delete: resourceAwsSsmParameterDelete, Exists: resourceAwsSmmParameterExists, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, Schema: map[string]*schema.Schema{ "name": { @@ -58,10 +62,10 @@ func resourceAwsSsmParameter() *schema.Resource { } func resourceAwsSmmParameterExists(d *schema.ResourceData, meta interface{}) (bool, error) { - conn := meta.(*AWSClient).ssmconn + ssmconn := meta.(*AWSClient).ssmconn - resp, err := conn.GetParameters(&ssm.GetParametersInput{ - Names: []*string{aws.String(d.Get("name").(string))}, + resp, err := ssmconn.GetParameters(&ssm.GetParametersInput{ + Names: []*string{aws.String(d.Id())}, WithDecryption: aws.Bool(true), }) @@ -72,12 +76,12 @@ func resourceAwsSmmParameterExists(d *schema.ResourceData, meta interface{}) (bo } func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).ssmconn + ssmconn := meta.(*AWSClient).ssmconn log.Printf("[DEBUG] Reading SSM Parameter: %s", d.Id()) - resp, err := conn.GetParameters(&ssm.GetParametersInput{ - Names: []*string{aws.String(d.Get("name").(string))}, + resp, err := ssmconn.GetParameters(&ssm.GetParametersInput{ + Names: []*string{aws.String(d.Id())}, WithDecryption: aws.Bool(true), }) if err != nil { @@ -89,7 +93,7 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error d.Set("type", param.Type) d.Set("value", param.Value) - respDetailed, err := conn.DescribeParameters(&ssm.DescribeParametersInput{ + respDetailed, err := ssmconn.DescribeParameters(&ssm.DescribeParametersInput{ Filters: []*ssm.ParametersFilter{ &ssm.ParametersFilter{ Key: aws.String("Name"), @@ -116,7 +120,7 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error d.Set("description", detail.Description) d.Set("allowed_pattern", detail.AllowedPattern) - if tagList, err := conn.ListTagsForResource(&ssm.ListTagsForResourceInput{ + if tagList, err := ssmconn.ListTagsForResource(&ssm.ListTagsForResourceInput{ ResourceId: aws.String(d.Get("name").(string)), ResourceType: aws.String("Parameter"), }); err != nil { @@ -129,11 +133,11 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error } func resourceAwsSsmParameterDelete(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).ssmconn + ssmconn := meta.(*AWSClient).ssmconn log.Printf("[INFO] Deleting SSM Parameter: %s", d.Id()) - _, err := conn.DeleteParameter(&ssm.DeleteParameterInput{ + _, err := ssmconn.DeleteParameter(&ssm.DeleteParameterInput{ Name: aws.String(d.Get("name").(string)), }) if err != nil { @@ -145,7 +149,7 @@ func resourceAwsSsmParameterDelete(d *schema.ResourceData, meta interface{}) err } func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).ssmconn + ssmconn := meta.(*AWSClient).ssmconn log.Printf("[INFO] Creating SSM Parameter: %s", d.Get("name").(string)) @@ -171,11 +175,11 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error } log.Printf("[DEBUG] Waiting for SSM Parameter %v to be updated", d.Get("name")) - if _, err := conn.PutParameter(paramInput); err != nil { + if _, err := ssmconn.PutParameter(paramInput); err != nil { return errwrap.Wrapf("[ERROR] Error creating SSM parameter: {{err}}", err) } - if err := setTagsSSM(conn, d, d.Get("name").(string), "Parameter"); err != nil { + if err := setTagsSSM(ssmconn, d, d.Get("name").(string), "Parameter"); err != nil { return errwrap.Wrapf("[ERROR] Error creating SSM parameter tags: {{err}}", err) } diff --git a/website/docs/r/ssm_parameter.html.markdown b/website/docs/r/ssm_parameter.html.markdown index 2c436fd9b5d..953576e9f4e 100644 --- a/website/docs/r/ssm_parameter.html.markdown +++ b/website/docs/r/ssm_parameter.html.markdown @@ -74,3 +74,11 @@ The following attributes are exported: * `description` - (Required) The description of the parameter. * `type` - (Required) The type of the parameter. Valid types are `String`, `StringList` and `SecureString`. * `value` - (Required) The value of the parameter. + +## Import + +SSM Parameters can be imported using the `parameter store name`, e.g. + +``` +$ terraform import aws_ssm_parameter.my_param /my_path/my_paramname +``` From 85289a40cce13cd421765a02d599a8636f0d62c1 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Mon, 5 Mar 2018 11:04:11 -0500 Subject: [PATCH 4/5] Corrected issues with SSM features PR --- aws/resource_aws_ssm_parameter.go | 34 ++++++++++++++++---------- aws/resource_aws_ssm_parameter_test.go | 34 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index 3f91cee44f3..d4f01f0ebfb 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -32,7 +32,6 @@ func resourceAwsSsmParameter() *schema.Resource { "description": { Type: schema.TypeString, Optional: true, - Default: "", }, "type": { Type: schema.TypeString, @@ -52,6 +51,7 @@ func resourceAwsSsmParameter() *schema.Resource { "key_id": { Type: schema.TypeString, Optional: true, + Computed: true, }, "overwrite": { Type: schema.TypeBool, @@ -60,7 +60,6 @@ func resourceAwsSsmParameter() *schema.Resource { "allowed_pattern": { Type: schema.TypeString, Optional: true, - Default: "", }, "tags": tagsSchema(), }, @@ -93,34 +92,45 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error if err != nil { return errwrap.Wrapf("[ERROR] Error getting SSM parameter: {{err}}", err) } + if len(resp.Parameters) == 0 { + log.Printf("[WARN] SSM Param %q not found, removing from state", d.Id()) + d.SetId("") + return nil + } param := resp.Parameters[0] d.Set("name", param.Name) d.Set("type", param.Type) d.Set("value", param.Value) - respDetailed, err := ssmconn.DescribeParameters(&ssm.DescribeParametersInput{ + describeParamsInput := &ssm.DescribeParametersInput{ Filters: []*ssm.ParametersFilter{ &ssm.ParametersFilter{ Key: aws.String("Name"), Values: []*string{aws.String(d.Get("name").(string))}, }, }, - }) + } + detailedParameters := []*ssm.ParameterMetadata{} + err = ssmconn.DescribeParametersPages(describeParamsInput, + func(page *ssm.DescribeParametersOutput, lastPage bool) bool { + detailedParameters = append(detailedParameters, page.Parameters...) + return !lastPage + }) if err != nil { return errwrap.Wrapf("[ERROR] Error describing SSM parameter: {{err}}", err) } + if len(detailedParameters) == 0 { + log.Printf("[WARN] SSM Param %q not found, removing from state", d.Id()) + d.SetId("") + return nil + } - detail := respDetailed.Parameters[0] + detail := detailedParameters[0] if detail.Description != nil { // Trailing spaces are not considered as a difference *detail.Description = strings.TrimSpace(*detail.Description) } - if _, ok := d.GetOk("key_id"); !ok && detail.KeyId != nil && *detail.KeyId == "alias/aws/ssm" { - // If the key_id is not specified and the actual key is set to the AWS default key, we set - // the key to nil to ensure that terraform does not consider that the actual key has changed. - detail.KeyId = nil - } d.Set("key_id", detail.KeyId) d.Set("description", detail.Description) @@ -179,9 +189,7 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error if description, ok := d.GetOk("description"); ok { paramInput.SetDescription(description.(string)) } else if d.HasChange("description") { - // There is a "bug" in the AWS API and is it not possible to unset a description once - // it has been initially set - paramInput.SetDescription(" ") + paramInput.SetDescription("") } if keyID, ok := d.GetOk("key_id"); ok { diff --git a/aws/resource_aws_ssm_parameter_test.go b/aws/resource_aws_ssm_parameter_test.go index c35b29b62f6..469a7d66278 100644 --- a/aws/resource_aws_ssm_parameter_test.go +++ b/aws/resource_aws_ssm_parameter_test.go @@ -80,6 +80,29 @@ func TestAccAWSSSMParameter_update(t *testing.T) { }) } +func TestAccAWSSSMParameter_updateDescription(t *testing.T) { + var param ssm.Parameter + name := fmt.Sprintf("%s_%s", t.Name(), acctest.RandString(10)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSSMParameterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSSMParameterBasicConfigOverwrite(name, "String", "bar"), + }, + { + Config: testAccAWSSSMParameterBasicConfigOverwriteWithoutDescription(name, "String", "bar"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMParameterExists("aws_ssm_parameter.foo", ¶m), + resource.TestCheckResourceAttr("aws_ssm_parameter.foo", "description", ""), + ), + }, + }, + }) +} + func TestAccAWSSSMParameter_changeNameForcesNew(t *testing.T) { var beforeParam, afterParam ssm.Parameter before := fmt.Sprintf("%s_%s", t.Name(), acctest.RandString(10)) @@ -282,6 +305,17 @@ resource "aws_ssm_parameter" "foo" { `, rName, pType, value) } +func testAccAWSSSMParameterBasicConfigOverwriteWithoutDescription(rName, pType, value string) string { + return fmt.Sprintf(` +resource "aws_ssm_parameter" "foo" { + name = "test_parameter-%[1]s" + type = "%[2]s" + value = "%[3]s" + overwrite = true +} +`, rName, pType, value) +} + func testAccAWSSSMParameterSecureConfig(rName string, value string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "secret_foo" { From bd9088e9059382a31d38ad22b4740b87bae832e4 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Tue, 6 Mar 2018 07:42:25 -0500 Subject: [PATCH 5/5] Corrected further issues with SSM features PR + added kms test --- aws/resource_aws_ssm_parameter.go | 21 ++++------- aws/resource_aws_ssm_parameter_test.go | 51 +++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index d4f01f0ebfb..f5d0ac6b3af 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -8,7 +8,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ssm" - "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/helper/schema" ) @@ -90,7 +89,7 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error WithDecryption: aws.Bool(true), }) if err != nil { - return errwrap.Wrapf("[ERROR] Error getting SSM parameter: {{err}}", err) + return fmt.Errorf("error getting SSM parameter: %s", err) } if len(resp.Parameters) == 0 { log.Printf("[WARN] SSM Param %q not found, removing from state", d.Id()) @@ -118,7 +117,7 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error return !lastPage }) if err != nil { - return errwrap.Wrapf("[ERROR] Error describing SSM parameter: {{err}}", err) + return fmt.Errorf("error describing SSM parameter: %s", err) } if len(detailedParameters) == 0 { log.Printf("[WARN] SSM Param %q not found, removing from state", d.Id()) @@ -127,11 +126,6 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error } detail := detailedParameters[0] - if detail.Description != nil { - // Trailing spaces are not considered as a difference - *detail.Description = strings.TrimSpace(*detail.Description) - } - d.Set("key_id", detail.KeyId) d.Set("description", detail.Description) d.Set("allowed_pattern", detail.AllowedPattern) @@ -186,10 +180,9 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error AllowedPattern: aws.String(d.Get("allowed_pattern").(string)), } - if description, ok := d.GetOk("description"); ok { - paramInput.SetDescription(description.(string)) - } else if d.HasChange("description") { - paramInput.SetDescription("") + if d.HasChange("description") { + _, n := d.GetChange("description") + paramInput.Description = aws.String(n.(string)) } if keyID, ok := d.GetOk("key_id"); ok { @@ -199,11 +192,11 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] Waiting for SSM Parameter %v to be updated", d.Get("name")) if _, err := ssmconn.PutParameter(paramInput); err != nil { - return errwrap.Wrapf("[ERROR] Error creating SSM parameter: {{err}}", err) + return fmt.Errorf("error creating SSM parameter: %s", err) } if err := setTagsSSM(ssmconn, d, d.Get("name").(string), "Parameter"); err != nil { - return errwrap.Wrapf("[ERROR] Error creating SSM parameter tags: {{err}}", err) + return fmt.Errorf("error creating SSM parameter tags: %s", err) } d.SetId(d.Get("name").(string)) diff --git a/aws/resource_aws_ssm_parameter_test.go b/aws/resource_aws_ssm_parameter_test.go index 469a7d66278..189c5362cb5 100644 --- a/aws/resource_aws_ssm_parameter_test.go +++ b/aws/resource_aws_ssm_parameter_test.go @@ -168,6 +168,7 @@ func TestAccAWSSSMParameter_secure(t *testing.T) { testAccCheckAWSSSMParameterExists("aws_ssm_parameter.foo", ¶m), resource.TestCheckResourceAttr("aws_ssm_parameter.foo", "value", "secret"), resource.TestCheckResourceAttr("aws_ssm_parameter.foo", "type", "SecureString"), + resource.TestCheckResourceAttr("aws_ssm_parameter.foo", "key_id", "alias/aws/ssm"), // Default SSM key id ), }, }, @@ -176,7 +177,8 @@ func TestAccAWSSSMParameter_secure(t *testing.T) { func TestAccAWSSSMParameter_secure_with_key(t *testing.T) { var param ssm.Parameter - name := fmt.Sprintf("%s_%s", t.Name(), acctest.RandString(10)) + randString := acctest.RandString(10) + name := fmt.Sprintf("%s_%s", t.Name(), randString) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -184,11 +186,44 @@ func TestAccAWSSSMParameter_secure_with_key(t *testing.T) { CheckDestroy: testAccCheckAWSSSMParameterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSSSMParameterSecureConfigWithKey(name, "secret"), + Config: testAccAWSSSMParameterSecureConfigWithKey(name, "secret", randString), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSSMParameterExists("aws_ssm_parameter.secret_foo", ¶m), resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "value", "secret"), resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "type", "SecureString"), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "key_id", "alias/"+randString), + ), + }, + }, + }) +} + +func TestAccAWSSSMParameter_secure_keyUpdate(t *testing.T) { + var param ssm.Parameter + randString := acctest.RandString(10) + name := fmt.Sprintf("%s_%s", t.Name(), randString) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSSMParameterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSSMParameterSecureConfig(name, "secret"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMParameterExists("aws_ssm_parameter.secret_foo", ¶m), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "value", "secret"), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "type", "SecureString"), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "key_id", "alias/aws/ssm"), // Default SSM key id + ), + }, + { + Config: testAccAWSSSMParameterSecureConfigWithKey(name, "secret", randString), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMParameterExists("aws_ssm_parameter.secret_foo", ¶m), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "value", "secret"), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "type", "SecureString"), + resource.TestCheckResourceAttr("aws_ssm_parameter.secret_foo", "key_id", "alias/"+randString), ), }, }, @@ -327,21 +362,27 @@ resource "aws_ssm_parameter" "secret_foo" { `, rName, value) } -func testAccAWSSSMParameterSecureConfigWithKey(rName string, value string) string { +func testAccAWSSSMParameterSecureConfigWithKey(rName string, value string, keyAlias string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "secret_foo" { name = "test_secure_parameter-%[1]s" description = "description for parameter %[1]s" type = "SecureString" value = "%[2]s" - key_id = "${aws_kms_key.test_key.id}" + key_id = "alias/%[3]s" + depends_on = ["aws_kms_alias.test_alias"] } resource "aws_kms_key" "test_key" { description = "KMS key 1" deletion_window_in_days = 7 } -`, rName, value) + +resource "aws_kms_alias" "test_alias" { + name = "alias/%[3]s" + target_key_id = "${aws_kms_key.test_key.id}" +} +`, rName, value, keyAlias) } func TestAWSSSMParameterShouldUpdate(t *testing.T) {