Support Functions as DeletionPolicy/UpdateReplacePolicy Values

[jthomerson]

Currently the DeletionPolicy attribute for any resource can only be a string. This means we can't use functions like !If [ IsProduction, 'Retain', 'Delete' ].

I'd expect to be able to use conditions and other CloudFormation functions as a value for a DeletionPolicy the same as I can elsewhere in the template.

Others who have asked the same thing:

• https://forums.aws.amazon.com/message.jspa?messageID=560586
• Using a Condition in the DeletionPolicy of a resource is deemed invalid. aws/aws-cli#3825

[bdrx312]

It would be nice if the DeletionPolicy also accepted a parameter from the Parameters section although you could work around that if DeletionPolicy at least supported !If

[PatMyron]

A workaround until proper support is having multiple duplicate resources with different policies and using Conditions to create only resources with correct policies

[jthomerson]

Yeah, @PatMyron, that works, but I'm sure you'd agree that "copy and paste your resources and hope you always remember to keep them in sync" is a pretty bad pattern. 😜 Glad you added "until this is properly supported" ... that gives me hope!

[bdrx312]

That work around is really bad if you have an resources that have dependencies (DependsOn) on the resources.

[danieljamesscott]

I think that a better work around is to use AWS::Include and parameterise the name of the included snippet:

"Fn::Transform" : {
  "Name" : "AWS::Include",
  "Parameters" : {
    "Location" : {"Fn::Sub": "s3://${S3Bucket}/deletionPolicy/${RDSDeletionPolicy}.json" }
  }
}

It does mean you have to have snippets for all possible values (and a separate set for UpdateReplacePolicy), but better than duplicating resources.

[PatMyron]

@danieljamesscott due to AWS::Include regional availability, I've used Jinja to avoid maintaining duplicate source code

[bdrx312]

The AWS::Include that @danieljamesscott mentioned is a better work around than having duplicate resources but as @PatMyron mentioned is not available in all regions.

[danieljamesscott]

I've also just discovered that you can't use that work around for multiple policies (DeletionPolicy and UpdateReplacePolicy for example) because it results in duplicate keys in the JSON. The work around for that is to generate transforms with each combination of the 3 possibilities.

If AWS would just let us parameterise the policies it would save all this craziness... :)

[PatMyron]

@danieljamesscott duplicate resources should have different Logical IDs
As @bdrx312 mentioned, painful workaround if you have resources with DependsOn because of that

And agreed, temporary workarounds are not to say this isn't still a most important feature request :)

[danieljamesscott]

@PatMyron I'll admit that I haven't tested it for 'real' (my editor and cfn-lint complained) but is this really valid CF?

"S3Bucket": {
  "Type": "AWS::S3::Bucket",
  "Properties": {
    "BucketName": "dan-test-bucket4",
    "VersioningConfiguration": {
      "Status": "Suspended"
    }
  },
  "Fn::Transform" : {
    "Name" : "AWS::Include",
    "Parameters" : {
      "Location" : {"Fn::Sub": "s3://cloudformation-partials/deletePolicy/${DeletePolicy}.json" }
    }
  },
  "Fn::Transform" : {
    "Name" : "AWS::Include",
    "Parameters" : {
      "Location" : {"Fn::Sub": "s3://cloudformation-partials/updateReplacePolicy/${UpdateReplacePolicy}.json" }
    }
  }
}

[mrowles]

This has a high level of user experience, automation, cost and security implications. This was first raised in 2014 and we, as paying customers, still have no way to set DeletionPolicy dynamically.

[jobarjo]

Has there been any feedback from AWS on this topic and at least a tentative target date for completion?

[zetas]

@PatMyron I'll admit that I haven't tested it for 'real' (my editor and cfn-lint complained) but is this really valid CF?

"S3Bucket": {
  "Type": "AWS::S3::Bucket",
  "Properties": {
    "BucketName": "dan-test-bucket4",
    "VersioningConfiguration": {
      "Status": "Suspended"
    }
  },
  "Fn::Transform" : {
    "Name" : "AWS::Include",
    "Parameters" : {
      "Location" : {"Fn::Sub": "s3://cloudformation-partials/deletePolicy/${DeletePolicy}.json" }
    }
  },
  "Fn::Transform" : {
    "Name" : "AWS::Include",
    "Parameters" : {
      "Location" : {"Fn::Sub": "s3://cloudformation-partials/updateReplacePolicy/${UpdateReplacePolicy}.json" }
    }
  }
}

I'm not sure if using 2 transforms like this is necessary or works, I'm doing something similar and combining both policies in one include like so:

Resources:
  Instance:
    Type: 'AWS::RDS::DBInstance'
    Fn::Transform:
      Name: AWS::Include
      Parameters:
        Location: !Sub 's3://${S3SourceBucketName}/${CPVersion}/templates/customer/rds-instance-${StackType}-policy.yaml'
    Properties:
      Engine: MySQL
      EngineVersion: '5.7'
...

and then in the rds-instance-dev-policy.yaml:

DeletionPolicy: Delete
UpdateReplacePolicy: Delete

and all seems to work fine. I am however getting an error with cfn-lint: E3001 Invalid resource attribute Fn::Transform for resource Instance rds.yaml:64:5 which is super annoying because I use taskcat for CF testing and it relies on the cfn linter under the hood so I can't complete my CI pipeline now because it fails the linter. I've posted about it on their issue tracker here but I think this is an aws-cfn-bootstrap issue which is why I'm posting here. I hope someone with knowledge with that package can read this and help.

Also as an aside in that issue I linked there is the processed json template directly from the CF ui so you can see definitively that those directives are being included properly.

[PatMyron]

@zetas AWS::Include transform isn't fully supported by the Linter yet, but workaround with resource-level ignores:

  Instance:
    Type: AWS::RDS::DBInstance
    Metadata:
      cfn-lint:
        config:
          ignore_checks:
          - E3001

[zetas]

@PatMyron omg you are amazing, thank you so much! That worked great.

[nefkor]

It would be important to fix this issue soon, since many customer projects required parametrize the deletion policies, and making "copies" with conditional is not an option when cfn template increases in size beyond the limit

[mariaines]

Kinda hacky workaround: specify 'Retain' for your DeletionPolicy, then use a custom resource to delete the resource(s) on stack deletion when desired. This avoids duplication and works without any transforms.

E.g.:

  Table:
    Type: AWS::DynamoDB::Table
    DeletionPolicy: Retain
    ...
  CleanupTableOnStackDelete:
    Type: AWS::Serverless::Function
    Condition: ShouldDeleteTableOnStackDelete
    Properties:
      InlineCode:
        !Sub |
          import json, boto3, logging, uuid
          import cfnresponse
          logger = logging.getLogger()
          logger.setLevel(logging.INFO)

          def lambda_handler(event, context):
              logger.info("event: {}".format(event))
              try:
                  tableName = event['ResourceProperties']['TableName']
                  logger.info("tableName: {}, event['RequestType']: {}".format(tableName,event['RequestType']))
                  if event['RequestType'] == 'Delete':
                      dynamodb = boto3.client('dynamodb')
                      dynamodb.delete_table(TableName=tableName)

                  sendResponseCfn(event, context, cfnresponse.SUCCESS)
              except Exception as e:
                  logger.info("Exception: {}".format(e))
                  sendResponseCfn(event, context, cfnresponse.FAILED)

          def sendResponseCfn(event, context, responseStatus):
              responseData = {}
              responseData['Data'] = {}
              physicalResourceId = event.get('PhysicalResourceId', str(uuid.uuid4()))
              cfnresponse.send(event, context, responseStatus, responseData, physicalResourceId)

      Handler: "index.lambda_handler"
      Runtime: python3.8
      MemorySize: 128
      Timeout: 60
      Role: "arn:aws:iam::...:role/DeleteTestTableRole"
  DoCleanupTableOnStackDelete:
    Type: Custom::CleanupTableOnStackDelete
    Condition: ShouldDeleteTableOnStackDelete
    Properties:
      ServiceToken: !GetAtt CleanupTableOnStackDelete.Arn
      TableName: !Ref Table
    DependsOn:
      - Table
      - CleanupTableOnStackDelete

[sairoko12]

@mariaines Your approach is very helpful 🥇 , but I feel this is very complex for future maintenance because the behavior depends on the other resource that is the lambda, and maybe this produces a little confuses between other developers 😬 but right now is a good option

[bdrx312]

I have a hard time imagining how the back end code for cloudformation could have been written that makes this such a difficult feature (bug fix in my opinion) to implement. This is one of the oldest feature requests on the cloudformation coverage roadmap with one of the top number of thumbs up, yet there has been no response from AWS to even acknowledge that they are looking at it and give an idea of why it can't be implemented in any kind of reasonable time period.

[andrewlytle]

Please get this fixed, this causes a huge amount of wasted effort and time.

[bogdantimes]

Agreed, this has to be addressed ASAP by the AWS CloudFormation team... c'mon

[wesleymooiman]

Still waiting on this....

[uncledru-veho]

Would love some sort of response from AWS/CloudFormation team!

[lejiati]

@jthomerson Thank you very much for your feedback! Since this repository is focused on resource coverage, I'm transferring this issue over to a new GitHub repository dedicated to CloudFormation template language issues.

FYI, We have an RFC #74 proposed that allows intrinsic function inDeletionPolicy/UpdateReplacePolicy.

[lejiati]

Hi all, really appreciate all the feedback and I'm happy to share that intrinsic functions such as Fn::If is supported now with the launch of AWS::LanguageExtensions transform. Please feel free to try it out and leave you feedback here.

[MalikAtalla-AWS]

This functionality is now available, so we can close this issue.

[lukepafford]

Hopefully this issue is just isolated to my environment. Let me know if anyone else experiences this problem with the new AWS::LanguageExtensions Transform.

Potential Bug : When combining the transform with the AWS::Serverless-2016-10-31 transform, only the first transform in the list is applied.

---

AWS::Serverless-2016-10-31 listed first:

AWSTemplateFormatVersion: '2010-09-09'
Transform:
- AWS::Serverless-2016-10-31
- AWS::LanguageExtensions

Produces the error:

Creating ChangeSet-2022-08-31T20-19-32Z in --- failed
"StatusReason": "Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 1. Structure of the SAM template is invalid. Every DeletionPolicy member must be a string."

This error occurs when creating the ChangeSet, indicating that the AWS::LanguageExtensions transform is being ignored.

---

AWS::LanguageExtensions listed first:

AWSTemplateFormatVersion: '2010-09-09'
Transform:
- AWS::LanguageExtensions
- AWS::Serverless-2016-10-31

Produces the error:

Policy statement must contain actions. (Service: AmazonIdentityManagement; Status Code: 400; Error Code: MalformedPolicyDocument; Request ID: 9dcda1f8-0cbc-472d-891c-5e72378b500e; Proxy: null)

This error is referring to a role that previously deployed successfully, which appears to indicate that the AWS::Serverless-2016-10-31 transform is being ignored.

---

If others can confirm that the AWS::LanguageExtensions transform plays nice with AWS::Serverless-2016-10-31 then that will be helpful

---

Minimal Template Which Reproduces Bug

Sorry for being lazy, here's a functional template which displays the issue:

---
# Deployed with
# aws cloudformation deploy --template-file ~/cloudformation-language-extension-with-serverless-test.yaml --stack "LanguageExtensionsTest" --capabilities CAPABILITY_IAM

# The following template deploys successfully AS IS.
# However if you uncomment the `AWS::LanguageExtensions` transform, and the corresponding `TestLogGroup` resource, then the 
# LambdaRole resource fails to deploy with the error:
# > Policy statement must contain actions. (Service: AmazonIdentityManagement; Status Code: 400; Error Code: MalformedPolicyDocument; Request ID: 08df462c-2472-4b76-91a0-87d293c285ee; Proxy: null)

AWSTemplateFormatVersion: 2010-09-09
Transform:
# - 'AWS::LanguageExtensions'
- 'AWS::Serverless-2016-10-31'
Parameters:
  Stage:
    Type: String
    AllowedValues:
      - Prod
      - Staging
      - Dev
    Default: Dev
  RunLambdaInVPC:
    Type: String
    Default: false
    AllowedValues: [true, false]
Conditions:
  IsProd: !Equals 
    - !Ref Stage
    - Prod
  RunLambdaInVPC: !Equals
    - !Ref RunLambdaInVPC
    - true
Resources:
  # TestLogGroup:
  #   Type: "AWS::Logs::LogGroup"
  #   DeletionPolicy: !If [IsProd, Retain, Delete]
  #   Properties:
  #     LogGroupName: "TestLogGroup"
  LambdaRole:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
        - Action: ['sts:AssumeRole']
          Effect: Allow
          Principal:
            Service: [lambda.amazonaws.com]
        Version: '2012-10-17'
      Policies:
      - PolicyDocument:
          Statement:
          - Action: ['cloudwatch:*', 'logs:*']
            Effect: Allow
            Resource: '*'
          - Fn::If:
            - RunLambdaInVPC
            - Action: ['ec2:CreateNetworkInterface', 'ec2:DescribeNetworkInterfaces', 'ec2:DeleteNetworkInterface']
              Effect: Allow
              Resource: '*'
            - {Ref: 'AWS::NoValue'}
          Version: '2012-10-17'
        PolicyName: lambdaRole

[lejiati]

Hey @lukepafford , thank you for submitting this issue! The issue should be fixed now and please give it another try and let us know if you still have any questions.

[jlhood]

@lukepafford Note, you must put the AWS::LanguageExtensions transform before the AWS::Serverless transform in the list. We are working on updating our documentation to indicate this ordering.

[benkehoe]

If this is known for AWS transforms, does it make sense for CloudFormation to even accept a template with the transforms out of order?

[jlhood]

@benkehoe Thanks for the feedback. We've had internal discussions on how to help guide customers here. First step is documentation. We're also planning to add a rule to cfn-lint to check for this. We'll discuss the possibility of a server-side check.

[benkehoe]

ooh, adding it to cfn-lint is a great idea!

[lukepafford]

@jlhood @lejiati
I confirmed that my fully uncommented minimal template successfully deploys from both the UPDATE_ROLLBACK_COMPLETE state, and from a fresh deploy.

Thanks for the quick turnaround!