Skip to content

feat: per-bucket CORS configuration (S3 ?cors) with enforcement - #71

Open
h5vx wants to merge 6 commits into
pgsty:mainfrom
h5vx:feature/per-bucket-cors
Open

feat: per-bucket CORS configuration (S3 ?cors) with enforcement#71
h5vx wants to merge 6 commits into
pgsty:mainfrom
h5vx:feature/per-bucket-cors

Conversation

@h5vx

@h5vx h5vx commented Aug 25, 2026

Copy link
Copy Markdown

Contribution Licensing (no CLA, inbound=outbound, DCO required)

This project does not use a CLA; contributions are accepted inbound=outbound.
By submitting this pull request I represent that I have the right to contribute
the changes, which are licensed under this repository's
GNU Affero General Public License v3.0 or later
and remain my copyright. Every commit must carry a DCO Signed-off-by trailer
(git commit -s) certifying the
Developer Certificate of Origin — see
CONTRIBUTING.md.

Description

Implements S3-compatible per-bucket CORS. Clients manage a bucket's CORS
configuration through the standard ?cors subresource (PUT/GET/DELETE), and
the server enforces it on requests (OPTIONS preflight and Access-Control-*
response headers on actual requests).

  • New internal/bucket/cors package: the <CORSConfiguration> XML type,
    validation (≤100 rules, methods restricted to GET/PUT/HEAD/POST/DELETE,
    non-empty origins/methods, non-negative MaxAgeSeconds), and origin/method/
    header matching (single-* wildcard, first-match, and preflight
    fall-through so a restrictive earlier rule does not shadow a later match).
  • Storage: CorsConfigXML / CorsConfigUpdatedAt added to BucketMetadata
    (msgp regenerated), persisted as cors.xml, cached in BucketMetadataSys
    with GetCorsConfig / GetCorsConfigXML getters.
  • S3 handlers in cmd/bucket-cors-handlers.go replace the previous
    NotImplemented/stub handlers; the stale cors entry is removed from
    rejectedBucketAPIs.
  • Enforcement in the corsHandler middleware: a bucket with a stored config
    governs its own requests (no matching rule ⇒ no CORS headers / 403 on
    preflight, per S3 semantics); buckets without a config keep today's global
    rs/cors behavior unchanged.

Compatible with standard S3 tooling (aws-cli, boto3, the ansible
community.aws.s3_cors module).

Motivation and Context

The ?cors routes and policy actions (GetBucketCorsAction,
PutBucketCorsAction, DeleteBucketCorsAction) already existed, but the
handlers were stubs returning NotImplemented, and CORS could only be
configured globally via cors_allow_origin. This adds real per-bucket CORS
so browser clients can be scoped per bucket, matching AWS S3 behavior.

How to test this PR?

Unit / package tests:

go test ./internal/bucket/cors/... ./cmd/ -run 'Cors|CORS'
make verifiers

Manual smoke test with boto3:

s3.put_bucket_cors(Bucket="b", CORSConfiguration={"CORSRules": [
    {"AllowedOrigins": ["https://app.example.com"],
     "AllowedMethods": ["GET", "PUT"],
     "AllowedHeaders": ["Authorization"],
     "MaxAgeSeconds": 3000}]})
s3.get_bucket_cors(Bucket="b")     # returns the rule
s3.delete_bucket_cors(Bucket="b")  # subsequent GET -> NoSuchCORSConfiguration

Compatibility impact

  • APIs/clients: Adds the S3 ?cors PUT/GET/DELETE endpoints (previously
    stubs returning NotImplemented). Buckets with no CORS config behave exactly
    as before (global rs/cors fallback). No client changes required.
  • Storage metadata: BucketMetadata gains CorsConfigXML and
    CorsConfigUpdatedAt, persisted as the cors.xml per-bucket config file.
    The msgp map header grows accordingly. Additive and backward-compatible:
    older metadata decodes with empty CORS fields; a server without this change
    ignores the extra cors.xml. Rollback-safe (no CORS enforcement, config
    file left dormant on disk).
  • Preserved compatibility names (unchanged by design, kept for MinIO/S3
    interoperability): error code NoSuchCORSConfiguration; policy actions
    s3:GetBucketCors / PutBucketCors / DeleteBucketCors; the ?cors
    subresource; x-amz-* request/response headers; the github.com/minio/*
    import paths. New exported symbols are recorded in the rebrand-guard
    compatibility baseline (buildscripts/rebrand-guard/compat-baseline.json).
  • Site replication: invokes the existing BucketMetaHook
    (SRBucketMetaTypeCorsConfig) for parity with other per-bucket configs.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • All commits are signed off (git commit -s) per the DCO
  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • make verifiers passes
  • Relevant package tests and make build pass
  • Compatibility and rollback impact documented
  • Internal documentation updated
  • Public documentation update opened in pgsty/silo.pgsty.com, if needed

h5vx added 6 commits August 25, 2026 19:24
Signed-off-by: h5vx <h5v@protonmail.com>
Signed-off-by: h5vx <h5v@protonmail.com>
Signed-off-by: h5vx <h5v@protonmail.com>
Signed-off-by: h5vx <h5v@protonmail.com>
…T, e2e test)

Signed-off-by: h5vx <h5v@protonmail.com>
Reformat cors_test.go per gofumpt and regenerate the rebrand-guard
compatibility baseline to record the per-bucket CORS feature's new
exported symbols (internal/bucket/cors types and BucketMetadata/
BucketMetadataSys additions).

Signed-off-by: h5vx <h5v@protonmail.com>
@h5vx

h5vx commented Aug 25, 2026

Copy link
Copy Markdown
Author

Hi @Vonng! Recently, I found out that per-bucket CORS policies are not available in the MinIO Community Edition, but are provided in the paid MinIO AIStor. I need this feature, so I decided to implement it in Silo.

Could you please let me know if there are any licensing, ideological, or technical concerns that could prevent this PR from being accepted? Right now, I also don't fully understand what the rebrand-guard is intended to protect against.

If there is a chance that this PR could be merged, I'm also planning to add bucket-level CORS policy settings to the Silo Console UI.

@Vonng Vonng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing this. Per-bucket CORS addresses a real S3 compatibility gap and is useful for browser clients that need bucket-specific policies.

I took another pass focused strictly on functional behavior and verified the PR against current main. The core single-site implementation works end to end with a standard minio-go client:

  • PUT, GET, and DELETE the bucket CORS configuration
  • accept a matching preflight and reject a non-matching origin
  • apply the configured CORS headers to an actual object response
  • retain the existing global fallback for buckets without a configuration

The overall implementation approach is sound: standard ?cors APIs, bucket-metadata persistence, and per-bucket evaluation ahead of the existing global fallback. Local build, targeted normal/race tests, lint, and generated-file checks all pass. GitHub does not currently report PR CI checks, so CI remains a separate merge gate.

I see one material integration item to finish before merge:

  • Site replication sends SRBucketMetaTypeCorsConfig, but the peer receive/apply path and the initial sync/heal/status paths do not yet carry the new CORS metadata. This can leave CORS configuration different between replicated sites even though the originating request succeeds. Please complete those paths and add focused tests. Alternatively, if site-replicated deployments are intentionally outside this PR's scope, please state that limitation clearly and avoid claiming site-replication parity.

A few protocol-hardening items can be handled here or as focused follow-ups: validate the supplied Content-MD5/checksum using the existing request helper, tighten wildcard and ID validation to the S3 constraints, and complete cache-variation headers for preflight responses. These do not change my positive assessment of the main feature.

Once the site-replication scope is resolved and CI is green, I would be happy to re-review. The Console UI can remain a separate follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants