cfn-lint icon indicating copy to clipboard operation
cfn-lint copied to clipboard

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

Open 0pt1c opened this issue 4 years ago • 10 comments

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: https://github.com/aws-cloudformation/cfn-python-lint/pull/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"
            ]
          }
        ]
      }
    },

0pt1c avatar Mar 17 '20 14:03 0pt1c

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

unacceptable avatar Mar 17 '20 18:03 unacceptable

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 avatar Mar 17 '20 18:03 kddejong

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

kddejong avatar Mar 17 '20 19:03 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.

kddejong avatar Mar 19 '20 15:03 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.

@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. :)

jason-idk avatar Mar 25 '20 14:03 jason-idk

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.

kddejong avatar Mar 25 '20 18:03 kddejong

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

Based on https://github.com/aws-cloudformation/cfn-python-lint/issues/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.

andrew-glenn avatar Apr 24 '20 13:04 andrew-glenn

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

Blanko2 avatar Aug 02 '21 22:08 Blanko2

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?

ashemedai avatar Aug 13 '21 11:08 ashemedai

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.

farski avatar Aug 13 '21 13:08 farski

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

kddejong avatar Oct 27 '22 21:10 kddejong