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

s3: CorsRule not setting exposeHeaders due to a mis-spelled property name #30230

Closed
donicek opened this issue May 16, 2024 · 3 comments
Closed
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@donicek
Copy link

donicek commented May 16, 2024

Describe the bug

The s3.CorsRule interface contains a typo in the exposeHeaders CORS property (has exposedHeaders instead, "d" being the difference), which in turn prevents exposeHeaders to be actually set on an S3 bucket.

Expected Behavior

            cors=[
                s3.CorsRule(
                    allowed_methods=[
                        s3.HttpMethods.GET,
                        s3.HttpMethods.PUT,
                    ],
                    allowed_origins=["*"],
                    max_age=3000,
                    exposed_headers=["ETag"],
                )
            ],

sets this CORS policy on a bucket:

[
    {
        "AllowedHeaders": [
            "*"
        ],
        "AllowedMethods": [
            "GET",
            "PUT"
        ],
        "AllowedOrigins": [
            "*"
        ],
        "ExposeHeaders": [
            "ETag"
        ],
        "MaxAgeSeconds": 3000
    }
]

Current Behavior

The above code is effectively setting the CORS policy to this:

[
    {
        "AllowedHeaders": [
            "*"
        ],
        "AllowedMethods": [
            "GET",
            "PUT"
        ],
        "AllowedOrigins": [
            "*"
        ],
        "ExposeHeaders": [],
        "MaxAgeSeconds": 3000
    }
]

Reproduction Steps

Set CORS policy on an S3 Bucket with CDK, that contains settings for exposing headers.

Possible Solution

Use the correct key name here:

exposedHeaders: rule.exposedHeaders,
(exposeHeaders, that is recognized by S3).

Note that the parameter name in CDK can possibly stay the same, similar to what is being done here in a Lambda URL:

exposeHeaders: cors.exposedHeaders,

Additional Information/Context

No response

CDK CLI Version

2.118.0 (build a40f2ec)

Framework Version

No response

Node.js Version

v18.18.0

OS

any

Language

Python

Language Version

3.12

Other information

No response

@donicek donicek added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 16, 2024
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label May 16, 2024
@donicek
Copy link
Author

donicek commented May 16, 2024

Found out that CloudFormation handles this internally, and is setting the right key after all -> closing. My apologies for the false alarm.

@donicek donicek closed this as completed May 16, 2024
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.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants