Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@aws-cdk/aws-s3: Unable to delete the existing S3 event notifications #28915

Closed
sarangarav opened this issue Jan 29, 2024 · 25 comments · Fixed by #30610, #30706, codu-code/codu#969 or rwlxxvii/containers#185 · May be fixed by NOUIY/aws-solutions-constructs#112
Assignees
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@sarangarav
Copy link
Contributor

sarangarav commented Jan 29, 2024

Describe the bug

I'm facing an issue while trying to delete the existing S3 event notifications.

The problem arises with S3 bucket event notification handler Lambda(AWS CloudFormation handler for "Custom::S3BucketNotifications" resources (@aws-cdk/aws-s3)), where it fails and reports an error: 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.

Custom event notification handler reference : https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py

The issue is likely related to the change: 37be7b9

The offending line is Line 39 - we are updating the ID of a JSON object, but the ID is inside of that object when it comes from outside. (https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py#L39)

Expected Behavior

Should delete the existing event notifications

Current Behavior

Unable to delete the existing filters:
Error: 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.

Reproduction Steps

  1. Add few event notifications
  2. Remove some of the the added notifications.
    Please find the example stack attached to reproduce the issue.
    s3-event-notifications.txt

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.124.0

Framework Version

No response

Node.js Version

v20.11.0

OS

Linux

Language

TypeScript

Language Version

No response

Other information

No response

@sarangarav sarangarav added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 29, 2024
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Jan 29, 2024
@pahud
Copy link
Contributor

pahud commented Jan 30, 2024

Custom event notification handler reference
I am getting a 404 from this link.

Are you able to provide a minimal reproducible code snippets that we can simply copy/paste to reproduce this issue in our environment? That would be very helpful if we share the same code snippets. Thank you.

@pahud pahud added p2 effort/medium Medium work item – several days of effort response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 30, 2024
Copy link

github-actions bot commented Feb 2, 2024

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Feb 2, 2024
@gkaskonas
Copy link
Contributor

Hi, we are getting this error everytime we update the lambda target.

  import { Function as Handler } from "aws-cdk-lib/aws-lambda";
  ...
  const handler = Handler.fromFunctionArn(stack, "Handler", stackLocalArn);
  const bucket = Bucket.fromBucketName(stack, "Bucket", bucketName);
  bucket.addEventNotification(EventType.OBJECT_CREATED_PUT, new LambdaDestination(handler));
  return stack;

Every time stackLocalArn is updated the stack update fails with this error

Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configurations overlap. Configurations on the same bucket cannot share a common event type.. See the details in CloudWatch Log Stream: 2024/01/29/[$LATEST]099d50dec4ab44dcb5be29f1d1b6e13c (RequestId: 9582e198-4537-479b-b112-b17761fb58a2)

Why does it try to create a new notification config instead of updating the existing one? We tried deleting the existing notification config and allowing cf to create a new one but as soon as we update the lambda it fails again.

I think this is related to this change 37be7b9

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Feb 5, 2024
@sarangarav
Copy link
Contributor Author

sarangarav commented Feb 5, 2024

Here is the correct code path :

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py

We found out one more issue when we add a S3 event notification to the existing ones causes creation failure.

The issue is likely related to the change: 37be7b9

The offending line is Line 39 - we are updating the ID of a JSON object, but the ID is inside of that object when it comes from outside. (https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/custom-resource-handlers/lib/aws-s3/notifications-resource-handler/index.py#L39)

We can easily reproduce this issue when we try to add a new S3 event notifications on top of existing ones.
Attached the sample test data to reproduce the issue:
test_samples.json

Custom event notification handler reference
I am getting a 404 from this link.

Are you able to provide a minimal reproducible code snippets that we can simply copy/paste to reproduce this issue in our environment? That would be very helpful if we share the same code snippets. Thank you.

@pahud
Copy link
Contributor

pahud commented Feb 7, 2024

I was not able to deploy this with cdk 2.126.0

export class ExampleStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const handler = this.getDummyHandler();
    const importedHandler = lambda.Function.fromFunctionArn(this, "Handler", handler.functionArn);
    const bucket = s3.Bucket.fromBucketName(this, "Bucket", my_existing_bucket_name);
    bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.LambdaDestination(importedHandler));
    new CfnOutput(this, 'importedHandlerArn', { value: importedHandler.functionArn });


  }
  private getDummyHandler(): lambda.Function {
    return new lambda.Function(this, 'Func', {
      code: lambda.Code.fromInline(`def handler(_, _): return {"foo": "bar"} `),
      functionName: 'Func',
      handler: 'index.handler',
      runtime: lambda.Runtime.PYTHON_3_10,
  })
  }
}

And the log reads:

[ERROR]	2024-02-07T16:57:21.437Z	287f7e14-ce20-45bc-a32f-3a07897f14c9	Failed to put bucket notification configuration
Traceback (most recent call last):
  File "/var/task/index.py", line 24, in handler
    s3.put_bucket_notification_configuration(Bucket=props["BucketName"], NotificationConfiguration=config)
  File "/var/runtime/botocore/client.py", line 530, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/var/runtime/botocore/client.py", line 960, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Unable to validate the following destination configurations

Upgrading this issue to p1.

@Exter-dg
Copy link

Exter-dg commented Feb 12, 2024

I am facing the same issue when adding a new SNS event notification to my already-created S3 bucket. The bucket is created using CDK only.

Error - An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation

let bucket = new s3.Bucket(this, "Bucket", {
  bucketName: bucketName
}

// new code
const snsTopic = sns.Topic.fromTopicArn(
  this,
  `snsTopic`,
  "arn"
);

bucket.addEventNotification(
  s3.EventType.OBJECT_CREATED
  new s3n.SnsDestination(snsTopic)
);

Is there a workaround?

@manmartgarc
Copy link

@Exter-dg check that there's not an existing rule that's referencing a construct that does not exist anymore. If this is the case, recreate the construct (with the same arn) and then delete the notification setting before removing the construct.

@Exter-dg
Copy link

Exter-dg commented Feb 13, 2024

@Exter-dg check that there's not an existing rule that's referencing a construct that does not exist anymore. If this is the case, recreate the construct (with the same arn) and then delete the notification setting before removing the construct.

@manmartgarc There isn't. This is the first notification that is being added to the bucket

@manmartgarc
Copy link

@Exter-dg check that there's not an existing rule that's referencing a construct that does not exist anymore. If this is the case, recreate the construct (with the same arn) and then delete the notification setting before removing the construct.

@manmartgarc There isn't. This is the first notification that is being added to the bucket

When you check the bucket notification configuration, can you verify that there's nothing there?

@Exter-dg
Copy link

Exter-dg commented Feb 13, 2024

When you check the bucket notification configuration, can you verify that there's nothing there?

@manmartgarc yes, it is empty. There are no notifications configured

@GavinZZ
Copy link
Contributor

GavinZZ commented Apr 24, 2024

@Exter-dg I can confirm that I can reproduce the issue. However, one thing I noticed is that the default access policy for SNS topic has the following condition:

"Condition": {
        "StringEquals": {
          "AWS:SourceOwner": "649563674902"
        }
      }

This seems to be the issue here and causes configuration error. According to the official documentation, one of the step is to replace the default access policy with the one they provided which uses

"Condition": {
                "ArnLike": {
                    "aws:SourceArn": "arn:aws:s3:*:*:bucket-name"
                },
                "StringEquals": {
                    "aws:SourceAccount": "bucket-owner-account-id"
                }
            }

I tried it out and this time it deploys successfully.

@GavinZZ
Copy link
Contributor

GavinZZ commented Apr 24, 2024

To summarize this issue, there are three issues I noticed in the description and comments.

  1. When using imported function to configure a lambda destination event for s3 notifications. For imported function, we will set the property canCreatePermission to false, see code. This will result in not creating lambda permission which is necessary to configure the event notification through lambda destination. Without necessary permission, creating a s3 event notification will then fail due to Unable to validate the following destination configurations.
  2. Issue when using s3 notifications on SNS topic. The default access policy of SNS topic uses "AWS:SourceOwner" in the condition. According to the official documentation, it requires a different access policy condition, specifically
  "Condition": {
      "ArnLike": {
          "aws:SourceArn": "arn:aws:s3:*:*:bucket-name"
      },
      "StringEquals": {
          "aws:SourceAccount": "bucket-owner-account-id"
      }
  }
  1. Failure when using the same type of event with overlapping prefix. Configuration is ambiguously defined. Cannot have overlapping suffixes in two rules if the prefixes are overlapping for the same event type. This is expected when the prefix is the same for the same event type. My understanding is that this issue itself complains about not able to delete the stack that is in CREATE_FAILED state due to this issue. I cannot seem to reproduce the deletion failure issue. @sarangarav would you be able to provide an example stack that can reproduce this?

@Wurstnase
Copy link
Contributor

Wurstnase commented Apr 25, 2024

Hi,

we also have sometimes issues with the notifications.

Please check the following python example:

from typing import Any

from aws_cdk import App
from aws_cdk import Stack
from aws_cdk import aws_s3 as s3
from aws_cdk import aws_s3_notifications as s3n
from aws_cdk import aws_sns as sns
from constructs import Construct


class S3BucketStack(Stack):
    def __init__(
        self,
        scope: Construct | None = None,
        id: str | None = None,
        **kwargs: Any,
    ) -> None:
        super().__init__(scope, id, **kwargs)

        bucket = s3.Bucket(self, "Bucket")
        self.bucket_arn = bucket.bucket_arn


class SnsSubscriptionStack(Stack):
    def __init__(
        self,
        scope: Construct | None = None,
        id: str | None = None,
        *,
        bucket_arn: str,
        enable_notification: bool = False,
        **kwargs: Any,
    ) -> None:
        super().__init__(scope, id, **kwargs)

        bucket = s3.Bucket.from_bucket_arn(self, "Bucket", bucket_arn=bucket_arn)

        topic = sns.Topic(self, "Topic")

        if enable_notification:
            bucket.add_event_notification(
                s3.EventType.OBJECT_CREATED,
                s3n.SnsDestination(topic),
                s3.NotificationKeyFilter(prefix="foo")
            )
        bucket.add_event_notification(
            s3.EventType.OBJECT_REMOVED,
            s3n.SnsDestination(topic),
        )

app = App()
bucket_stack = S3BucketStack(app, "BucketStack")
SnsSubscriptionStack(app, "SnsSubscriptionStack", bucket_arn=bucket_stack.bucket_arn)
app.synth()

I toggle the enable_notification flag. Sometimes this will fail with the following exception:

Error message from CDK:

The stack named SnsSubscriptionStack failed to deploy: UPDATE_ROLLBACK_FAILED (The following resource(s) failed to update: [BucketNotifications8F2E257D]. ): Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configurations overlap. Configurations on the same bucket cannot share a common event type.. See the details in CloudWatch Log Stream: 2024/04/25/[$LATEST]14ea6cb226b346e0bf9d83a800bc6461 (RequestId: 5814bc53-3bf5-40ef-be0b-578566e076b8), Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configurations overlap. Configurations on the same bucket cannot share a common event type.. See the details in CloudWatch Log Stream: 2024/04/25/[$LATEST]14ea6cb226b346e0bf9d83a800bc6461 (RequestId: c38aed01-eba7-4cd9-907a-aa6f3fcbea96)

CloudWatch log:

[ERROR]	2024-04-25T06:14:13.128Z	16a5e9ca-ba74-489c-86b3-6486d9748812	Failed to put bucket notification configuration
Traceback (most recent call last):
  File "/var/task/index.py", line 24, in handler
    s3.put_bucket_notification_configuration(Bucket=props["BucketName"], NotificationConfiguration=config)
  File "/var/runtime/botocore/client.py", line 553, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/var/runtime/botocore/client.py", line 1009, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configurations overlap. Configurations on the same bucket cannot share a common event type.

@gkaskonas
Copy link
Contributor

Hi, we are getting this error everytime we update the lambda target.

  import { Function as Handler } from "aws-cdk-lib/aws-lambda";
  ...
  const handler = Handler.fromFunctionArn(stack, "Handler", stackLocalArn);
  const bucket = Bucket.fromBucketName(stack, "Bucket", bucketName);
  bucket.addEventNotification(EventType.OBJECT_CREATED_PUT, new LambdaDestination(handler));
  return stack;

Every time stackLocalArn is updated the stack update fails with this error

Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Configurations overlap. Configurations on the same bucket cannot share a common event type.. See the details in CloudWatch Log Stream: 2024/01/29/[$LATEST]099d50dec4ab44dcb5be29f1d1b6e13c (RequestId: 9582e198-4537-479b-b112-b17761fb58a2)

Why does it try to create a new notification config instead of updating the existing one? We tried deleting the existing notification config and allowing cf to create a new one but as soon as we update the lambda it fails again.
I think this is related to this change 37be7b9

Hi @gkaskonas, What you're seeing is actually not related to the change you mentioned. After a bit investigation, I notice that you're using imported lambda function. For imported function, we will set the property canCreatePermission to false, see code.

Since this property is set to false, then when binding the LambdaDestination to s3 event notifications, it will attempt to create lambda permisison (see code) but failed to create since canCreatePermissions is false.

Without necessary permission, creating a s3 event notification will then fail due to Unable to validate the following destination configurations

It works fine when creating the notification config the first time (permissions are fine) so not sure what changes when updating it.
We are migrating to EventBridge because this is taking too long

@sarangarav
Copy link
Contributor Author

sarangarav commented Jun 21, 2024

To summarize this issue, there are three issues I noticed in the description and comments.

  1. When using imported function to configure a lambda destination event for s3 notifications. For imported function, we will set the property canCreatePermission to false, see code. This will result in not creating lambda permission which is necessary to configure the event notification through lambda destination. Without necessary permission, creating a s3 event notification will then fail due to Unable to validate the following destination configurations.
  2. Issue when using s3 notifications on SNS topic. The default access policy of SNS topic uses "AWS:SourceOwner" in the condition. According to the official documentation, it requires a different access policy condition, specifically
  "Condition": {
      "ArnLike": {
          "aws:SourceArn": "arn:aws:s3:*:*:bucket-name"
      },
      "StringEquals": {
          "aws:SourceAccount": "bucket-owner-account-id"
      }
  }
  1. Failure when using the same type of event with overlapping prefix. Configuration is ambiguously defined. Cannot have overlapping suffixes in two rules if the prefixes are overlapping for the same event type. This is expected when the prefix is the same for the same event type. My understanding is that this issue itself complains about not able to delete the stack that is in CREATE_FAILED state due to this issue. I cannot seem to reproduce the deletion failure issue. @sarangarav would you be able to provide an example stack that can reproduce this?

Sure.. Please find the example stack attached.

import * as cdk from 'aws-cdk-lib';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as s3n from 'aws-cdk-lib/aws-s3-notifications';
import * as lambda from 'aws-cdk-lib/aws-lambda';
import {Bucket, EventType, IBucket} from 'aws-cdk-lib/aws-s3';




export class S3EventNotificationsStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

  

    // Create an S3 bucket
    const testBucket = new s3.Bucket(this, 'MyBucket', {
      bucketName: 'test-event-notifications1',
      autoDeleteObjects: true, // This will automatically delete the objects when the bucket is deleted
      removalPolicy: cdk.RemovalPolicy.DESTROY, // This will delete the bucket when the stack is deleted
    });


    const bucket = Bucket.fromBucketName(
      this,
      'notifications',
      testBucket.bucketName,
    );

    console.log("print value : " + (bucket instanceof Bucket));
    // Create a Lambda function to handle the S3 event notifications
    const lambdaFunction = new lambda.Function(this, 'MyLambdaFunction', {
      runtime: lambda.Runtime.NODEJS_LATEST,
      handler: 'index.handler',
      code: lambda.Code.fromInline(`
        exports.handler = async (event) => {
          console.log('S3 event:', event);
          // Add your custom logic to handle the S3 event
          return { statusCode: 200 };
        };
      `),
      environment: {
        BUCKET_NAME: bucket.bucketName,
      },
    });

    // Grant the Lambda function permissions to access the S3 bucket
    bucket.grantReadWrite(lambdaFunction);



    /*
    *  Step 1: Deploy the stack by adding two event notifications.
    *  Step 2: Remove one of the event notifications, then redeploy the stack to reproduce the issue.
    */
    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, new s3n.LambdaDestination(lambdaFunction), { prefix: 'TEST/' });
    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, new s3n.LambdaDestination(lambdaFunction), { prefix: 'TEST_CDD/' });

  }
}

const app = new cdk.App();
new S3EventNotificationsStack(app, 'S3EventNotificationsStack');

app.synth();

@mergify mergify bot closed this as completed in #30610 Jun 26, 2024
@mergify mergify bot closed this as completed in b880067 Jun 26, 2024
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

1 similar comment
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@GavinZZ GavinZZ reopened this Jun 28, 2024
@GavinZZ
Copy link
Contributor

GavinZZ commented Jun 28, 2024

Re-opening as the original fix has a bug in the Feature flag implementation.

Copy link

github-actions bot commented Jul 2, 2024

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

1 similar comment
Copy link

github-actions bot commented Jul 2, 2024

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment