Skip to content

Wire role_arn/role_session_name into Keeper S3 snapshot client#104140

Merged
alexey-milovidov merged 2 commits intomasterfrom
keeper-s3-snapshot-assume-role
May 7, 2026
Merged

Wire role_arn/role_session_name into Keeper S3 snapshot client#104140
alexey-milovidov merged 2 commits intomasterfrom
keeper-s3-snapshot-assume-role

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

Forward role_arn and role_session_name from the auth settings into the CredentialsConfiguration used when constructing the S3 client for Keeper snapshots. The AssumeRole credentials provider chain (AwsAuthSTSAssumeRoleCredentialsProvider) and the S3AuthSettings fields already exist in the public repo — only this call site in KeeperSnapshotManagerS3 was missed when the broader S3 AssumeRole infrastructure was synced from clickhouse-private. Without this, Keeper snapshot uploads cannot use STS-based authentication even though the rest of the server can.

The matching change in clickhouse-private is part of https://github.com/ClickHouse/clickhouse-private/pull/2819.

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Honor role_arn and role_session_name auth settings in the Keeper S3 snapshot client, allowing snapshot uploads to use STS AssumeRole-based authentication.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Pass `role_arn` and `role_session_name` from the auth settings into the
`CredentialsConfiguration` used to build the S3 client for Keeper
snapshots. The fields and credentials provider chain already exist in
the public repo (`AwsAuthSTSAssumeRoleCredentialsProvider`); only this
call site in `KeeperSnapshotManagerS3` was missed. The matching change
landed in clickhouse-private master in PR
ClickHouse/clickhouse-private#2819.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented May 5, 2026

Workflow [PR], commit [2541838]

Summary:
Keeper Stress Tests (PR): Grafana: Run details (this run), Grafana: 1vs1 Comparison, Grafana: Historical progression


AI Review

Summary

This PR forwards role_arn and role_session_name into the S3::CredentialsConfiguration used by KeeperSnapshotManagerS3, so Keeper snapshot uploads can use STS AssumeRole credentials like other S3 paths. The change is small, targeted, and consistent with existing S3 client wiring, with no correctness or safety issues identified.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
No large/binary files
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label May 5, 2026
@antonio2368 antonio2368 self-assigned this May 6, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented May 6, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.10% +0.00%
Functions 91.10% 91.10% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 100.00% (8/8) · Uncovered code

Full report · Diff report

@alexey-milovidov alexey-milovidov added this pull request to the merge queue May 7, 2026
Merged via the queue into master with commit c93903f May 7, 2026
165 checks passed
@alexey-milovidov alexey-milovidov deleted the keeper-s3-snapshot-assume-role branch May 7, 2026 21:19
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants