aws-cdk aws-s3: SkipDestinationValidation should not have default value when not used. It triggers CR's accidentally

Describe the bug

https://github.com/aws/aws-cdk/issues/30914 introduced default value that is set always to false. The wanted behaviour would be that it would be omit when not defined.

Similar issue like this which was rollback: https://github.com/aws/aws-cdk/issues/30121

Regression Issue

[ ] Select this option if this issue appears to be a regression.

Last Known Working CDK Version

2.154.1

Expected Behavior

No default value. Default value triggers all custom resources.

Current Behavior

Added new default value -> triggers CRs

Reproduction Steps

Update CDK, CRs are triggered which causes problems in many CR definitions in generally (not designed for re-run).

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.154.1

Framework Version

No response

Node.js Version

latest

OS

any

Language

TypeScript

Language Version

No response

Other information

No response

Aug 27 '24 12:08 heikkis

#30914 https://github.com/aws/aws-cdk/pull/30916/files#diff-86b9ce1dfff6c730deb18027ed257d3bdaefce441ece0354d481081972c98af2R81 SkipDestinationValidation is not set but still added as false.

Aug 27 '24 12:08 heikkis

Findings:

Looks like PR https://github.com/aws/aws-cdk/pull/30916 per per file diff:
- Introduced a mandatory property skipDestinationValidation which sets the SkipDestinationValidation in createResourceOnce() while creating Custom::S3BucketNotifications custom resource.
- It also modified the notifications-resource-handler to by default use the value of skipDestinationValidation as false, if not defined.
If we don't want to set the property SkipDestinationValidation if not defined, then we would need to make the property skipDestinationValidation optional.
Per Amazon S3 > PutBucketNotificationConfiguration API documentation, the value for x-amz-skip-destination-validation is either true or false.

@heikkis Please advise an end-to-end scenario with minimal reproducible CDK code which demonstrates that the custom resource would be triggered when SkipDestinationValidation is not set. SkipDestinationValidation is only used when adding notifications. If not defined, it would default to false per Amazon S3 > PutBucketNotificationConfiguration API.

Thanks, Ashish

Aug 27 '24 18:08 ashishdhingra

I think @heikkis 's concern is that - when it's not set, it should leave as undefined(implicit false) rather than an explicit false.

https://github.com/yerzhan7/aws-cdk/blob/26b21e1f8bc934449031796ce336a5ae1a69971a/packages/aws-cdk-lib/aws-s3/lib/bucket.ts#L895C9-L895C34

This passes an explicit false when undefined, which is arguably wrong.

And because of that, here would be passed an explicit false which cause unnecessary custom resource update.

Am I understanding this correct?

Aug 27 '24 18:08 pahud

Thank you for bringing this to our attention. I can implement a similar fix to #30418 where we set the default of SkipDestinationValidation as undefined to prevent triggering an update of the custom resource. Can you confirm if this change would cause another update? If so we can either warn the user or introduce a feature flag for customers who wish to keep the existing behavior where SkipDestinationValidation is set to false by default (and avoid another update of their CR setting the value from false to undefined with the fix).

Aug 27 '24 20:08 gracelu0

No parameter changes (add or delete) should be added by default to CR's. I think if it is undefined it want be added at all => wanted way 👍🏼

Aug 29 '24 05:08 heikkis

I wrote a work-around for this today by finding all stack resources of type Custom::S3BucketNotifications and using the .AddPropertyDeletionOverride() escape hatch to remove the offending line of CloudFormation.

public static Stack OverrideBucketNotificationProperties(this Stack instance)
{
    var bucketNotifications = instance
            .Node
            .FindAll()
            .Select(x => x.Node.DefaultChild)
            .OfType<CfnResource>()
            .Where(x => x.CfnResourceType == "Custom::S3BucketNotifications");

    foreach (var resource in bucketNotifications)
        resource.AddPropertyDeletionOverride("SkipDestinationValidation");

    return instance;
}

Sep 25 '24 08:09 ryanwilliams83

@ryanwilliams83 not to ask too stupid of a question, but how do you apply your workaround? Thank you!

Oct 01 '24 01:10 mdw123

@ryanwilliams83 not to ask too stupid of a question, but how do you apply your workaround? Thank you!

My workaround is code in the CDK stack that you add last in the constructor. It enumerates all nodes in the stack safe casting them from IResource to CfnResource. Then filter the list to find the node that is the CustomResource itself. Finally use the Amazon provided "escape hatch" function CfnResource.addPropertyDeletionOverride() to cause CDK to Delete a CloudFormation property from the synthesized output.

The problem arises because the CloudFormation synthesised with new versions of CDK contains the SkipValidation property whereas older versions did not. The fact that the CloudFormation has changed between the time you first provisioned your bucket trigger now causes the CR (Custom Resource) to be executed by CloudFormation which can result in the error about ambiguous triggers.

As you may be aware CloudFormation will attempt to provision new resources before cleaning up old ones while applying a "Change Set".

If you're still stuck paste my code into ChatGPT, tell it you found a workaround for the ambiguous bucket trigger problem introduced when the SkipValidation property was recently added, and ask it to write the equivalent work-around in your language of choice.

Oct 01 '24 02:10 ryanwilliams83

@heikkis I'm working on a fix

Nov 27 '24 22:11 megarach0

@heikkis @mdw123 @ryanwilliams83 Feel free to review my PR so we can move this fix along

Dec 30 '24 20:12 megarach0

I got stuck because of SkipDestinationValidation behavior. I got error like below.

Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configuration is ambiguously defined. Cannot have overlapping suffixes in two rules if the prefixes are overlapping for the same event type.. See the details in CloudWatch Log Stream: 2024/10/09/[$LATEST]ddb23884496c4b069ba07632c4d9da66 (RequestId: cc9644a1-8614-4593-a4e5-e84a90584352)

This is because stack with definition of s3 event try to set SkipDestinationValidation that is different from s3 event that alrady-existing, then prefixes and suffixes are duplicate.

I tried the way @ryanwilliams83 mentioned, but it didn't work in my case.

So I thought another workaround that can escape this trap with Aspects.

import { IAspect, CfnResource } from 'aws-cdk-lib';
import { IConstruct } from 'constructs';

export class RemoveSkipDestinationValidationAspect implements IAspect {
  visit(node: IConstruct): void {
    if (
      node instanceof CfnResource &&
      node.cfnResourceType === 'Custom::S3BucketNotifications'
    ) {
      node.addPropertyDeletionOverride('SkipDestinationValidation');
    }
  }
}

And in cdk.ts apply for some target stacks.

cdk.Aspects.of(someStack).add(new RemoveSkipDestinationValidationAspect());

Hope this helps someone else facing a similar issue!!!

May 15 '25 01:05 kiitosu

Still interested about getting this fixed. https://github.com/aws/aws-cdk/pull/32361 was closed due inactivity.

May 26 '25 10:05 heikkis

Still interested about getting this fixed. #32361 was closed due inactivity.

yea couldn't get good support for the code review. If you want, you can basically pull down the changes I made and open a new PR. I don't have time to work on this currently.

Aug 07 '25 15:08 megarach0