crossplane-contrib / provider-aws

Crossplane AWS Provider
Apache License 2.0
429 stars 367 forks source link

S3 Bucket constantly calls PutBucketNotification #1165

Closed empath-nirvana closed 2 years ago

empath-nirvana commented 2 years ago

What happened?

The issue is similar to this issue, however, I've hardcoded the ID in the notifications queue configuration and we're using version 0.21.2 which should have this fix in place: https://github.com/crossplane/provider-aws/issues/476

I've created a queue and it continuously tries to put in the notifications configuration, generating a test message every time.

How can we reproduce it?

Create a bucket with a notifications queue, check cloudtrail logs to see that it's making an api call every minute.

What environment did it happen in?

Crossplane version:

MisterMX commented 2 years ago

@empath-nirvana are you using queueArnRef to get the queue? If you so, this might have been fixed in #1161. Can you check again with the latest master?

empath-nirvana commented 2 years ago

We aren't using that.

      - ID: Queue
        events:
        - s3:ObjectCreated:*
        filter:
          key:
            filterRules:
            - name: prefix
              value: (some prefix)
        queueArn: <some queue arn>

the bucket does work, it's just resyncing 1000 times a day and causing test messages to be sent.

Is there a way to get detailed logging so i can see what it thinks the difference is?

MisterMX commented 2 years ago

Thanks @empath-nirvana. I was able to reproduce the bug with your YAML.

It seems the cause of this issue is that the prefix filter rule is converted to camel case Prefix which generates a diff:

  []types.QueueConfiguration{
    {
        ... // 1 ignored and 1 identical fields
        Filter: &types.NotificationConfigurationFilter{
            Key: &types.S3KeyFilter{
                FilterRules: []types.FilterRule{
                    {
-                       Name: "Prefix",
+                       Name: "prefix",
                        ... // 2 ignored fields
                    },
                },
                ... // 1 ignored field
            },
            ... // 1 ignored field
        },
        ... // 2 ignored fields
    },
  }

This seems to be an issue on AWS side:

{
    "QueueConfigurations": [
        {
            "Id": "...",
            "QueueArn": "...",
            "Events": [
                "s3:ObjectCreated:*"
            ],
            "Filter": {
                "Key": {
                    "FilterRules": [
                        {
                            "Name": "Prefix",
                            "Value": "some-prefix"
                        }
                    ]
                }
            }
        }
    ]
}

The SDK itself also only defines prefix and suffix as correct values.

empath-nirvana commented 2 years ago

Is there a way to work around this that doesn't involve waiting for a patch to the aws sdk for go?

MisterMX commented 2 years ago

Its a bit difficult because the controller uses go-cmp to compare the configurations and it does not have an ignore case option AFAIK.

@muvaf what would be the best way to circumvent this issue?

empath-nirvana commented 2 years ago

What happens if you change the enum to allow "Prefix" and "Suffix"? Does s3 only accept lower case?

empath-nirvana commented 2 years ago

Given that aws-sdk has said they won't fix the issue, is there some way this can be fixed by the cross-plane provider?

MisterMX commented 2 years ago

Since its a bug at AWS it would be best to wait for them to fix it on their side.

On provider level you could only add a hacky workaround by converting the respective fields to lowercase strings or find a way to ignore case when doing cmp.Equal.

empath-nirvana commented 2 years ago

I talked to someone on the S3 team. It’s extremely unlikely they’re going to modify S3 api behavior.

On Thu, Mar 10, 2022 at 6:32 AM MisterMX @.***> wrote:

Since its a bug at AWS it would be best to wait for them to fix it on their side.

On provider level you could only add a hacky workaround by converting the respective fields to lowercase strings or find a way to ignore case when doing cmp.Equal.

— Reply to this email directly, view it on GitHub https://github.com/crossplane/provider-aws/issues/1165#issuecomment-1063959137, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUVYTL5PMTCDTPKHU6PIIUTU7HMTHANCNFSM5O7OYNCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- “The world is not comprehensible, but it is embraceable.”

Yaswanth45 commented 6 months ago

Facing same issue but with TopicConfiguration.Filter.Key.FilterRules.Name. Crossplane is infinitely calling the PutBucketNotification on our bucket. Seems the same fix need to be applied to TopicConfiguration as well.