(synthetics): getbucketlocation policy is incorrect

[csumpter]

Synthetics canary default role policy contains the incorrect arn syntax for a call to s3:GetBucketLocation

When using Synthetics runtime "syn-nodejs-puppeteer-3.1" the canary will try to call s3:GetBucketLocation but with an improper policy which will result in denied access.

Reproduction Steps

minimal amount of code that causes the bug (if possible) or a reference:
The current implementation on master is bugged:

        new iam.PolicyStatement({
          resources: [this.artifactsBucket.arnForObjects(`${prefix ? prefix+'/*' : '*'}`)],
          actions: ['s3:PutObject', 's3:GetBucketLocation'],
        })

What did you expect to happen?

Allow the "syn-nodejs-puppeteer-3.1" runtime to operate correctly without generating IAM access denied errors.

What actually happened?

The role will be denied access by IAM get call s3:GetBucketLocation on that S3 bucket.

Environment

• CDK CLI Version :
• Framework Version:
• Node.js Version:
• OS :
• Language (Version):

Other

Should be fixed by creating a separate policy that breaks s3:GetBucketLocation out into a separate policy that is targeted specifically at just the bucket arn:

        new iam.PolicyStatement({
          resources: [this.artifactsBucket.bucketArn],
          actions: ['s3:GetBucketLocation'],
        }),
        new iam.PolicyStatement({
          resources: [this.artifactsBucket.arnForObjects(`${prefix ? prefix+'/*' : '*'}`)],
          actions: ['s3:PutObject'],
        }),

---

This is 🐛 Bug Report

[NetaNir]

What error are you seeing? Is it specific to the runtime you are using? Would be helpful if you can attache the documentation supporting the need to add this policy. We tested this use case when we wrote the construct and it work. I'm also troubled with the years old question "how did it work till now?"

[csumpter]

Hello @NetaNir - thank you for taking the time to respond.

The specific runtime is syn-nodejs-puppeteer-3.1. When you manually create a syn-nodejs-puppeteer-3.1 based synthetic in the console, you get the following policy as the default IAM role attached to the canary.

...
        {
            "Effect": "Allow",
            "Action": [
                "s3:PutObject"
            ],
            "Resource": [
                "arn:aws:s3:::cw-syn-results-<account-id>-us-east-1/canary/us-east-1/testscanary-733-11b42a87105e/*"
            ]
        },
        {
            "Effect": "Allow",
            "Action": [
                "s3:GetBucketLocation"
            ],
            "Resource": [
                "arn:aws:s3:::cw-syn-results-<account-id>-us-east-1"
            ]
        },
...

Versus the synthetics-cdk default role that gets provisioned is equivalent to:

        {
            "Effect": "Allow",
            "Action": [
                "s3:PutObject",
                "s3:GetBucketLocation"
            ],
            "Resource": [
                "arn:aws:s3:::cw-syn-results-<account-id>-us-east-1/canary/us-east-1/testscanary-733-11b42a87105e/*"
            ]
        },

The above policy generates the following error in the canary logs:

INFO: S3 destination for uploading artifacts determined: {"s3Bucket":"cw-syn-results-<account-id>-us-east-1","s3Key":"<redacted>"}
ERROR: Unable to fetch S3 bucket location: Access Denied. Fallback to S3 client in current region: us-east-1.

---

syn-nodejs-2.2 does not exhibit this error. The first runtime to produce the access denied error is syn-nodejs-puppeteer-3.0 - so my best guess as to "how did it work till now?" is that section of the role policy was not getting exercised until syn-nodejs-puppeteer-3.0 was introduced.

[peterwoodworth]

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

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

[wilhen01]

We've just run across this issue, would be great if it could be fixed.

[shooit]

There is an open PR for a fix for this issue. Would it be possible to review that and merge?

[github-actions]

⚠️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.