aws-cloudformation / cfn-lint

CloudFormation Linter
MIT No Attribution
2.46k stars 597 forks source link

I3013 - Incorrectly flagged for aurora-<engine> database instances #2313

Closed jbarnes closed 2 years ago

jbarnes commented 2 years ago

cfn-lint version: 0.61.3

cfn-lint is falsely flagging the missing BackupRetentionPeriod property and it's subsequent value from AWS::RDS:DBInstance for an Aurora based database instance.

This property and a set value is not required if it is part of an Aurora cluster. Assumption is the Engine property value would need to be checked for the presence of aurora-<engine> to not flag this linting issue.

I suspect if the value is set via a CloudFormation function cfn-lint would still incorrectly flag this.

AWS Documentation reference - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-rds-database-instance.html#cfn-rds-dbinstance-backupretentionperiod

Amazon Aurora Not applicable. The retention period for automated backups is managed by the DB cluster.

CloudFormation example

AuroraInstance:
    Type: AWS::RDS::DBInstance
    DeletionPolicy: Retain
    UpdateReplacePolicy: Retain
    Properties:
      AllowMajorVersionUpgrade: false
      AutoMinorVersionUpgrade: !Ref AutoMinorVersionUpgrade
      DBClusterIdentifier: !Ref AuroraCluster
      DBInstanceClass: !Ref InstanceClass
      DBInstanceIdentifier: "MyAuroraInstance"
      DBParameterGroupName: !Ref ParamGroup
      DBSubnetGroupName: !Ref SubnetGroup
      DeleteAutomatedBackups: !Ref DeleteAutomatedBackups
      EnablePerformanceInsights: !Ref EnablePerformanceInsights
      Engine: aurora-postgresql
      EngineVersion: !Ref EngineVersion
      PerformanceInsightsKMSKeyId: !Ref KmsKey
      PerformanceInsightsRetentionPeriod: 7
      PubliclyAccessible: false
PatMyron commented 2 years ago

https://github.com/aws-cloudformation/cfn-lint/issues/2311


(nice to see more informational rule feedback coming in, assuming https://github.com/aws-cloudformation/cfn-lint-visual-studio-code/pull/223 is surfacing them more based on that being released this week)