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-lambda-event-sources: S3EventSource Requires Bucket, but only needs an IBucket #23940

Closed
Styerp opened this issue Feb 1, 2023 · 6 comments
Labels
@aws-cdk/aws-lambda-event-sources duplicate This issue is a duplicate. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@Styerp
Copy link
Contributor

Styerp commented Feb 1, 2023

Describe the bug

I want to use an externally created bucket as an event notification source for a lambda

Expected Behavior

An IBucket can be used as the notification source

Current Behavior

Type errors. An IBucket cannot be assigned to a Bucket.

Reproduction Steps

import * as core from "aws-cdk-lib";
import * as event_sources from "aws-cdk-lib/aws-lambda-event-sources";
import * as s3 from "aws-cdk-lib/aws-s3";

const app = new core.App()
const ibucket = s3.Bucket.fromBucketName(app, "IBucket", "IBucket");
const eventSource = new event_sources.S3EventSource(ibucket, {events: [s3.EventType.OBJECT_CREATED]})

TS2345: Argument of type 'IBucket' is not assignable to parameter of type 'Bucket'.

Possible Solution

Since addEventNotification is the only method used by S3EventSource and is available on an IBucket, I think it should be safe to loosen the type restriction to IBucket.

Coercing the type and deploying works.

Additional Information/Context

No response

CDK CLI Version

2.40.0

Framework Version

No response

Node.js Version

16

OS

OSX

Language

Typescript

Language Version

No response

Other information

No response

@Styerp Styerp added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 1, 2023
@peterwoodworth peterwoodworth added feature-request A feature should be added or improved. p2 effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 1, 2023
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Feb 1, 2023

Thanks for this request, it looks to me like it's not necessary to only allow a Bucket, and that having IBucket here would be fine. However, this would cause issues when adding a notification for a LambdaDestination, as that requires having the Bucket construct

However there is already an existing check for this:

if (!(bucket instanceof Construct)) {
throw new Error(`LambdaDestination for function ${Names.nodeUniqueId(this.fn.permissionsNode)} can only be configured on a
bucket construct (Bucket ${bucket.bucketName})`);
}

If properly documented we could be able to allow for imported buckets to be used here to allow for more use cases

I am marking this issue as p2, which means that we are unable to work on this immediately.

@casyalex
Copy link

+1

1 similar comment
@bferdousi
Copy link

+1

@MacHu-GWU
Copy link

Tested on 2.81.0, this is not fixed yet. In case you are having the same issue, here's my alternative solution:

        bucket = s3.Bucket.from_bucket_attributes(
            self,
            "ImportedBucket",
            bucket_arn=f"arn:aws:s3:::{self.env.s3dir_source.bucket}",
        )

        use latest
        bucket.add_event_notification(
            s3.EventType.OBJECT_CREATED,
            s3_notifications.LambdaDestination(
                self.lambda_func_mapper[self.env.lbd_s3sync.name][KEY_FUNC],
            ),
            s3.NotificationKeyFilter(
                prefix=f"{self.env.s3dir_source.key}",
            ),
        )

        use alias
        bucket.add_event_notification(
            s3.EventType.OBJECT_CREATED,
            s3_notifications.LambdaDestination(
                lambda_.Function.from_function_attributes(
                    self,
                    f"LambdaAliasAttribute{self.env.lbd_s3sync.short_name_camel}",
                    function_arn=self.lambda_func_mapper[self.env.lbd_s3sync.name][KEY_ALIAS].function_arn,
                    same_environment=True,
                ),
            ),
            s3.NotificationKeyFilter(
                prefix=f"{self.env.s3dir_source.key}",
            ),
        )

@tim-finnigan
Copy link

This appears to be a duplicate of this older issue: #4323. I'm going to close this in order to consolidate issues.

@github-actions
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.

@tim-finnigan tim-finnigan added the duplicate This issue is a duplicate. label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-event-sources duplicate This issue is a duplicate. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
6 participants