AWS::Route53::RecordSet.TTL should be String not Long

[0pt1c]

cfn-lint version: (0.29.0)

Description of issue.

One of our pipelines started failing on cfn-lint today, and the error given is:
E3012 Property Resources/DbCNAME/Properties/TTL should be of type Long

I see this was due to a change in the new version released yesterday. The change is from this PR:
#1417

The AWS CloudFormation docs show this value should be a string:
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-route53-recordset.html

We're just ignoring this rule for now but I think it needs to be looked at again.

Here is the excerpt from the template:

"DbCNAME": {
      "Type": "AWS::Route53::RecordSet",
      "Properties": {
        "HostedZoneId": {
          "Fn::ImportValue": {
            "Fn::Sub": "${AWS::Region}-hosted-zone-id"
          }
        },
        "Name": {
          "Fn::Join": [
            ".",
            [
              "db",
              "inspections",
              {
                "Fn::ImportValue": {
                  "Fn::Sub": "${AWS::Region}-hosted-zone-name"
                }
              }
            ]
          ]
        },
        "Type": "CNAME",
        "TTL": "300",
        "ResourceRecords": [
          {
            "Fn::GetAtt": [
              "DbCluster",
              "Endpoint.Address"
            ]
          }
        ]
      }
    },

[unacceptable]

Here's the exact link:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-route53-recordset.html#cfn-route53-recordset-ttl

TTL
...
Type: String

[kddejong]

Looking into this one. https://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets.html#API_ChangeResourceRecordSets_RequestParameters has TTL as Long. Talking with a few people at AWS about how to handle this one.

[kddejong]

Additional details if you specify something non-numeric. Encountered non numeric value for property TTL

[kddejong]

I've been debating switch the strict check on E3012 to false by default. This will still test that "1" is 1 so this would allow a string and long for this value. This is how the CloudFormation service works as it will convert to the appropriate type as needed.

It allows us to test that TTL is long so we can provide quicker feedback but would allow the value type to be string or long. However, I would really like some feedback on this as strict checking has been the default since nearly the beginning.

[jason-idk]

I've been debating switch the strict check on E3012 to false by default. This will still test that "1" is 1 so this would allow a string and long for this value. This is how the CloudFormation service works as it will convert to the appropriate type as needed.

It allows us to test that TTL is long so we can provide quicker feedback but would allow the value type to be string or long. However, I would really like some feedback on this as strict checking has been the default since nearly the beginning.

@kddejong If you are interested in my feedback, I would vote yes for disabling E3012 strict checking by default. It seems like this change is causing more headaches and there is not sufficient documentation to go with the change.

Since cloudformation will convert as needed, I think that whether the value is a string or long doesnt really matter as it will be handled appropriately either way. I would like to hear your thoughts on this.

I am currently disabling strict checking on my templates and this is not something I want to continue to add to all my templates where the issue is happening and would prefer it happen at the source. :)

[kddejong]

I'm in agreement with @JLH993. The service seems to do that on your behalf so I'm not sure we have to be more strict than that.

[andrew-glenn]

I'm not sure E3012 to False is a good idea, honestly.

Based on #547, the original concern was that there are situations where the backend service doesn't always convert between actual-type and expected-type. I've seen evidence as recently as last week where this was still occurring.

[Blanko2]

to jump in on this:
the json spec for the TTL property is also set as String
https://dnwj8swjjbsbt.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json

[ashemedai]

String is also what the AWS::Route53::RecordSet documentation, that we refer our developers to, says.

This will needlessly cause support questions for platform teams. Unless you override this setting, of course.

So the main question is: are we trying to lint against the spec (as per typical static code analysis) or per intent, i.e. the letter or the spirit?

[farski]

So the main question is: are we trying to lint against the spec (as per typical static code analysis) or per intent, i.e. the letter or the spirit?

The description of this project says it lints against the spec, so I think it would be strange to do anything else.

[kddejong]

The default for rule E3012 is to disable strict checking. Going to close this one and we can re-open as needed.