Skip to content

SS-528 Fix Iceberg Catalog ALTER CONNECTION panic - #40069

Merged
patrickwwbutler merged 3 commits into
mainfrom
patrick/iceberg-alter-conn
Oct 22, 2026
Merged

patrickwwbutler merged 3 commits into
mainfrom
patrick/iceberg-alter-conn

Conversation

@patrickwwbutler

@patrickwwbutler patrickwwbutler commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

This fixes SS-528 by properly implementing alter_compatible for various Iceberg Catalog and Iceberg Catalog Connection types, which prevent the ALTER CONNECTION statement from panicking when run on any of these types, or on parent S3 or GCP connections used for s3tables or biglake catalogs.

Added a testdrive to confirm the fix.

Also 2 driveby changes, one renaming some variables in alter_export_connections to better match reality, and adding some additional connection cases to the alter_compatible match statement.

@patrickwwbutler
patrickwwbutler requested a review from a team as a code owner October 21, 2026 18:29
@linear-code

linear-code Bot commented Oct 21, 2026

Copy link
Copy Markdown

SS-528

@peterdukelarsen peterdukelarsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please double check whether this risks logging anything we shouldn't

Comment thread src/storage-types/src/sinks.rs Resolved
Comment thread src/storage-types/src/connections.rs Resolved
Comment thread src/storage-types/src/connections.rs Resolved
@patrickwwbutler
patrickwwbutler merged commit 559b313 into main Oct 22, 2026
85 checks passed
@patrickwwbutler
patrickwwbutler deleted the patrick/iceberg-alter-conn branch October 22, 2026 14:47
peterdukelarsen added a commit that referenced this pull request Oct 22, 2026
…40101)

`StringOrSecret` holds OAuth credentials and AWS session tokens as
inline strings whenever the user did not wrap them in a `SECRET`. Its
derived `Debug` printed them verbatim, and connection structs are dumped
with `{:#?}` on every `ALTER CONNECTION` compatibility failure and on
every `RunSink` the cluster reconciles.

This moves the redaction into `StringOrSecret`'s own `Debug`, so an
inline value prints as `String(<redacted>)` at every dump site, present
and future. Secret references still print as `Secret(id)`. The per-field
`redacted()` view and the hand-written `Debug` impls on
`IcebergCatalogAuth` and `AwsCredentials` from #40069 are removed, since
the type now covers them.

Usernames, access key IDs and TLS certificates share this type and are
redacted too. The `at {field}` compatibility logs still name which field
differed.

Adds a unit test that checks both `{:?}` and `{:#?}` output.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <[email protected]>
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