Skip to content

fix: include per-bucket CORS in bucket metadata export and import - #127

Merged
Vonng merged 1 commit into
mainfrom
fix/issue-112-bucket-metadata-cors
Sep 6, 2026
Merged

fix: include per-bucket CORS in bucket metadata export and import#127
Vonng merged 1 commit into
mainfrom
fix/issue-112-bucket-metadata-cors

Conversation

@Vonng

@Vonng Vonng commented Sep 5, 2026

Copy link
Copy Markdown
Member

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 carries a DCO Signed-off-by trailer.

Description

Per-bucket CORS was absent from the admin bucket-metadata export/import: export emitted no cors.xml, import ignored a cors.xml entry, and both reported success. This adds cors.xml to the export list (the stored document verbatim), an import case that validates the document with the same parser and validator as PutBucketCors and reports per-file status in BucketStatus.Cors, the merge into the bucket metadata under the existing lock with a timestamp clamped above bucket creation and the previous CORS timestamp (localCORSUpdatedAt, extracted from the local PutBucketCors path so both stay identical), and the dedicated CORS site-replication event after the save. Two corrections required by the adversarial review: the import reads one byte past the declared entry size so archive/zip verifies the entry CRC (a CRC-corrupted but well-formed document was otherwise stored), and the CORS event is sent even when the shared bucket-metadata hook failed, with both errors kept via errors.Join.

Fixes #112. Fix plan agreed with the Codex reviewer: #112 (comment). Adversarial Codex review of this implementation: round 1 FAIL (two MAJOR: zip checksum, suppressed CORS hook), round 2 FAIL (one required MINOR: partial-peer-failure test; one NIT), round 3 PASS with no findings.

Motivation and Context

Bucket backup and migration were incomplete for the newly supported configuration while reporting success. Integration omission of the per-bucket CORS feature (ce4525632, ff3395d3c); not an upstream inheritance. No mcli or madmin-go change is needed: madmin.BucketStatus already defines Cors and mcli copies the server-produced ZIP verbatim.

How to test this PR?

GOWORK=off go test -tags kqueue,dev ./cmd -count=1 -v -run '^(TestAdminBucketMetadataCORSRoundTrip|TestAdminBucketMetadataCORSImportReplicat.*)$'

TestAdminBucketMetadataCORSRoundTrip (ErasureSD and Erasure): export contains the stored bytes; re-export after deletion omits cors.xml; import restores the document with a timestamp after bucket creation; an archive without cors.xml leaves CORS alone; a bucket the import itself creates also lands above its creation time; a malformed document is reported in BucketStatus.Cors.Err with the stored configuration unchanged. The added replication test drives both hooks through HTTP stubs and asserts the healthy peer receives the saved CORS payload and timestamp even when the shared hook fails, and that both failures are reported. Red on main (export emits no cors.xml; import stores nothing), green with the fix. Counter-checks, each run and reverted: dropping the timestamp clamp fails the fresh-bucket assertion (CORS stamped before creation); dropping the extra-byte read accepts a CRC-corrupted document; restoring the err == nil condition starves the healthy peer of the CORS event. The plan's 58-test CORS and bucket-metadata selector passes; gofmt clean, go vet clean, golangci-lint 0 issues.

Compatibility impact

The admin API wire format is unchanged; the ZIP gains one entry. Archives taken before the fix stay importable and leave CORS untouched, as every other absent config does. A restored bucket announces its CORS through the existing SRBucketMetaTypeCorsConfig event; a peer that predates per-bucket CORS ignores the unknown field. No data or on-disk change; no resync needed.

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 (integration omission of a SILO feature, not a regression)
  • Unit tests added/updated
  • make verifiers equivalent: gofmt clean, go vet clean, golangci-lint 0 issues on ./cmd/...
  • Relevant package tests pass
  • Compatibility and rollback impact documented
  • Internal documentation updated (code comments)
  • Public documentation update opened in pgsty/silo.pgsty.com (none needed; the migration guides do not enumerate config types)

🤖 Generated with Claude Code

https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe

The admin bucket-metadata handlers enumerate every bucket config by name, and
per-bucket CORS was never added to that enumeration: export omitted cors.xml
(cmd/admin-bucket-handlers.go:414 cfgFiles) and import ignored the entry
outright, with no case in applyImportedBucketMetadata (:598) or SetStatus
(:629), so a CORS-only archive reported 0/0 buckets imported and a restored
bucket silently lost its configuration. Export now writes the stored document
verbatim and import validates it with the same parser and validator as
PutBucketCorsHandler, merging it under the existing bucket metadata lock and
announcing it through the dedicated SRBucketMetaTypeCorsConfig event; the local
CORS timestamp rule is extracted into localCORSUpdatedAt and reused so an
imported document always lands strictly above bucket creation, which matters
because the import stamps its fields before creating any missing bucket and a
CORS event below Created is dropped as an older bucket incarnation.

The import reads one byte past the declared entry size so archive/zip reaches
EOF and verifies the entry checksum, otherwise a corrupt or over-long entry
carrying well formed XML would overwrite the stored document; and the CORS
event is sent even when the shared bucket metadata hook failed, so an
unreachable peer cannot withhold an already committed CORS document from the
reachable ones.

Tests: TestAdminBucketMetadataCORSRoundTrip and
TestAdminBucketMetadataCORSImportReplicatesPastPeerFailure (new, ErasureSD and
Erasure).
Compatibility: the ZIP gains one entry, older archives stay importable and
leave CORS untouched; no mcli or madmin-go change is needed because
madmin.BucketStatus already carries Cors and mcli copies the export ZIP
verbatim.

Fixes #112

Signed-off-by: Feng Ruohang <rh@vonng.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe
@Vonng
Vonng merged commit 65d4806 into main Sep 6, 2026
8 checks passed
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.

[P2] Bucket metadata export/import silently loses per-bucket CORS configuration

1 participant