aws_s3_bucket_notification - lambda - append new instead of deleting all previous

[hashibot]

This issue was originally opened by @ventz as hashicorp/terraform#11536. It was migrated here as part of the provider split. The original body of the issue is below.

---

Terraform Version

Terraform v0.8.5

Affected Resource(s)

• aws_instance

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "aws_s3_bucket_notification" "LambdaS3Notification" {
    bucket = "${var.LogBucket}"
    lambda_function = [
        {
                id = "${var.customer}-LambdaFunction"
                lambda_function_arn = "${aws_lambda_function.LambdaFunction.arn}"
                events = ["s3:ObjectCreated:*"]
                filter_suffix = "gz"
        }
    ]
}

Debug Output

N/A

Panic Output

N/A

Expected Behavior

It should appends the added lambda functions to the existing S3 Event notification set - in this case, the existing set of lambda functions.

Actual Behavior

It deletes all existing lambda functions, and adds the new one. It seems to treat the item as a single value, instead of a list which contains multiple items.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:
terraform apply -parallelism=1

Important Factoids

N/A - full access, admin account.

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:
I found this which seems semi-related: hashicorp/terraform#6934
However, that's more about the end-user creating multiple functions/resources as events.

[automaticgiant]

so yeah. this is no bueno.
i want to manage a bucket notification to trigger a lambda, but i want to manage it with the lambda.
the bucket is multi-use and i don't have control over all the bucket notifications that other teams use.

the cause is that the notifications are a subresource of the bucket, right? and tf handles subresources of a resource as a single unit?

hashicorp/terraform#11536

[deekayw0n]

Are there plans to circle back on the augment-vs-replace bucket notification model? I would think the fundamentals are there with the 'id' argument to transition to the former. Look at notifications setup for specific bucket, if new id...augment, if existing id...replace.

Or has a deeper foundational architectural decision been made here that S3 buckets should not be shared across services? I can't imagine that the conclusion would be the datastore needing to know about every function making use of it to ensure those services are properly notified. It would be like a traditional database (e.g. mysql server) having to know of all of its consumers to ensure it can provide the data they need. Its an inversion of the client-server model. Just as scalability in compatibility testing between distributed application services warrants the use of consumer-driven contracts ... the consumer of an S3 bucket should define what it needs to successfully interact with the data.

OUR SCENARIO: An application that uses domain-modeled lambdas that orchestrate an entire pipeline of input/output file processing to get raw data, clean/filter through NLP, and inevitably run through various stages of machine learning to generate insight. Each stage creates output artifacts which trigger the launch of the next lambda in the workflow. We want those artifacts in one place so that it is incredibly easy to access all the artifacts used in the journey and provide a quick mechanism to determine opportunities and gaps.

[ventz]

Personally, I think there's a bigger issue (and potentially simpler solution) here - if terraform is to "support" AWS functionality, they simply need to mirror AWS support.

That is, if AWS overrides it, they need to override it.
If AWS appends, they need to append.

In my opinion, having a split decision on "what functions and how" is much worse.

Currently, AWS appends, and this is what terraform should do/support.

[joshuaeilers]

+1

[paulrigor]

+1

[plsanchezfaure]

+1

[ivankatliarchuk]

+1

[triduongtran]

+1

[m-hosoi]

+1

[CosterANZ]

+1

[acobaugh]

Can we get an official statement from Hashicorp regarding the behavior of this resource? If the desire is to alter the behavior to support multiple notification 'attachments', I imagine there are those in the community (myself included) that wouldn't mind putting together a PR to make this work, with guidance from Hashicorp.

In the project I'm working on, we're using SQS notifications where the same replace instead of add behavior is seen. We're trying to avoid bucket sprawl, but because of this behavior we're having to make an architectural decision that buckets:notifications need to be 1:1. This means we stand the chance of hitting the 100 bucket per account limit in AWS, as this is a design pattern we're likely to use in multiple projects (using S3 as a sort of drop box for batch file processing).

[woz5999]

@acobaugh I had a similar situation where I needed to send the s3 notification to multiple queues and ran up against this. I was able to work around it by sending the s3 notification to an SNS topic which was then configured to forward the notification into an arbitrary number of SQS queues. It's not as nice as just being able to attachment multiple notifications, but it works well and doesn't require 100 buckets.

[ventz]

@bflad @jbardin @apparentlymart ^ ping -- it seems this very significant bug was accidentally classified as an "enhancement" and forgotten.

The original was here: hashicorp/terraform#11536
It was then moved here: #501

This is pretty major functionality -- as of right now, Terraform has limited S3 notification to a single lambda function.

• The AWS spec for it is a list of lambda functions.
• Terraform treats it as single lambda function, and overrides previous when additional are added.

[apparentlymart]

Hi all,

This resource is exposing the functionality of the S3 PUT Bucket notification operation, which treats the entire notification set as a single object that must be replaced atomically.

Because individual notification rules are not independently addressable, Terraform can't get any more granular than what it's already doing within the current design of the API.

It would theoretically be possible to implement separate resource types like aws_s3_bucket_notification_lambda_function, aws_s3_bucket_notification_sqs_queue, etc that represent individual notification targets within a bucket by attempting to fetch the current configuration, search for any rule with a matching id, replace it, and re-submit the whole object.

resource "aws_s3_bucket_notification_lambda_function" "example" {
  bucket = "${var.LogBucket}"
  id = "${var.customer}-LambdaFunction"
  lambda_function_arn = "${aws_lambda_function.LambdaFunction.arn}"
  events = ["s3:ObjectCreated:*"]
  filter_suffix = "gz"
}

Resource types like this would have to somehow avoid creating race conditions for other concurrent writers to the notification configuration for the same bucket, particularly if there were multiple resources of these types in the same configuration where Terraform itself might try to write them all concurrently. Since the underlying API provides no locking mechanism for this, it is not totally solvable. It may be partially solvable with certain caveats, but it would mean that these resources types would have quite complex implementations in relation to other resource types.

I didn't originally set the labels here, but my guess is that the "enhancement" label here is representing that implementing management of individual rules in isolation from inside a single remote notification object would be additional functionality above what the underlying API provides and is thus an extension of the current behavior, not a fix for an implementation defect.

[oceanlewis]

Just throwing this out there that I'm experiencing pain around this same issue:

resource "aws_s3_bucket_notification" "S3BucketNotification" {
  bucket = "${data.aws_s3_bucket.S3Bucket.id}"

  queue {
    queue_arn     = "${module.sqs.queue_arn}"
    events        = ["s3:ObjectCreated:*"]
    filter_prefix = "${local.S3EventsPrefix}/"
    filter_suffix = ".gz"
  }
}

Consider the above example, if I'm using multiple Terraform Workspaces to have staging, production, etc. environments operate on the same bucket, I'm going to overwrite every other notification set up for every other environment. This means if I apply any changes to staging, production is affected. So yeah, if this could be prioritized and a fix released, that would be absolutely fantastic.

[OferE]

The problem is not with Hashicorp. The Problem is AWS and their poor API for S3 events. This is a huge design bug of AWS.

…

On Sat, Jan 19, 2019 at 1:26 AM David Lewis ***@***.***> wrote: Just throwing this out there that I'm experiencing pain around this same issue: resource "aws_s3_bucket_notification" "S3BucketNotification" { bucket = "${data.aws_s3_bucket.S3Bucket.id}" queue { queue_arn = "${module.sqs.queue_arn}" events = ["s3:ObjectCreated:*"] filter_prefix = "${local.S3EventsPrefix}/" filter_suffix = ".gz" } } Consider the above example, if I'm using multiple Terraform Workspaces to have staging, production, etc. environments operate on the same bucket, I'm going to overwrite every other notification set up for every other environment. This means if I apply any changes to staging, production is affected. So yeah, if this could be prioritized and a fix released, that would be absolutely fantastic. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#501 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB-znxjPve9oGIIuu4fJ_VJuXoYj1_wMks5vElgVgaJpZM4N49yk> .

-- Regards, Ofer Eliassaf

[oceanlewis]

It looks like it's possible to get and put a bucket's notification configuration, so no, not a limitation on AWS's side. It should be possible to put a configuration rule and store the id of that configuration rule in the tfstate to reference later if Terraform detects a change in the configuration. Maybe there's another limitation I'm not aware of, if so I'd be interested to hear it.

Relevant docs:

https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html?highlight=bucket%20notification%20config#S3.Client.put_bucket_notification_configuration

https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html?highlight=bucket%20notification%20config#S3.Client.get_bucket_notification_configuration

[butzist]

Biggest issue seems to be that you can not have two rules with overlapping prefixes. So even if you could define multiple separate rules for one bucket, you could not define two Lambdas to be triggered for same prefix.

[ethicalmohit]

I am getting the same issue. Are there any workarounds?

[oceanlewis]

@ethicalmohit I've stopped managing these with Terraform. Best workaround I've come across so far :).

[eldondevat]

Maybe there's another limitation I'm not aware of, if so I'd be interested to hear it.

I think the design flaw mentioned here is the lack of granularity of the AWS service. Yes, you can put/set, and maybe it isn't a big deal in this specific scenario, as contention is unlikely, but seems like this could represent a TOCTOU bug. If two teams were managing these things with terraform, the concern may be that
Terraform for team A checks for the notification on the bucket, there is none(GET).
Terraform for team B checks for the notification on the bucket(GET).
Terraform for team B creates a notification on the bucket(PUT).
Terraform for team A creates a notification on the bucket (PUT), blowing away team B's notification.
AFAIK remedying this isn't possible without a compare-and-set type operation. I didn't see that in the docs.

[Wenzil]

Any update on this?

[bryanyang0528]

It's really a very dangerous function. When you apply a new rule on an existing s3 bucket, all existing rules will be deleted without any prompt warning and can't be rolled back.

[OferE]

The problem is not in Terraform. It's an aws API bug. The workaround i found is a dispatcher mechanism in which, a single lambda is registered, and when it is called, it send a SNS notification with the path. All the multiple lambdas I have listen to this SNS topic and are triggered. This works great and highly reccomended. בתאריך יום ה׳, 30 באפר׳ 2020, 18:10, מאת Bryan Yang ‏< notifications@github.com>:

…

It's really a very dangerous function. When you apply a new rule on an existing s3 bucket, all existing rules will be deleted without any prompt warning and can't be rolled back. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#501 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAP3HHYYOWLDYXH6OVLLVZLRPGIFNANCNFSM4DPD3SSA> .

[ventz]

@bryanyang0528 Yes - exactly the reason I first created this issue 3+ years ago.

But I believe somewhere a while back (can't keep track at this point since this ticket has been moved/re-opened 4-5x) it was mentioned that the problem is the Amazon API available for lambda? That said, I really think this is something that should be brought up and fixed (be it Amazon or TF) quickly.

[bryanyang0528]

@ventz I'm not sure if terraform could refer to this project serverless-plugin-existing-s3? This serverless plugin will fetch all existing rules, add a new rule, and deploy them together for implementing this function.

[ventz]

@bryanyang0528 That would be fantastic!

[bflad]

Hi folks 👋

The core of this aws_s3_bucket_notification resource issue is still pending any sort of enhancement from the S3 API to identify or support individual bucket notification configuration operations. Essentially, this comment still applies where the S3 PutBucketNotificationConfiguration API call has the following behavior:

This operation replaces the existing notification configuration with the configuration you include in the request body.

The current maintainers are still very hesitant to implement a custom solution of individual notification management since this would be extremely prone to S3 eventual consistency behaviors and unable to handle race conditions across Terraform AWS Provider instances. Our best recommendation would still be to reach out to the S3 service team about this particular API via AWS Support cases or discussions with your Technical Account Manager, should you have one.

However, I did want to mention here the feature request for Terraform and Terraform AWS Provider to give an error message for conflicting resources in configurations, which falls under the provider-wide enhancement proposal of #14394. By adding this link here it will add a reference to that issue so we can include it as a use case when thinking about the implementation details about that particular enhancement. Please feel free to 👍 and provide feedback about that enhancement there.

[harrydaniels-IW]

We solved this issue by using a null resource which calls a python script with the necessary arguments. The script pulls the existing notifications into memory, adds the new one and uploads them all again to s3. The reason we needed this is because we use both Serverless and Terraform and needed a way to handle bucket notifications across stacks.

[spritchard-aptiv]

We solved this issue by using a null resource which calls a python script with the necessary arguments. The script pulls the existing notifications into memory, adds the new one and uploads them all again to s3. The reason we needed this is because we use both Serverless and Terraform and needed a way to handle bucket notifications across stacks.

Hi @harrydaniels-IW, that's the best idea I've heard yet. Are you willing to share that script with us?

[matthewpick]

boto3 put_bucket_notification_configuration has the same behavior. There is no easy way to "non-destructively" update a bucket's notification rules, without creating your own custom script which merges old and new rules.

Best case scenario, my company moves all of our bucket notification rules to terraform. Otherwise I will continue to manage my bucket notifications via boto3 + some custom logic (rather than terraform). Not ideal, but it works.

[Blaked84]

I made a Terraform module to manage S3 notifications to Lambda without deleting existing ones.
It only manages S3 to Lambda notifications for now (other notification will be deleted) and I'll support other destinations in the next weeks.

It's still in alpha, but it's works on my own infrastructure.
I share it here if it can help you save a few hours: https://github.com/evalmee/s3_bucket_notification_lambda

[caseyc49]

+1!

My team (data engineering) is working out a POC for moving everything over to terraform but this is a huge issue for us since we have a ton of different s3 triggers from our main bucket.

Is this fix currently in the pipeline?

[naslanidis]

We solved this issue by using a null resource which calls a python script with the necessary arguments. The script pulls the existing notifications into memory, adds the new one and uploads them all again to s3. The reason we needed this is because we use both Serverless and Terraform and needed a way to handle bucket notifications across stacks.

If you can do this via Python why can't Terraform do it? People are blaming the AWS API but surely you're consuming the python boto3 sdk?

[walter9388]

I also ran into an issue here! Hoping Hashicorp can sort out an official solution soon.

Considering this idea:

We solved this issue by using a null resource which calls a python script with the necessary arguments. The script pulls the existing notifications into memory, adds the new one and uploads them all again to s3. The reason we needed this is because we use both Serverless and Terraform and needed a way to handle bucket notifications across stacks.

And considering @Blaked84 has already done some of the work:

I made a Terraform module to manage S3 notifications to Lambda without deleting existing ones. It only manages S3 to Lambda notifications for now (other notification will be deleted) and I'll support other destinations in the next weeks.

It's still in alpha, but it's works on my own infrastructure. I share it here if it can help you save a few hours: https://github.com/evalmee/s3_bucket_notification_lambda

I forked the above repo and implemented something similar using a bash script as Ruby isn't a possibility in my team's current setup.

Available here if it helps save anyone some work: https://github.com/walter9388/s3_bucket_notification_lambda
(Please read the list of limitations in the README before using it!!!)

[ngjm]

+1 also running into this issue.

[matthewpick]

For additional context, aws-cdk implemented a pattern which auto-merges old and new Bucket Configurations via their addEventNotifications helper method.

This deploys a custom resource lambda under the hood, code for this pattern uses boto3 for API calls (custom lambda code).

[cwalker-vizio]

+1 please fix