Skip to content

refactor: Move SensitiveBytes and Credential into shared module - #2971

Open
DerGut wants to merge 5 commits into
apache:mainfrom
DerGut:sensitive-string
Open

refactor: Move SensitiveBytes and Credential into shared module#2971
DerGut wants to merge 5 commits into
apache:mainfrom
DerGut:sensitive-string

Conversation

@DerGut

@DerGut DerGut commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Fixes #2972

This is derived from a thread in #2836 (comment) and from @plusplusjiajia's original implementation on #2838. There's three issues I see with the freshly introduced iceberg::catalog::session::Credential type:

  1. the naming of Credential already exists in the context of credential vending (see iceberg_catalog_rest::StorageCredential - but in Java it's plain Credential). A new Credential concept meaning "any sensitive string" conflicts with it
  2. use cases exist for a sensitive string type outside of catalog credentials, like FileIO properties (e.g. s3.session-token): iceberg::catalog is the wrong place to share it
  3. The existing (and analogous) iceberg::encryption::SensitiveBytes type is located somewhere completely different, making it non-obvious that both exists and share semantics

What changes are included in this PR?

This PR moves the recently introduced (and unreleased) Credential type to a separate iceberg::sensitive module and colocates it with the existing iceberg::encryption::SensitiveBytes.

Are these changes tested?

I added two new tests to assert the redaction behavior of the SensitiveBytes type and a third test to assert that imports of SensitiveBytes by its old location remain backwards-compatible.

AI Disclosure

I used AI to rubber-duck approaches and rebase my PR onto latest main.

DerGut added 4 commits August 7, 2026 02:16
Signed-off-by: Jannik Steinmann <jannik.steinmann@datadoghq.com>
Signed-off-by: Jannik Steinmann <jannik.steinmann@datadoghq.com>
Signed-off-by: Jannik Steinmann <jannik.steinmann@datadoghq.com>
Signed-off-by: Jannik Steinmann <jannik.steinmann@datadoghq.com>
@DerGut
DerGut force-pushed the sensitive-string branch from b706d0b to bcde3dc Compare August 7, 2026 00:17
Signed-off-by: Jannik Steinmann <jannik.steinmann@datadoghq.com>

Re-generate public API
@DerGut
DerGut force-pushed the sensitive-string branch from bcde3dc to 509cf21 Compare August 7, 2026 10:11
@DerGut
DerGut marked this pull request as ready for review August 7, 2026 10:58
@DerGut DerGut mentioned this pull request Aug 7, 2026

@xanderbailey xanderbailey 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.

This refactor makes sense to me and the import for sensitive bytes is maintained from the encryption mod so I'm happy with this change! Thanks!

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.

Credential refactor

2 participants