DLM/GetLifecyclePolicy fails to parse created/modified timestamps

[tomelliff]

I feel like I must be missing something obvious here and am limited by my relative lack of Go knowledge and the use of this SDK so let me know if I'm doing something stupid/missing something obvious.

Version of AWS SDK for Go?

v1.14.26 but also seems to be the case in latest (v1.15.10)

Version of Go (go version)?

go version go1.10 linux/amd64

What issue did you see?

When calling DLM's GetLifecyclePolicy I get the following error:

2018/08/13 15:08:17 [DEBUG] [aws-sdk-go] {"Policy":{"State":"ENABLED","DateModified":"2018-08-13T11:49:27+0000","Description":"foo","PolicyId":"policy-05f7adaed27074a6b","ExecutionRoleArn":"arn:aws:iam::1234567890:role/service-role/AWSDataLifecycleManagerDefaultRole","PolicyDetails":{"Schedules":[{"Name":"foo","CreateRule":{"Times":["16:13"],"IntervalUnit":"HOURS","Interval":12},"RetainRule":{"Count":24},"TagsToAdd":null}],"TargetTags":[{"Key":"Unexistent-Tag","Value":"This value doesn't exist either"}],"ResourceTypes":["VOLUME"]},"DateCreated":"2018-08-13T11:49:27+0000"}}
2018/08/13 15:08:17 [DEBUG] [aws-sdk-go] DEBUG: Unmarshal Response DLM/GetLifecyclePolicy failed, not retrying, error SerializationError: failed decoding JSON RPC response
caused by: parsing time "2018-08-13T11:49:27+0000" as "2006-01-02T15:04:05Z": cannot parse "+0000" as "Z"

Steps to reproduce

I've started working on a DLM lifecycle policy resource for Terraform (commit on my fork here) so you could build that and run it if necessary but I'm not sure about how to best build a minimal reproduction case outside of Terraform without taking a ton of time.

This looks like the API is returning timestamps that are wrong as can be seen in this minimal Playground as the AWS SDK is attempting to parse the +0000 timezone. I can't see a way to control the timestamp that the SDK uses to parse the time to decode the JSON response but could be missing something here.

I've tried viewing the policy via the AWS CLI (which uses the Python SDK) but this seems fine. The only other services I can find seem to use the epoch timestamp so I'm struggling to see if I can see any differences across other services.

[tomelliff]

Digging around a bit more I've found that Lambda functions have their LastModified values return as an ISO8601 formatted string (with +0000 instead of Z) but the SDK seems to handle this by setting it as a string and not a timestamp so it doesn't attempt to parse it. Should that be the case here too?

[tomelliff]

The following patch seems to solve things for me locally:

diff --git a/vendor/github.com/aws/aws-sdk-go/service/dlm/api.go b/vendor/github.com/aws/aws-sdk-go/service/dlm/api.go
index 0cb19f0..fc3ec75 100644
--- a/vendor/github.com/aws/aws-sdk-go/service/dlm/api.go
+++ b/vendor/github.com/aws/aws-sdk-go/service/dlm/api.go
@@ -4,7 +4,6 @@ package dlm
 
 import (
        "fmt"
-       "time"
 
        "github.com/aws/aws-sdk-go/aws"
        "github.com/aws/aws-sdk-go/aws/awsutil"
@@ -847,10 +846,10 @@ type LifecyclePolicy struct {
        _ struct{} `type:"structure"`
 
        // The local date and time when the lifecycle policy was created.
-       DateCreated *time.Time `type:"timestamp"`
+       DateCreated *string `type:"timestamp"`
 
        // The local date and time when the lifecycle policy was last modified.
-       DateModified *time.Time `type:"timestamp"`
+       DateModified *string `type:"timestamp"`
 
        // The description of the lifecycle policy.
        Description *string `type:"string"`
@@ -880,13 +879,13 @@ func (s LifecyclePolicy) GoString() string {
 }
 
 // SetDateCreated sets the DateCreated field's value.
-func (s *LifecyclePolicy) SetDateCreated(v time.Time) *LifecyclePolicy {
+func (s *LifecyclePolicy) SetDateCreated(v string) *LifecyclePolicy {
        s.DateCreated = &v
        return s
 }
 
 // SetDateModified sets the DateModified field's value.
-func (s *LifecyclePolicy) SetDateModified(v time.Time) *LifecyclePolicy {
+func (s *LifecyclePolicy) SetDateModified(v string) *LifecyclePolicy {
        s.DateModified = &v
        return s
 }

I'm not overly sure how to make this change in the SDK though as I gather most of it is generated from the REST API and I also don't know how you guys add tests for things like this?

[xibz]

Hello @tomelliff, thank you for reaching out to us. I've went ahead and contacted the service team as they are sending the incorrect timestamp format for their protocol. I will let you know when I hear back form them.

Also, the files you've edited are code generated, and while that is a way of bypassing the issue, I would wait until the service team fixes it on their end.

[tomelliff]

@xibz thanks, figured that was the case but wasn't sure if the solution was going to be fixing that the service outputs or fixing the SDK so it doesn't attempt to parse it as a time like with Lambda functions.

I assume if the service changes what they output then we won't need to change anything in the SDK to support this properly? Do you know if the teams involved here know when this might be fixed?

[xibz]

@tomelliff - The service team has been made aware that this is an issue on their end. They've also informed me they are working on a fix but have not given me a timeline. And yes, once they fix it on their end, the SDK doesn't need to be updated and should start working as intended. We will keep you updated once we have more information!

[JGalego]

Any news?

[xibz]

@JGalego - The service team has the fix completed and is currently testing it. Once we have news on when this is rolled out, I'll post back here.

[ryan-dyer-sp]

@xibz Any update on the fix?

[xibz]

@ryan-dyer-sp @JGalego @tomelliff - The service team has released a fix for this. Please let us know if you guys are still having issues. Going to go ahead and close this.

[tomelliff]

@xibz thanks for that. I've rebased my PR to Terraform so hopefully DLM support will land in Terraform soon.

On a side note (and as mentioned in the above linked PR) the validation for both description and policyDetails.schedule.name are inconsistent across the API docs and the CLI/SDK docs with the API referencing the length limit alone and the CLI/SDK docs referencing the allowed characters/regex alone. Quickly testing that seems to be that both are applied to both fields so I can add that to Terraform's plan time validation but it would be great if the docs for the API and CLI/SDK matched the behaviour of the API and showed what is actually allowed.

Did you want me to raise a separate issue for the above validation stuff?