Skip to content

aws_s3: allow empty string as valid object_canned_acl value#4413

Merged
Jeffail merged 4 commits intomainfrom
jw/fixs3output
May 8, 2026
Merged

aws_s3: allow empty string as valid object_canned_acl value#4413
Jeffail merged 4 commits intomainfrom
jw/fixs3output

Conversation

@josephwoodward
Copy link
Copy Markdown
Contributor

@josephwoodward josephwoodward commented May 7, 2026

PR to fix a regression in #4372

The default was changed to "" in to avoid errors on buckets with ACLs disabled, but the config validation was not updated to accept it, causing output initialisation to fail whenever no ACL was configured.

image

@claude
Copy link
Copy Markdown

claude Bot commented May 7, 2026

Commits
LGTM

Review
Single-line regression fix that allows the empty-string default for object_canned_acl to bypass enum validation, restoring behavior after #4372 changed the default to "". The fix is correct and properly scoped.

LGTM

@josephwoodward josephwoodward changed the title aws_s3: allow empty string as valid object_canned_acl value aws_s3: allow empty string as valid object_canned_acl value May 7, 2026
Jeffail added 2 commits May 8, 2026 09:46
The previous commit fixed the runtime validator to accept "" as a valid
object_canned_acl value, but the schema enum field still listed only the
SDK canned ACL values. As a result, an explicit `object_canned_acl: ""`
in user YAML would trip the schema linter even though the default value
of "" was tolerated. Add "" as a valid option and clarify the field
description so the schema, the runtime validator, and the writer all
agree that an empty value means "do not set an ACL on the upload".
Add unit and integration coverage for object_canned_acl to lock in the
agreement between the schema linter, the runtime validator, and the
writer. The unit tests exercise both the schema lint path and the
config parser for the empty default, explicit empty, valid SDK values,
and an invalid value. The integration tests round-trip a valid canned
ACL through LocalStack and assert that an invalid value is rejected at
stream build time.
@claude
Copy link
Copy Markdown

claude Bot commented May 8, 2026

Commits
LGTM

Review
Small regression fix that aligns the aws_s3 schema enum with the runtime validator so that object_canned_acl: "" (the default since #4372) parses cleanly. Three commits split between runtime validator, schema enum, and tests; messages, scope, and granularity all conform to policy. Unit tests cover the parse/lint paths and the integration test round-trips a valid value through LocalStack while asserting an invalid one is rejected.

LGTM

@claude
Copy link
Copy Markdown

claude Bot commented May 8, 2026

Commits
LGTM

Review
Targeted regression fix: aligns the runtime validator and schema enum so object_canned_acl: "" (the default) is accepted, plus unit and integration coverage.

LGTM

@claude
Copy link
Copy Markdown

claude Bot commented May 8, 2026

Commits
LGTM

Review
Small, focused fix: aligns the aws_s3 runtime validator and schema enum so that the empty-string default for object_canned_acl (used when uploading to buckets with ACLs disabled) is actually accepted. Unit + integration coverage and regenerated docs included.

LGTM

@Jeffail Jeffail merged commit d98ba51 into main May 8, 2026
7 checks passed
@Jeffail Jeffail deleted the jw/fixs3output branch May 8, 2026 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants