From 75f4cbf4d341054baacb0f5fa23120257fa9bc2b Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Wed, 18 Dec 2019 04:38:43 +0200 Subject: [PATCH] service/ssm: Refactor to use keyvaluetags package and common test practices (#11290) Output from acceptance testing: ``` --- PASS: TestAccAWSSSMActivation_basic (34.55s) --- PASS: TestAccAWSSSMActivation_expirationDate (36.02s) --- PASS: TestAccAWSSSMActivation_update (48.14s) --- PASS: TestAccAWSSSMDocument_automation (36.26s) --- PASS: TestAccAWSSSMDocument_basic (46.11s) --- PASS: TestAccAWSSSMDocument_DocumentFormat_YAML (66.95s) --- PASS: TestAccAWSSSMDocument_params (19.32s) --- PASS: TestAccAWSSSMDocument_permission_batching (19.49s) --- PASS: TestAccAWSSSMDocument_permission_change (73.30s) --- PASS: TestAccAWSSSMDocument_permission_private (42.17s) --- PASS: TestAccAWSSSMDocument_permission_public (30.52s) --- PASS: TestAccAWSSSMDocument_SchemaVersion_1 (48.34s) --- PASS: TestAccAWSSSMDocument_session (25.64s) --- PASS: TestAccAWSSSMDocument_Tags (65.48s) --- PASS: TestAccAWSSSMDocument_update (32.67s) --- PASS: TestAccAWSSSMMaintenanceWindow_basic (13.84s) --- PASS: TestAccAWSSSMMaintenanceWindow_Cutoff (31.15s) --- PASS: TestAccAWSSSMMaintenanceWindow_disappears (12.43s) --- PASS: TestAccAWSSSMMaintenanceWindow_Duration (32.88s) --- PASS: TestAccAWSSSMMaintenanceWindow_Enabled (32.06s) --- PASS: TestAccAWSSSMMaintenanceWindow_EndDate (40.70s) --- PASS: TestAccAWSSSMMaintenanceWindow_multipleUpdates (26.45s) --- PASS: TestAccAWSSSMMaintenanceWindow_Schedule (34.80s) --- PASS: TestAccAWSSSMMaintenanceWindow_ScheduleTimezone (42.91s) --- PASS: TestAccAWSSSMMaintenanceWindow_StartDate (41.86s) --- PASS: TestAccAWSSSMMaintenanceWindow_tags (40.01s) --- PASS: TestAccAWSSSMMaintenanceWindowTarget_basic (16.03s) --- PASS: TestAccAWSSSMMaintenanceWindowTarget_noNameOrDescription (15.03s) --- PASS: TestAccAWSSSMMaintenanceWindowTarget_update (21.87s) --- PASS: TestAccAWSSSMMaintenanceWindowTarget_validation (7.58s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (24.99s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_emptyNotificationConfig (16.71s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationAutomationParameters (24.40s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationLambdaParameters (45.00s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationRunCommandParameters (29.90s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskInvocationStepFunctionParameters (16.33s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_TaskParameters (15.49s) --- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (22.87s) --- PASS: TestAccAWSSSMParameter_basic (21.89s) --- PASS: TestAccAWSSSMParameter_changeNameForcesNew (32.04s) --- PASS: TestAccAWSSSMParameter_disappears (17.83s) --- PASS: TestAccAWSSSMParameter_fullPath (19.99s) --- PASS: TestAccAWSSSMParameter_overwrite (32.00s) --- PASS: TestAccAWSSSMParameter_secure (20.16s) --- PASS: TestAccAWSSSMParameter_secure_keyUpdate (42.76s) --- PASS: TestAccAWSSSMParameter_secure_with_key (38.15s) --- PASS: TestAccAWSSSMParameter_tags (37.14s) --- PASS: TestAccAWSSSMParameter_Tier (41.45s) --- PASS: TestAccAWSSSMParameter_updateDescription (32.29s) --- PASS: TestAccAWSSSMPatchBaseline_basic (22.51s) --- PASS: TestAccAWSSSMPatchBaseline_disappears (14.07s) --- PASS: TestAccAWSSSMPatchBaseline_OperatingSystem (20.14s) --- PASS: TestAccAWSSSMPatchBaseline_tags (26.73s) ``` --- aws/resource_aws_ssm_activation.go | 6 +- aws/resource_aws_ssm_activation_test.go | 17 +- aws/resource_aws_ssm_document.go | 22 +-- aws/resource_aws_ssm_document_test.go | 150 +++++++--------- aws/resource_aws_ssm_maintenance_window.go | 19 +- ...esource_aws_ssm_maintenance_window_test.go | 68 ++++--- aws/resource_aws_ssm_parameter.go | 30 ++-- aws/resource_aws_ssm_parameter_test.go | 66 ++++--- aws/resource_aws_ssm_patch_baseline.go | 19 +- aws/resource_aws_ssm_patch_baseline_test.go | 166 +++++++++++------- aws/tagsSSM.go | 137 --------------- aws/tagsSSM_test.go | 79 --------- 12 files changed, 329 insertions(+), 450 deletions(-) delete mode 100644 aws/tagsSSM.go delete mode 100644 aws/tagsSSM_test.go diff --git a/aws/resource_aws_ssm_activation.go b/aws/resource_aws_ssm_activation.go index b79972fc44cf..8bd53cc977e8 100644 --- a/aws/resource_aws_ssm_activation.go +++ b/aws/resource_aws_ssm_activation.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsSsmActivation() *schema.Resource { @@ -92,7 +93,7 @@ func resourceAwsSsmActivationCreate(d *schema.ResourceData, meta interface{}) er activationInput.RegistrationLimit = aws.Int64(int64(d.Get("registration_limit").(int))) } if v, ok := d.GetOk("tags"); ok { - activationInput.Tags = tagsFromMapSSM(v.(map[string]interface{})) + activationInput.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SsmTags() } // Retry to allow iam_role to be created and policy attachment to take place @@ -160,6 +161,9 @@ func resourceAwsSsmActivationRead(d *schema.ResourceData, meta interface{}) erro d.Set("iam_role", activation.IamRole) d.Set("registration_limit", activation.RegistrationLimit) d.Set("registration_count", activation.RegistrationsCount) + if err := d.Set("tags", keyvaluetags.SsmKeyValueTags(activation.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) + } return nil } diff --git a/aws/resource_aws_ssm_activation_test.go b/aws/resource_aws_ssm_activation_test.go index a6088d3ba94f..96a11efd2448 100644 --- a/aws/resource_aws_ssm_activation_test.go +++ b/aws/resource_aws_ssm_activation_test.go @@ -37,6 +37,7 @@ func TestAccAWSSSMActivation_basic(t *testing.T) { func TestAccAWSSSMActivation_update(t *testing.T) { var ssmActivation1, ssmActivation2 ssm.Activation name := acctest.RandString(10) + resourceName := "aws_ssm_activation.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -45,19 +46,19 @@ func TestAccAWSSSMActivation_update(t *testing.T) { { Config: testAccAWSSSMActivationBasicConfig(name, "My Activation"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMActivationExists("aws_ssm_activation.foo", &ssmActivation1), - resource.TestCheckResourceAttrSet("aws_ssm_activation.foo", "activation_code"), - resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.%", "1"), - resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.Name", "My Activation"), + testAccCheckAWSSSMActivationExists(resourceName, &ssmActivation1), + resource.TestCheckResourceAttrSet(resourceName, "activation_code"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", "My Activation"), ), }, { Config: testAccAWSSSMActivationBasicConfig(name, "Foo"), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMActivationExists("aws_ssm_activation.foo", &ssmActivation2), - resource.TestCheckResourceAttrSet("aws_ssm_activation.foo", "activation_code"), - resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.%", "1"), - resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.Name", "Foo"), + testAccCheckAWSSSMActivationExists(resourceName, &ssmActivation2), + resource.TestCheckResourceAttrSet(resourceName, "activation_code"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.Name", "Foo"), testAccCheckAWSSSMActivationRecreated(t, &ssmActivation1, &ssmActivation2), ), }, diff --git a/aws/resource_aws_ssm_document.go b/aws/resource_aws_ssm_document.go index f6b299470043..c28f6227cfe7 100644 --- a/aws/resource_aws_ssm_document.go +++ b/aws/resource_aws_ssm_document.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) const ( @@ -160,7 +161,7 @@ func resourceAwsSsmDocumentCreate(d *schema.ResourceData, meta interface{}) erro } if v, ok := d.GetOk("tags"); ok { - docInput.Tags = tagsFromMapSSM(v.(map[string]interface{})) + docInput.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SsmTags() } log.Printf("[DEBUG] Waiting for SSM Document %q to be created", d.Get("name").(string)) @@ -192,10 +193,6 @@ func resourceAwsSsmDocumentCreate(d *schema.ResourceData, meta interface{}) erro log.Printf("[DEBUG] Not setting permissions for %q", d.Id()) } - if err := setTagsSSM(ssmconn, d, d.Id(), ssm.ResourceTypeForTaggingDocument); err != nil { - return fmt.Errorf("error setting SSM Document tags: %s", err) - } - return resourceAwsSsmDocumentRead(d, meta) } @@ -302,14 +299,9 @@ func resourceAwsSsmDocumentRead(d *schema.ResourceData, meta interface{}) error return err } - tagList, err := ssmconn.ListTagsForResource(&ssm.ListTagsForResourceInput{ - ResourceId: aws.String(d.Id()), - ResourceType: aws.String(ssm.ResourceTypeForTaggingDocument), - }) - if err != nil { - return fmt.Errorf("error listing SSM Document tags for %s: %s", d.Id(), err) + if err := d.Set("tags", keyvaluetags.SsmKeyValueTags(doc.Tags).IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) } - d.Set("tags", tagsToMapSSM(tagList.TagList)) return nil } @@ -318,8 +310,10 @@ func resourceAwsSsmDocumentUpdate(d *schema.ResourceData, meta interface{}) erro ssmconn := meta.(*AWSClient).ssmconn if d.HasChange("tags") { - if err := setTagsSSM(ssmconn, d, d.Id(), ssm.ResourceTypeForTaggingDocument); err != nil { - return fmt.Errorf("error setting SSM Document tags: %s", err) + o, n := d.GetChange("tags") + + if err := keyvaluetags.SsmUpdateTags(ssmconn, d.Id(), ssm.ResourceTypeForTaggingDocument, o, n); err != nil { + return fmt.Errorf("error updating SSM Document (%s) tags: %s", d.Id(), err) } } diff --git a/aws/resource_aws_ssm_document_test.go b/aws/resource_aws_ssm_document_test.go index bae12b9b037a..790074dd9368 100644 --- a/aws/resource_aws_ssm_document_test.go +++ b/aws/resource_aws_ssm_document_test.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -15,6 +14,7 @@ import ( func TestAccAWSSSMDocument_basic(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -23,17 +23,14 @@ func TestAccAWSSSMDocument_basic(t *testing.T) { { Config: testAccAWSSSMDocumentBasicConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "document_format", "JSON"), - resource.TestMatchResourceAttr("aws_ssm_document.foo", "arn", - regexp.MustCompile(`^arn:aws:ssm:[a-z]{2}-[a-z]+-\d{1}:\d{12}:document/.*$`)), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "tags.%", "1"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "tags.Name", "My Document"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "document_format", "JSON"), + testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ssm", fmt.Sprintf("document/%s", name)), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -43,6 +40,7 @@ func TestAccAWSSSMDocument_basic(t *testing.T) { func TestAccAWSSSMDocument_update(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -51,28 +49,23 @@ func TestAccAWSSSMDocument_update(t *testing.T) { { Config: testAccAWSSSMDocument20Config(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "schema_version", "2.0"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "latest_version", "1"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "default_version", "1"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "schema_version", "2.0"), + resource.TestCheckResourceAttr(resourceName, "latest_version", "1"), + resource.TestCheckResourceAttr(resourceName, "default_version", "1"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSSSMDocument20UpdatedConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "latest_version", "2"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "default_version", "2"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "latest_version", "2"), + resource.TestCheckResourceAttr(resourceName, "default_version", "2"), ), }, }, @@ -81,6 +74,7 @@ func TestAccAWSSSMDocument_update(t *testing.T) { func TestAccAWSSSMDocument_permission_public(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -89,15 +83,13 @@ func TestAccAWSSSMDocument_permission_public(t *testing.T) { { Config: testAccAWSSSMDocumentPublicPermissionConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.account_ids", "all"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), + resource.TestCheckResourceAttr(resourceName, "permissions.account_ids", "all"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -107,6 +99,7 @@ func TestAccAWSSSMDocument_permission_public(t *testing.T) { func TestAccAWSSSMDocument_permission_private(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" ids := "123456789012" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -116,13 +109,12 @@ func TestAccAWSSSMDocument_permission_private(t *testing.T) { { Config: testAccAWSSSMDocumentPrivatePermissionConfig(name, ids), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -132,6 +124,7 @@ func TestAccAWSSSMDocument_permission_private(t *testing.T) { func TestAccAWSSSMDocument_permission_batching(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" ids := "123456789012,123456789013,123456789014,123456789015,123456789016,123456789017,123456789018,123456789019,123456789020,123456789021,123456789022,123456789023,123456789024,123456789025,123456789026,123456789027,123456789028,123456789029,123456789030,123456789031,123456789032" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -141,13 +134,12 @@ func TestAccAWSSSMDocument_permission_batching(t *testing.T) { { Config: testAccAWSSSMDocumentPrivatePermissionConfig(name, ids), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -157,6 +149,7 @@ func TestAccAWSSSMDocument_permission_batching(t *testing.T) { func TestAccAWSSSMDocument_permission_change(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" idsInitial := "123456789012,123456789013" idsRemove := "123456789012" idsAdd := "123456789012,123456789014" @@ -168,33 +161,30 @@ func TestAccAWSSSMDocument_permission_change(t *testing.T) { { Config: testAccAWSSSMDocumentPrivatePermissionConfig(name, idsInitial), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "permissions.account_ids", idsInitial), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), + resource.TestCheckResourceAttr(resourceName, "permissions.account_ids", idsInitial), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSSSMDocumentPrivatePermissionConfig(name, idsRemove), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "permissions.account_ids", idsRemove), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), + resource.TestCheckResourceAttr(resourceName, "permissions.account_ids", idsRemove), ), }, { Config: testAccAWSSSMDocumentPrivatePermissionConfig(name, idsAdd), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "permissions.type", "Share"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "permissions.account_ids", idsAdd), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "permissions.type", "Share"), + resource.TestCheckResourceAttr(resourceName, "permissions.account_ids", idsAdd), ), }, }, @@ -203,6 +193,7 @@ func TestAccAWSSSMDocument_permission_change(t *testing.T) { func TestAccAWSSSMDocument_params(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -211,23 +202,17 @@ func TestAccAWSSSMDocument_params(t *testing.T) { { Config: testAccAWSSSMDocumentParamConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.0.name", "commands"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.0.type", "StringList"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.1.name", "workingDirectory"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.1.type", "String"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.2.name", "executionTimeout"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "parameter.2.type", "String"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "parameter.0.name", "commands"), + resource.TestCheckResourceAttr(resourceName, "parameter.0.type", "StringList"), + resource.TestCheckResourceAttr(resourceName, "parameter.1.name", "workingDirectory"), + resource.TestCheckResourceAttr(resourceName, "parameter.1.type", "String"), + resource.TestCheckResourceAttr(resourceName, "parameter.2.name", "executionTimeout"), + resource.TestCheckResourceAttr(resourceName, "parameter.2.type", "String"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -237,6 +222,7 @@ func TestAccAWSSSMDocument_params(t *testing.T) { func TestAccAWSSSMDocument_automation(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -245,13 +231,12 @@ func TestAccAWSSSMDocument_automation(t *testing.T) { { Config: testAccAWSSSMDocumentTypeAutomationConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "document_type", "Automation"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "document_type", "Automation"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -293,6 +278,7 @@ func TestAccAWSSSMDocument_SchemaVersion_1(t *testing.T) { func TestAccAWSSSMDocument_session(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -301,13 +287,12 @@ func TestAccAWSSSMDocument_session(t *testing.T) { { Config: testAccAWSSSMDocumentTypeSessionConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr( - "aws_ssm_document.foo", "document_type", "Session"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "document_type", "Session"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -317,6 +302,7 @@ func TestAccAWSSSMDocument_session(t *testing.T) { func TestAccAWSSSMDocument_DocumentFormat_YAML(t *testing.T) { name := acctest.RandString(10) + resourceName := "aws_ssm_document.foo" content1 := ` --- schemaVersion: '2.2' @@ -347,22 +333,22 @@ mainSteps: { Config: testAccAWSSSMDocumentConfig_DocumentFormat_YAML(name, content1), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "content", content1+"\n"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "document_format", "YAML"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "content", content1+"\n"), + resource.TestCheckResourceAttr(resourceName, "document_format", "YAML"), ), }, { - ResourceName: "aws_ssm_document.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSSSMDocumentConfig_DocumentFormat_YAML(name, content2), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMDocumentExists("aws_ssm_document.foo"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "content", content2+"\n"), - resource.TestCheckResourceAttr("aws_ssm_document.foo", "document_format", "YAML"), + testAccCheckAWSSSMDocumentExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "content", content2+"\n"), + resource.TestCheckResourceAttr(resourceName, "document_format", "YAML"), ), }, }, @@ -447,7 +433,7 @@ func testAccCheckAWSSSMDocumentDestroy(s *terraform.State) error { if err != nil { // InvalidDocument means it's gone, this is good - if wserr, ok := err.(awserr.Error); ok && wserr.Code() == "InvalidDocument" { + if wserr, ok := err.(awserr.Error); ok && wserr.Code() == ssm.ErrCodeInvalidDocument { return nil } return err @@ -470,13 +456,9 @@ Based on examples from here: https://docs.aws.amazon.com/AWSEC2/latest/WindowsGu func testAccAWSSSMDocumentBasicConfig(rName string) string { return fmt.Sprintf(` resource "aws_ssm_document" "foo" { - name = "test_document-%s" + name = "%s" document_type = "Command" - tags = { - Name = "My Document" - } - content = < 0 { - return fmt.Errorf("Expected AWS SSM Maintenance Document to be gone, but was still found") + // Return nil if the SSM Maintenance Window is already destroyed + if isAWSErr(err, ssm.ErrCodeDoesNotExistException, "") { + continue } - return nil + return err } return nil @@ -488,19 +496,35 @@ resource "aws_ssm_maintenance_window" "test" { `, rName) } -func testAccAWSSSMMaintenanceWindowConfigTags(rName string) string { +func testAccAWSSSMMaintenanceWindowConfigTags1(rName, tagKey1, tagValue1 string) string { return fmt.Sprintf(` resource "aws_ssm_maintenance_window" "test" { cutoff = 1 duration = 3 - name = %q + name = %[1]q schedule = "cron(0 16 ? * TUE *)" tags = { - Name = "My Maintenance Window" + %[2]q = %[3]q } } -`, rName) +`, rName, tagKey1, tagValue1) +} + +func testAccAWSSSMMaintenanceWindowConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_ssm_maintenance_window" "test" { + cutoff = 1 + duration = 3 + name = %[1]q + schedule = "cron(0 16 ? * TUE *)" + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } +} +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) } func testAccAWSSSMMaintenanceWindowConfigCutoff(rName string, cutoff int) string { diff --git a/aws/resource_aws_ssm_parameter.go b/aws/resource_aws_ssm_parameter.go index 95ecb8e72d09..6aef7c26db8e 100644 --- a/aws/resource_aws_ssm_parameter.go +++ b/aws/resource_aws_ssm_parameter.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) func resourceAwsSsmParameter() *schema.Resource { @@ -122,7 +123,8 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error } param := resp.Parameter - d.Set("name", param.Name) + name := *param.Name + d.Set("name", name) d.Set("type", param.Type) d.Set("value", param.Value) d.Set("version", param.Version) @@ -132,7 +134,7 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error { Key: aws.String("Name"), Option: aws.String("Equals"), - Values: []*string{aws.String(d.Get("name").(string))}, + Values: []*string{aws.String(name)}, }, }, } @@ -156,13 +158,14 @@ func resourceAwsSsmParameterRead(d *schema.ResourceData, meta interface{}) error } d.Set("allowed_pattern", detail.AllowedPattern) - if tagList, err := ssmconn.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)) + tags, err := keyvaluetags.SsmListTags(ssmconn, name, ssm.ResourceTypeForTaggingParameter) + + if err != nil { + return fmt.Errorf("error listing tags for SSM Maintenance Window (%s): %s", name, err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) } arn := arn.ARN{ @@ -228,8 +231,13 @@ func resourceAwsSsmParameterPut(d *schema.ResourceData, meta interface{}) error return fmt.Errorf("error creating SSM parameter: %s", err) } - if err := setTagsSSM(ssmconn, d, d.Get("name").(string), "Parameter"); err != nil { - return fmt.Errorf("error creating SSM parameter tags: %s", err) + name := d.Get("name").(string) + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.SsmUpdateTags(ssmconn, name, ssm.ResourceTypeForTaggingParameter, o, n); err != nil { + return fmt.Errorf("error updating SSM Parameter (%s) tags: %s", name, 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 173b6dfe90ac..22a86267c686 100644 --- a/aws/resource_aws_ssm_parameter_test.go +++ b/aws/resource_aws_ssm_parameter_test.go @@ -29,8 +29,7 @@ func TestAccAWSSSMParameter_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "value", "test2"), resource.TestCheckResourceAttr(resourceName, "type", "String"), resource.TestCheckResourceAttr(resourceName, "tier", "Standard"), - resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), - resource.TestCheckResourceAttr(resourceName, "tags.Name", "My Parameter"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttrSet(resourceName, "version"), ), }, @@ -138,9 +137,9 @@ func TestAccAWSSSMParameter_overwrite(t *testing.T) { }) } -func TestAccAWSSSMParameter_updateTags(t *testing.T) { +func TestAccAWSSSMParameter_tags(t *testing.T) { var param ssm.Parameter - name := fmt.Sprintf("%s_%s", t.Name(), acctest.RandString(10)) + rName := fmt.Sprintf("%s_%s", t.Name(), acctest.RandString(10)) resourceName := "aws_ssm_parameter.test" resource.ParallelTest(t, resource.TestCase{ @@ -149,7 +148,12 @@ func TestAccAWSSSMParameter_updateTags(t *testing.T) { CheckDestroy: testAccCheckAWSSSMParameterDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSSSMParameterBasicConfig(name, "String", "test2"), + Config: testAccAWSSSMParameterBasicConfigTags1(rName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMParameterExists(resourceName, ¶m), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), }, { ResourceName: resourceName, @@ -158,12 +162,20 @@ func TestAccAWSSSMParameter_updateTags(t *testing.T) { ImportStateVerifyIgnore: []string{"overwrite"}, }, { - Config: testAccAWSSSMParameterBasicConfigTagsUpdated(name, "String", "test3"), + Config: testAccAWSSSMParameterBasicConfigTags2(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSSMParameterExists(resourceName, ¶m), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), - resource.TestCheckResourceAttr(resourceName, "tags.Name", "My Parameter Updated"), - resource.TestCheckResourceAttr(resourceName, "tags.AnotherTag", "AnotherTagValue"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSSSMParameterBasicConfigTags1(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMParameterExists(resourceName, ¶m), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), }, }, @@ -449,13 +461,9 @@ func testAccCheckAWSSSMParameterDestroy(s *terraform.State) error { func testAccAWSSSMParameterBasicConfig(rName, pType, value string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "test" { - name = "%s" - type = "%s" - value = "%s" - - tags = { - Name = "My Parameter" - } + name = %[1]q + type = %[2]q + value = %[3]q } `, rName, pType, value) } @@ -471,19 +479,33 @@ resource "aws_ssm_parameter" "test" { `, rName, tier) } -func testAccAWSSSMParameterBasicConfigTagsUpdated(rName, pType, value string) string { +func testAccAWSSSMParameterBasicConfigTags1(rName, tagKey1, tagValue1 string) string { return fmt.Sprintf(` resource "aws_ssm_parameter" "test" { - name = "%s" - type = "%s" - value = "%s" + name = %[1]q + type = "String" + value = %[1]q tags = { - Name = "My Parameter Updated" - AnotherTag = "AnotherTagValue" + %[2]q = %[3]q } } -`, rName, pType, value) +`, rName, tagKey1, tagValue1) +} + +func testAccAWSSSMParameterBasicConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_ssm_parameter" "test" { + name = %[1]q + type = "String" + value = %[1]q + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } +} +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) } func testAccAWSSSMParameterBasicConfigOverwrite(rName, pType, value string) string { diff --git a/aws/resource_aws_ssm_patch_baseline.go b/aws/resource_aws_ssm_patch_baseline.go index b25929b47b7c..d1853aa69d7a 100644 --- a/aws/resource_aws_ssm_patch_baseline.go +++ b/aws/resource_aws_ssm_patch_baseline.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/service/ssm" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" ) var ssmPatchComplianceLevels = []string{ @@ -157,7 +158,7 @@ func resourceAwsSsmPatchBaselineCreate(d *schema.ResourceData, meta interface{}) } if v, ok := d.GetOk("tags"); ok { - params.Tags = tagsFromMapSSM(v.(map[string]interface{})) + params.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SsmTags() } if v, ok := d.GetOk("description"); ok { @@ -235,8 +236,10 @@ func resourceAwsSsmPatchBaselineUpdate(d *schema.ResourceData, meta interface{}) } if d.HasChange("tags") { - if err := setTagsSSM(ssmconn, d, d.Id(), ssm.ResourceTypeForTaggingPatchBaseline); err != nil { - return fmt.Errorf("error setting tags for SSM Patch Baseline (%s): %s", d.Id(), err) + o, n := d.GetChange("tags") + + if err := keyvaluetags.SsmUpdateTags(ssmconn, d.Id(), ssm.ResourceTypeForTaggingPatchBaseline, o, n); err != nil { + return fmt.Errorf("error updating SSM Patch Baseline (%s) tags: %s", d.Id(), err) } } @@ -274,8 +277,14 @@ func resourceAwsSsmPatchBaselineRead(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("Error setting approval rules error: %#v", err) } - if err := saveTagsSSM(ssmconn, d, d.Id(), ssm.ResourceTypeForTaggingPatchBaseline); err != nil { - return fmt.Errorf("error saving tags for SSM Patch Baseline (%s): %s", d.Id(), err) + tags, err := keyvaluetags.SsmListTags(ssmconn, d.Id(), ssm.ResourceTypeForTaggingPatchBaseline) + + if err != nil { + return fmt.Errorf("error listing tags for SSM Patch Baseline (%s): %s", d.Id(), err) + } + + if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil { + return fmt.Errorf("error setting tags: %s", err) } return nil diff --git a/aws/resource_aws_ssm_patch_baseline_test.go b/aws/resource_aws_ssm_patch_baseline_test.go index d568dd4dacd5..f9bdc34cf9bf 100644 --- a/aws/resource_aws_ssm_patch_baseline_test.go +++ b/aws/resource_aws_ssm_patch_baseline_test.go @@ -14,6 +14,7 @@ import ( func TestAccAWSSSMPatchBaseline_basic(t *testing.T) { var before, after ssm.PatchBaselineIdentity name := acctest.RandString(10) + resourceName := "aws_ssm_patch_baseline.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -22,45 +23,31 @@ func TestAccAWSSSMPatchBaseline_basic(t *testing.T) { { Config: testAccAWSSSMPatchBaselineBasicConfig(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMPatchBaselineExists("aws_ssm_patch_baseline.foo", &before), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches.#", "1"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches.2062620480", "KB123456"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "name", fmt.Sprintf("patch-baseline-%s", name)), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches_compliance_level", ssm.PatchComplianceLevelCritical), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "description", "Baseline containing all updates approved for production systems"), - resource.TestCheckResourceAttr("aws_ssm_patch_baseline.foo", "tags.%", "1"), - resource.TestCheckResourceAttr("aws_ssm_patch_baseline.foo", "tags.Name", "My Patch Baseline"), + testAccCheckAWSSSMPatchBaselineExists(resourceName, &before), + resource.TestCheckResourceAttr(resourceName, "approved_patches.#", "1"), + resource.TestCheckResourceAttr(resourceName, "approved_patches.2062620480", "KB123456"), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("patch-baseline-%s", name)), + resource.TestCheckResourceAttr(resourceName, "approved_patches_compliance_level", ssm.PatchComplianceLevelCritical), + resource.TestCheckResourceAttr(resourceName, "description", "Baseline containing all updates approved for production systems"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { - ResourceName: "aws_ssm_patch_baseline.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSSSMPatchBaselineBasicConfigUpdated(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMPatchBaselineExists("aws_ssm_patch_baseline.foo", &after), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches.#", "2"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches.2062620480", "KB123456"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches.2291496788", "KB456789"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "name", fmt.Sprintf("updated-patch-baseline-%s", name)), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approved_patches_compliance_level", ssm.PatchComplianceLevelHigh), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "description", "Baseline containing all updates approved for production systems - August 2017"), - resource.TestCheckResourceAttr("aws_ssm_patch_baseline.foo", "tags.%", "2"), - resource.TestCheckResourceAttr("aws_ssm_patch_baseline.foo", "tags.Name", "My Patch Baseline Aug 17"), - resource.TestCheckResourceAttr("aws_ssm_patch_baseline.foo", "tags.Environment", "production"), + testAccCheckAWSSSMPatchBaselineExists(resourceName, &after), + resource.TestCheckResourceAttr(resourceName, "approved_patches.#", "2"), + resource.TestCheckResourceAttr(resourceName, "approved_patches.2062620480", "KB123456"), + resource.TestCheckResourceAttr(resourceName, "approved_patches.2291496788", "KB456789"), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("updated-patch-baseline-%s", name)), + resource.TestCheckResourceAttr(resourceName, "approved_patches_compliance_level", ssm.PatchComplianceLevelHigh), + resource.TestCheckResourceAttr(resourceName, "description", "Baseline containing all updates approved for production systems - August 2017"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), func(*terraform.State) error { if *before.BaselineId != *after.BaselineId { t.Fatal("Baseline IDs changed unexpectedly") @@ -73,6 +60,49 @@ func TestAccAWSSSMPatchBaseline_basic(t *testing.T) { }) } +func TestAccAWSSSMPatchBaseline_tags(t *testing.T) { + var patch ssm.PatchBaselineIdentity + name := acctest.RandString(10) + resourceName := "aws_ssm_patch_baseline.foo" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSSMPatchBaselineDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSSMPatchBaselineBasicConfigTags1(name, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMPatchBaselineExists(resourceName, &patch), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSSMPatchBaselineBasicConfigTags2(name, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMPatchBaselineExists(resourceName, &patch), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSSSMPatchBaselineBasicConfigTags1(name, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSSMPatchBaselineExists(resourceName, &patch), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + }, + }) +} + func TestAccAWSSSMPatchBaseline_disappears(t *testing.T) { var identity ssm.PatchBaselineIdentity name := acctest.RandString(10) @@ -98,6 +128,7 @@ func TestAccAWSSSMPatchBaseline_disappears(t *testing.T) { func TestAccAWSSSMPatchBaseline_OperatingSystem(t *testing.T) { var before, after ssm.PatchBaselineIdentity name := acctest.RandString(10) + resourceName := "aws_ssm_patch_baseline.foo" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -106,40 +137,29 @@ func TestAccAWSSSMPatchBaseline_OperatingSystem(t *testing.T) { { Config: testAccAWSSSMPatchBaselineConfigWithOperatingSystem(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMPatchBaselineExists("aws_ssm_patch_baseline.foo", &before), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.#", "1"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.approve_after_days", "7"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.patch_filter.#", "2"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.compliance_level", ssm.PatchComplianceLevelCritical), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.enable_non_security", "true"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "operating_system", "AMAZON_LINUX"), + testAccCheckAWSSSMPatchBaselineExists(resourceName, &before), + resource.TestCheckResourceAttr(resourceName, "approval_rule.#", "1"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.approve_after_days", "7"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.patch_filter.#", "2"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.compliance_level", ssm.PatchComplianceLevelCritical), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.enable_non_security", "true"), + resource.TestCheckResourceAttr(resourceName, "operating_system", "AMAZON_LINUX"), ), }, { - ResourceName: "aws_ssm_patch_baseline.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, { Config: testAccAWSSSMPatchBaselineConfigWithOperatingSystemUpdated(name), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSSMPatchBaselineExists("aws_ssm_patch_baseline.foo", &after), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.#", "1"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.approve_after_days", "7"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.patch_filter.#", "2"), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "approval_rule.0.compliance_level", ssm.PatchComplianceLevelInformational), - resource.TestCheckResourceAttr( - "aws_ssm_patch_baseline.foo", "operating_system", ssm.OperatingSystemWindows), + testAccCheckAWSSSMPatchBaselineExists(resourceName, &after), + resource.TestCheckResourceAttr(resourceName, "approval_rule.#", "1"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.approve_after_days", "7"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.patch_filter.#", "2"), + resource.TestCheckResourceAttr(resourceName, "approval_rule.0.compliance_level", ssm.PatchComplianceLevelInformational), + resource.TestCheckResourceAttr(resourceName, "operating_system", ssm.OperatingSystemWindows), testAccCheckAwsSsmPatchBaselineRecreated(t, &before, &after), ), }, @@ -249,12 +269,39 @@ resource "aws_ssm_patch_baseline" "foo" { description = "Baseline containing all updates approved for production systems" approved_patches = ["KB123456"] approved_patches_compliance_level = "CRITICAL" +} +`, rName) +} + +func testAccAWSSSMPatchBaselineBasicConfigTags1(rName, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_ssm_patch_baseline" "foo" { + name = %[1]q + description = "Baseline containing all updates approved for production systems" + approved_patches = ["KB123456"] + approved_patches_compliance_level = "CRITICAL" tags = { - Name = "My Patch Baseline" + %[2]q = %[3]q } } -`, rName) +`, rName, tagKey1, tagValue1) +} + +func testAccAWSSSMPatchBaselineBasicConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_ssm_patch_baseline" "foo" { + name = %[1]q + description = "Baseline containing all updates approved for production systems" + approved_patches = ["KB123456"] + approved_patches_compliance_level = "CRITICAL" + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } +} +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) } func testAccAWSSSMPatchBaselineBasicConfigUpdated(rName string) string { @@ -264,11 +311,6 @@ resource "aws_ssm_patch_baseline" "foo" { description = "Baseline containing all updates approved for production systems - August 2017" approved_patches = ["KB123456", "KB456789"] approved_patches_compliance_level = "HIGH" - - tags = { - Name = "My Patch Baseline Aug 17" - Environment = "production" - } } `, rName) } diff --git a/aws/tagsSSM.go b/aws/tagsSSM.go deleted file mode 100644 index 54b85fc5b39d..000000000000 --- a/aws/tagsSSM.go +++ /dev/null @@ -1,137 +0,0 @@ -package aws - -import ( - "fmt" - "log" - "regexp" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ssm" - "github.com/hashicorp/terraform-plugin-sdk/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)) - 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) - r, _ := regexp.MatchString(v, *t.Key) - if r { - log.Printf("[DEBUG] Found AWS specific tag %s (val: %s), ignoring.\n", *t.Key, *t.Value) - return true - } - } - return false -} - -func saveTagsSSM(conn *ssm.SSM, d *schema.ResourceData, id, resourceType string) error { - resp, err := conn.ListTagsForResource(&ssm.ListTagsForResourceInput{ - ResourceId: aws.String(id), - ResourceType: aws.String(resourceType), - }) - - if err != nil { - return fmt.Errorf("Error retrieving tags for SSM resource (%s): %s", id, err) - } - - var dt []*ssm.Tag - if len(resp.TagList) > 0 { - dt = resp.TagList - } - - return d.Set("tags", tagsToMapSSM(dt)) -} diff --git a/aws/tagsSSM_test.go b/aws/tagsSSM_test.go deleted file mode 100644 index 06a0dbc47ea5..000000000000 --- a/aws/tagsSSM_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package aws - -import ( - "reflect" - "testing" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ssm" -) - -// 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) - } - } -}