Skip to content

Commit

Permalink
provider: Fix d.Set() calls with invalid *time.Time value input (#11491)
Browse files Browse the repository at this point in the history
Reference: #9954

Previously, these attributes were incorrectly trying to use `*time.Time` as the value input to `d.Set()`. These will return errors if error checking is performed, however our preferred convention of not performing error checking for "simple" attributes means that these silently would skip being added to the Terraform state with the refreshed API value.

The `tfproviderlint` `R004` check caught these previously though (which we would like to enable in the future but cannot until these and other failures are addressed):

```
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/data_source_aws_acmpca_certificate_authority.go:116:21: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/data_source_aws_acmpca_certificate_authority.go:117:22: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_acmpca_certificate_authority.go:362:21: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_acmpca_certificate_authority.go:363:22: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_emr_security_configuration.go:96:25: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_iam_service_linked_role.go:140:23: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_lambda_event_source_mapping.go:224:25: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_organizations_account.go:228:28: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_ssm_activation.go:159:27: R004: ResourceData.Set() incompatible value type: *time.Time
/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_ssm_document.go:243:24: R004: ResourceData.Set() incompatible value type: *time.Time
```

Here we update each of these `d.Set()` calls with the appropriate RFC3339 timestamp value (our preferred format in lieu of other API standards).

The additional changes for `aws_ssm_activation` are to handle the API automatically setting a default value of 24 hours in advance. We do not want Terraform to return a difference in cases where the configuration does not have a value.

Output from acceptance testing:

```
--- PASS: TestAccDataSourceAwsAcmpcaCertificateAuthority_Basic (29.17s)

--- PASS: TestAccAwsAcmpcaCertificateAuthority_Basic (23.49s)

--- PASS: TestAccAWSEmrSecurityConfiguration_basic (18.22s)

--- PASS: TestAccAWSIAMServiceLinkedRole_basic (23.39s)

--- PASS: TestAccAWSLambdaEventSourceMapping_kinesis_basic (85.92s)
--- PASS: TestAccAWSLambdaEventSourceMapping_sqs_basic (134.73s)

--- PASS: TestAccAWSSSMActivation_expirationDate (24.95s)
--- PASS: TestAccAWSSSMActivation_basic (29.94s)

--- PASS: TestAccAWSSSMDocument_basic (21.52s)
```
  • Loading branch information
bflad authored Jan 17, 2020
1 parent e6437e3 commit bd59ac2
Show file tree
Hide file tree
Showing 17 changed files with 32 additions and 15 deletions.
5 changes: 3 additions & 2 deletions aws/data_source_aws_acmpca_certificate_authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import (
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/acmpca"
Expand Down Expand Up @@ -113,8 +114,8 @@ func dataSourceAwsAcmpcaCertificateAuthorityRead(d *schema.ResourceData, meta in
certificateAuthority := describeCertificateAuthorityOutput.CertificateAuthority

d.Set("arn", certificateAuthority.Arn)
d.Set("not_after", certificateAuthority.NotAfter)
d.Set("not_before", certificateAuthority.NotBefore)
d.Set("not_after", aws.TimeValue(certificateAuthority.NotAfter).Format(time.RFC3339))
d.Set("not_before", aws.TimeValue(certificateAuthority.NotBefore).Format(time.RFC3339))

if err := d.Set("revocation_configuration", flattenAcmpcaRevocationConfiguration(certificateAuthority.RevocationConfiguration)); err != nil {
return fmt.Errorf("error setting tags: %s", err)
Expand Down
8 changes: 8 additions & 0 deletions aws/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/terraform"
)

const rfc3339RegexPattern = `^[0-9]{4}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])[Tt]([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9](\.[0-9]+)?([Zz]|([+-]([01][0-9]|2[0-3]):[0-5][0-9]))$`

var testAccProviders map[string]terraform.ResourceProvider
var testAccProviderFactories func(providers *[]*schema.Provider) map[string]terraform.ResourceProviderFactory
var testAccProvider *schema.Provider
Expand Down Expand Up @@ -193,6 +195,12 @@ func testAccMatchResourceAttrGlobalARN(resourceName, attributeName, arnService s
}
}

// testAccCheckResourceAttrRfc3339 ensures the Terraform state matches a RFC3339 value
// This TestCheckFunc will likely be moved to the Terraform Plugin SDK in the future.
func testAccCheckResourceAttrRfc3339(resourceName, attributeName string) resource.TestCheckFunc {
return resource.TestMatchResourceAttr(resourceName, attributeName, regexp.MustCompile(rfc3339RegexPattern))
}

// testAccCheckListHasSomeElementAttrPair is a TestCheckFunc which validates that the collection on the left has an element with an attribute value
// matching the value on the left
// Based on TestCheckResourceAttrPair from the Terraform SDK testing framework
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_acmpca_certificate_authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,8 @@ func resourceAwsAcmpcaCertificateAuthorityRead(d *schema.ResourceData, meta inte
}

d.Set("enabled", (aws.StringValue(certificateAuthority.Status) != acmpca.CertificateAuthorityStatusDisabled))
d.Set("not_after", certificateAuthority.NotAfter)
d.Set("not_before", certificateAuthority.NotBefore)
d.Set("not_after", aws.TimeValue(certificateAuthority.NotAfter).Format(time.RFC3339))
d.Set("not_before", aws.TimeValue(certificateAuthority.NotBefore).Format(time.RFC3339))

if err := d.Set("revocation_configuration", flattenAcmpcaRevocationConfiguration(certificateAuthority.RevocationConfiguration)); err != nil {
return fmt.Errorf("error setting tags: %s", err)
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_acmpca_certificate_authority_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ func TestAccAwsAcmpcaCertificateAuthority_Basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "certificate_chain", ""),
resource.TestCheckResourceAttrSet(resourceName, "certificate_signing_request"),
resource.TestCheckResourceAttr(resourceName, "enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "not_after", ""),
resource.TestCheckResourceAttr(resourceName, "not_before", ""),
testAccCheckResourceAttrRfc3339(resourceName, "not_after"),
testAccCheckResourceAttrRfc3339(resourceName, "not_before"),
resource.TestCheckResourceAttr(resourceName, "permanent_deletion_time_in_days", "30"),
resource.TestCheckResourceAttr(resourceName, "revocation_configuration.#", "1"),
resource.TestCheckResourceAttr(resourceName, "revocation_configuration.0.crl_configuration.#", "1"),
Expand Down
3 changes: 2 additions & 1 deletion aws/resource_aws_emr_security_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/emr"
Expand Down Expand Up @@ -93,7 +94,7 @@ func resourceAwsEmrSecurityConfigurationRead(d *schema.ResourceData, meta interf
return err
}

d.Set("creation_date", resp.CreationDateTime)
d.Set("creation_date", aws.TimeValue(resp.CreationDateTime).Format(time.RFC3339))
d.Set("name", resp.Name)
d.Set("configuration", resp.SecurityConfiguration)

Expand Down
1 change: 1 addition & 0 deletions aws/resource_aws_emr_security_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestAccAWSEmrSecurityConfiguration_basic(t *testing.T) {
Config: testAccEmrSecurityConfigurationConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckEmrSecurityConfigurationExists(resourceName),
testAccCheckResourceAttrRfc3339(resourceName, "creation_date"),
),
},
{
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_iam_service_linked_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func resourceAwsIamServiceLinkedRoleRead(d *schema.ResourceData, meta interface{

d.Set("arn", role.Arn)
d.Set("aws_service_name", serviceName)
d.Set("create_date", role.CreateDate)
d.Set("create_date", aws.TimeValue(role.CreateDate).Format(time.RFC3339))
d.Set("custom_suffix", customSuffix)
d.Set("description", role.Description)
d.Set("name", role.RoleName)
Expand Down
1 change: 1 addition & 0 deletions aws/resource_aws_iam_service_linked_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func TestAccAWSIAMServiceLinkedRole_basic(t *testing.T) {
testAccCheckAWSIAMServiceLinkedRoleExists(resourceName),
testAccCheckResourceAttrGlobalARN(resourceName, "arn", "iam", fmt.Sprintf("role%s%s", path, name)),
resource.TestCheckResourceAttr(resourceName, "aws_service_name", awsServiceName),
testAccCheckResourceAttrRfc3339(resourceName, "create_date"),
resource.TestCheckResourceAttr(resourceName, "description", ""),
resource.TestCheckResourceAttr(resourceName, "name", name),
resource.TestCheckResourceAttr(resourceName, "path", path),
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_lambda_event_source_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func resourceAwsLambdaEventSourceMappingRead(d *schema.ResourceData, meta interf
d.Set("maximum_batching_window_in_seconds", eventSourceMappingConfiguration.MaximumBatchingWindowInSeconds)
d.Set("event_source_arn", eventSourceMappingConfiguration.EventSourceArn)
d.Set("function_arn", eventSourceMappingConfiguration.FunctionArn)
d.Set("last_modified", eventSourceMappingConfiguration.LastModified)
d.Set("last_modified", aws.TimeValue(eventSourceMappingConfiguration.LastModified).Format(time.RFC3339))
d.Set("last_processing_result", eventSourceMappingConfiguration.LastProcessingResult)
d.Set("state", eventSourceMappingConfiguration.State)
d.Set("state_transition_reason", eventSourceMappingConfiguration.StateTransitionReason)
Expand Down
2 changes: 2 additions & 0 deletions aws/resource_aws_lambda_event_source_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestAccAWSLambdaEventSourceMapping_kinesis_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsLambdaEventSourceMappingExists(resourceName, &conf),
testAccCheckAWSLambdaEventSourceMappingAttributes(&conf),
testAccCheckResourceAttrRfc3339(resourceName, "last_modified"),
),
},
{
Expand Down Expand Up @@ -133,6 +134,7 @@ func TestAccAWSLambdaEventSourceMapping_sqs_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsLambdaEventSourceMappingExists(resourceName, &conf),
testAccCheckAWSLambdaEventSourceMappingAttributes(&conf),
testAccCheckResourceAttrRfc3339(resourceName, "last_modified"),
resource.TestCheckNoResourceAttr(resourceName,
"starting_position"),
),
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_organizations_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func resourceAwsOrganizationsAccountRead(d *schema.ResourceData, meta interface{
d.Set("arn", account.Arn)
d.Set("email", account.Email)
d.Set("joined_method", account.JoinedMethod)
d.Set("joined_timestamp", account.JoinedTimestamp)
d.Set("joined_timestamp", aws.TimeValue(account.JoinedTimestamp).Format(time.RFC3339))
d.Set("name", account.Name)
d.Set("parent_id", parentId)
d.Set("status", account.Status)
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_organizations_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func testAccAwsOrganizationsAccount_basic(t *testing.T) {
testAccCheckAwsOrganizationsAccountExists("aws_organizations_account.test", &account),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "arn"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "joined_method"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "joined_timestamp"),
testAccCheckResourceAttrRfc3339("aws_organizations_account.test", "joined_timestamp"),
resource.TestCheckResourceAttrSet("aws_organizations_account.test", "parent_id"),
resource.TestCheckResourceAttr("aws_organizations_account.test", "name", name),
resource.TestCheckResourceAttr("aws_organizations_account.test", "email", email),
Expand Down
3 changes: 2 additions & 1 deletion aws/resource_aws_ssm_activation.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func resourceAwsSsmActivation() *schema.Resource {
"expiration_date": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.ValidateRFC3339TimeString,
},
Expand Down Expand Up @@ -156,7 +157,7 @@ func resourceAwsSsmActivationRead(d *schema.ResourceData, meta interface{}) erro
activation := resp.ActivationList[0] // Only 1 result as MaxResults is 1 above
d.Set("name", activation.DefaultInstanceName)
d.Set("description", activation.Description)
d.Set("expiration_date", activation.ExpirationDate)
d.Set("expiration_date", aws.TimeValue(activation.ExpirationDate).Format(time.RFC3339))
d.Set("expired", activation.Expired)
d.Set("iam_role", activation.IamRole)
d.Set("registration_limit", activation.RegistrationLimit)
Expand Down
3 changes: 2 additions & 1 deletion aws/resource_aws_ssm_activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestAccAWSSSMActivation_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSSMActivationExists("aws_ssm_activation.foo", &ssmActivation),
resource.TestCheckResourceAttrSet("aws_ssm_activation.foo", "activation_code"),
testAccCheckResourceAttrRfc3339("aws_ssm_activation.foo", "expiration_date"),
resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.%", "1"),
resource.TestCheckResourceAttr("aws_ssm_activation.foo", "tags.Name", tag)),
},
Expand Down Expand Up @@ -69,7 +70,7 @@ func TestAccAWSSSMActivation_update(t *testing.T) {
func TestAccAWSSSMActivation_expirationDate(t *testing.T) {
var ssmActivation ssm.Activation
rName := acctest.RandString(10)
expirationTime := time.Now().Add(48 * time.Hour)
expirationTime := time.Now().Add(48 * time.Hour).UTC()
expirationDateS := expirationTime.Format(time.RFC3339)
resourceName := "aws_ssm_activation.foo"

Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_ssm_document.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func resourceAwsSsmDocumentRead(d *schema.ResourceData, meta interface{}) error
doc := describeDocumentOutput.Document

d.Set("content", getDocumentOutput.Content)
d.Set("created_date", doc.CreatedDate)
d.Set("created_date", aws.TimeValue(doc.CreatedDate).Format(time.RFC3339))
d.Set("default_version", doc.DefaultVersion)
d.Set("description", doc.Description)
d.Set("schema_version", doc.SchemaVersion)
Expand Down
1 change: 1 addition & 0 deletions aws/resource_aws_ssm_document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestAccAWSSSMDocument_basic(t *testing.T) {
testAccCheckAWSSSMDocumentExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "document_format", "JSON"),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ssm", fmt.Sprintf("document/%s", name)),
testAccCheckResourceAttrRfc3339(resourceName, "created_date"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
),
},
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/ssm_activation.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ The following arguments are supported:

* `name` - (Optional) The default name of the registered managed instance.
* `description` - (Optional) The description of the resource that you want to register.
* `expiration_date` - (Optional) A timestamp in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8) by which this activation request should expire. The default value is 24 hours from resource creation time.
* `expiration_date` - (Optional) UTC timestamp in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8) by which this activation request should expire. The default value is 24 hours from resource creation time. Terraform will only perform drift detection of its value when present in a configuration.
* `iam_role` - (Required) The IAM Role to attach to the managed instance.
* `registration_limit` - (Optional) The maximum number of managed instances you want to register. The default value is 1 instance.
* `tags` - (Optional) A mapping of tags to assign to the object.
Expand Down

0 comments on commit bd59ac2

Please sign in to comment.