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::S3::BucketNotification #79

Open
benkehoe opened this issue Aug 1, 2019 · 38 comments
Open

AWS::S3::BucketNotification #79

benkehoe opened this issue Aug 1, 2019 · 38 comments
Labels
enhancement New feature or request storage S3, EFS, FSx, S3 Glacier, Storage Gateway, AWS Backup
Milestone

Comments

@benkehoe
Copy link
Contributor

benkehoe commented Aug 1, 2019

  1. Title -> AWS::S3::BucketNotification
  2. Scope of request -> Allow bucket notifications to be managed separate from the bucket resource itself, resolving a longstanding circular reference problem
  3. Expected behavior -> I should be able to create auto-named buckets with notifications that invoke Lambda/SNS/SQS
  4. Links to existing API doc -> see below
  5. Category tag -> Compute, Storage
  6. Additional context:

The problem:

  1. Image thumbnailing is serverless 101. It involves setting up bucket notifications to invoke a Lambda function on file upload to a bucket (then generate the thumbnails and write them back to the bucket).
  2. A best practice for CloudFormation is to let CloudFormation name your resources wherever possible, and only deal with logical ids, not physical resource ids.
  3. These two things cannot currently be accomplished simultaneously. There needs to be a Lambda permission or SNS/SQS topic/queue policy, which needs to reference the bucket name, but the permission is checked for at notification configuration creation, before the bucket name could be provided to the permission resource.

Fundamentally, this is because there is not a separation between the the creation of a bucket (and its name) and the settings on that bucket. There are at least three separate places on AWS that say 🤷 to customers and tell them to manually create a bucket name in two separate places, which is brittle both in terms of multiple deployments of the template and in terms of updating that bucket name in the future.

This could instead be solved with a separate BucketNotification resource. The bucket resource would be created first, the name !Ref'd to the relevant places, and then the BucketNotification resource would install the notification configuration onto the bucket.

@rosskarchner
Copy link

Would this help solve this SAM issue? aws/serverless-application-model#138

@benkehoe
Copy link
Contributor Author

benkehoe commented Aug 1, 2019

Yes, it's exactly that same problem.

@TheDanBlanco TheDanBlanco added the storage S3, EFS, FSx, S3 Glacier, Storage Gateway, AWS Backup label Aug 1, 2019
@luiseduardocolon luiseduardocolon added the enhancement New feature or request label Aug 8, 2019
@rosskarchner
Copy link

AWS folks, any chance this will move onto the board soon?

@luiseduardocolon
Copy link
Contributor

We're keeping an eye on the +1s on this, but we're trying to prioritize coverage items first.

@tebruno99
Copy link

This took us by surprise today. Seems to make Policy Templates unusable.. Please Fix! We don't like letting workaround hacks live in our production environments.

@dennisandersen
Copy link

+1 on this. This has hit us more than once and feel this should be prioritized. It is not possible to achieve what i consider "THE" base use-case for bucket notifications: "read file that was just added to bucket", without resolving to cumbersome workarounds.

@TonyFNZ
Copy link

TonyFNZ commented May 7, 2020

A couple of useful links:

Existing Custom Resource which implements this functionality:
https://aws.amazon.com/premiumsupport/knowledge-center/cloudformation-s3-notification-lambda/

CDK issue which is blocked by this issue: aws/aws-cdk#4323

@purnesh
Copy link

purnesh commented Jul 11, 2020

+1

2 similar comments
@major-fire
Copy link

+1

@jorgeandresvasquez
Copy link

+1

@craigataws craigataws added this to the cov milestone Jul 21, 2020
@yeDor
Copy link

yeDor commented Jul 24, 2020

+1

1 similar comment
@jorgeandresvasquez
Copy link

+1

@jeffmarcinko
Copy link

+1

Still implementing workarounds like this, https://aws.amazon.com/blogs/mt/resolving-circular-dependency-in-provisioning-of-amazon-s3-buckets-with-aws-lambda-event-notifications/

Plus-one for all of Ben's original points.

@IanShoe
Copy link

IanShoe commented Jul 27, 2020

+1

2 similar comments
@adhandharia
Copy link

+1

@sahil-gt
Copy link

+1

@kz974
Copy link

kz974 commented Oct 19, 2020

Yes! +1

@jamescarignan
Copy link

+1

@benbridts
Copy link

@purnesh @yeDor @IanShoe @kz974 @jamescarignan

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request

@pam81
Copy link

pam81 commented Dec 11, 2020

+1

1 similar comment
@fwanghe
Copy link

fwanghe commented Apr 14, 2021

+1

@cfn-github-issues-bot cfn-github-issues-bot moved this from Researching to Shipped in coverage-roadmap Oct 4, 2021
@anowac01
Copy link

anowac01 commented Oct 4, 2021

How was this issue resolved? I don't see any updates in the CloudFormation documentation relevant to it, and it still warns against the circular dependency:
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-notificationconfig.html

@omkhegde omkhegde reopened this Oct 4, 2021
@cfn-github-issues-bot cfn-github-issues-bot moved this from Shipped to Researching in coverage-roadmap Oct 4, 2021
@cfn-github-issues-bot cfn-github-issues-bot moved this from Researching to We're working on it in coverage-roadmap Oct 8, 2021
@zwezheng
Copy link

+1

4 similar comments
@gnobre
Copy link

gnobre commented Nov 25, 2021

+1

@Us3rname
Copy link

+1

@qcurtemanjc
Copy link

+1

@agniswarmandal
Copy link

+1

@benkehoe
Copy link
Contributor Author

@zwezheng @gnobre @Us3rname @qcurtemanjc @agniswarmandal (and future potential "+1" commenters)

If you react with the 👍 button to the original issue, (the first comment, click on the smiley face if you're the first reacting), your votes can be used to sort issues and determine priorities.

A comment will send a notification to everyone (participants and watchers), but cannot be easily counted as a vote for an issue. Thus It's generally better to vote than to comment with "+1". To keep up to date, you can also add yourself as a watcher.

@polaskj
Copy link

polaskj commented Jan 13, 2022

It looks like a more elegant solution for this is finally here with this announcement, using EventBridge. https://aws.amazon.com/blogs/aws/new-use-amazon-s3-event-notifications-with-amazon-eventbridge/

Here's a pseudo Lambda invocation example I tested with success. This assumes your externally referenced S3 Bucket enables EventBridge

Stack:

EventRule:
  Type: AWS::Events::Rule
  Properties:
    Description: EventRule
    State: ENABLED
    EventPattern: # https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-event-patterns.html#eb-filtering-data-
      source:
        - aws.s3
      detail-type:
        - "Object Created"
      detail:
        bucket:
          name:
            - "EXTERNAL-BUCKET"
    Targets:
      - Arn: !GetAtt MyLambda.Arn
        Id: MyLambdaFunctionTarget

PermissionForEventsToInvokeLambda:
  Type: AWS::Lambda::Permission
  Properties:
    FunctionName: !Ref MyLambda
    Action: lambda:InvokeFunction
    Principal: events.amazonaws.com
    SourceArn: !GetAtt EventRule.Arn

MyLambda:
  Type: AWS::Serverless::Function
  ... 

Bucket Stack:

S3Bucket:
  Type: AWS::S3::Bucket
  Properties:
    ...
    BucketName: "EXTERNAL-BUCKET"
    NotificationConfiguration:
      EventBridgeConfiguration:
        EventBridgeEnabled: true

Linking back to aws/serverless-application-model#124 since this is almost exactly the use-case.

@cfn-github-issues-bot cfn-github-issues-bot moved this from We're working on it to Researching in coverage-roadmap Jan 26, 2022
@cfn-github-issues-bot cfn-github-issues-bot moved this from Researching to Coming Soon in coverage-roadmap Apr 26, 2022
@cfn-github-issues-bot cfn-github-issues-bot moved this from Coming Soon to Researching in coverage-roadmap Apr 27, 2022
@cfn-github-issues-bot cfn-github-issues-bot moved this from Researching to Coming Soon in coverage-roadmap Apr 27, 2022
@albaymar albaymar added enhancement New feature or request Coverage and removed Coverage enhancement New feature or request labels Apr 27, 2022
@souvikataws
Copy link

Thanks for the feedback. We have created a Product Feature Request which will be prioritized with other features planned for Amazon S3. As another option, you can enable EventBridge notifications on the S3 Bucket (there are details on getting started here). Additionally, you can refer to an example from Polaskj who has provided a CloudFormation template using EventBridge and Lambda as a destination.

@WaelA WaelA added enhancement New feature or request and removed Coverage labels Apr 27, 2022
cstyan pushed a commit to grafana/loki that referenced this issue Sep 9, 2023
**What this PR does / why we need it**:
Lambda promtail supports s3 events, which are used for scraping several
log sources such as ALB access logs. This works by configuring at the S3
bucket level "s3 event notification", that are configured to target the
lambda deployment of lambda-promtail.

However, if one is configuring this through CloudFormation, there's a
known issue with AWS that doesn't allow to configure both the lambda,
the bucket, and the notifications in the same stack. See [this
issue](aws-cloudformation/cloudformation-coverage-roadmap#79)
for details.

For that, AWS introduced EventBridge notifications, which can be used to
ship s3 events to a lambda deployment as well. This flow looks like:
s3 -> eventbridge bus -> eventbridge rule -> lambda

EventBridge has it's own message structure for s3 notifications. This PR
adds a translation layer, just for `Object created` events (since they
are the only ones we should take into account), so that EventBridge
events can be received, and trigger the lambda as if they were from s3.

**Which issue(s) this PR fixes**:
Fixes #10209

**Special notes for your reviewer**:

- [x] Pending testing this with an actual deployment of the s3 -> event
bridge -> lambda flow
- [x] ~~Add CF template for the `s3 -> event bridge -> lambda`
deployment~~ Follow up PR

**Checklist**
- [ ] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [x] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](d10549e)
@XrayBravoGolf
Copy link

XrayBravoGolf commented Mar 19, 2024

The related (downstream) SAM issue aws/serverless-application-model#124 has been around since 2017 and #79 has been on the Coming Soon ™️ project board for a while.
Any updates? Timelines?

I see many AWS members at this thread then have moved on, so I'd appreciate anybody who is able to check in on the status of the feature request / issue. @benkehoe is this something you are able to help?

rhnasc pushed a commit to inloco/loki that referenced this issue Apr 12, 2024
**What this PR does / why we need it**:
Lambda promtail supports s3 events, which are used for scraping several
log sources such as ALB access logs. This works by configuring at the S3
bucket level "s3 event notification", that are configured to target the
lambda deployment of lambda-promtail.

However, if one is configuring this through CloudFormation, there's a
known issue with AWS that doesn't allow to configure both the lambda,
the bucket, and the notifications in the same stack. See [this
issue](aws-cloudformation/cloudformation-coverage-roadmap#79)
for details.

For that, AWS introduced EventBridge notifications, which can be used to
ship s3 events to a lambda deployment as well. This flow looks like:
s3 -> eventbridge bus -> eventbridge rule -> lambda

EventBridge has it's own message structure for s3 notifications. This PR
adds a translation layer, just for `Object created` events (since they
are the only ones we should take into account), so that EventBridge
events can be received, and trigger the lambda as if they were from s3.

**Which issue(s) this PR fixes**:
Fixes grafana#10209

**Special notes for your reviewer**:

- [x] Pending testing this with an actual deployment of the s3 -> event
bridge -> lambda flow
- [x] ~~Add CF template for the `s3 -> event bridge -> lambda`
deployment~~ Follow up PR

**Checklist**
- [ ] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [ ] Documentation added
- [x] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [ ] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@d10549e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request storage S3, EFS, FSx, S3 Glacier, Storage Gateway, AWS Backup
Projects
coverage-roadmap
  
Coming Soon
Development

No branches or pull requests