Skip to content

feat: optional external_id for S3 role-based access - #698

Merged
koletzilla merged 3 commits into
ClickHouse:mainfrom
jessedobbelaere:feat/s3-external-id
Aug 7, 2026
Merged

feat: optional external_id for S3 role-based access#698
koletzilla merged 3 commits into
ClickHouse:mainfrom
jessedobbelaere:feat/s3-external-id

Conversation

@jessedobbelaere

@jessedobbelaere jessedobbelaere commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

ClickHouse Cloud accepts an external_id alongside role_arn in extra_credentials(...) and forwards it as the ExternalId parameter of the AWS STS AssumeRole call. This is something I explicitly requested to have a year ago with Clickhouse Cloud, and i noticed it has been introduced somewhere in the past year because I noticed it in the docs:

That lets the assumed role's trust policy require a shared secret through an sts:ExternalId condition, which mitigates the confused deputy problem and satisfies flagged cross-account trust policies with no external ID requirement.

clickhouse_s3source() had no way to pass it, so any project using the macro was limited to just the role_arn trust policy. Add an optional external_id argument, resolvable either per model or from the vars S3 configuration, and emit it only when a role_arn is present. Specifying external_id on its own raises, mirroring the existing validation for a half-specified access key pair.

The new argument is optional so there should not be any risk in merging this.

-- macro argument
select * from {{ clickhouse_s3source('taxi_s3', role_arn='arn:aws:iam::111111111111:role/ClickHouseAccessRole-001', external_id='my-external-id') }}
# or in vars, next to role_arn
vars:
  taxi_s3:
    bucket: 'https://my-bucket.s3.eu-west-3.amazonaws.com/data/'
    fmt: 'Parquet'
    role_arn: 'arn:aws:iam::111111111111:role/ClickHouseAccessRole-001'
    external_id: 'my-external-id'

Both compile to:

s3('https://my-bucket.s3.eu-west-3.amazonaws.com/data/', 'Parquet', extra_credentials(role_arn='arn:aws:iam::111111111111:role/ClickHouseAccessRole-001', external_id='my-external-id'))

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@CLAassistant

CLAassistant commented Aug 3, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

ClickHouse Cloud accepts an `external_id` alongside `role_arn` in
`extra_credentials(...)` and forwards it as the `ExternalId` parameter of
the AWS STS `AssumeRole` call. That lets the assumed role's trust policy
require a shared secret through an `sts:ExternalId` condition, which
mitigates the confused deputy problem and satisfies cloud security
scanners that flag cross-account trust policies with no external ID or
MFA requirement.

`clickhouse_s3source()` had no way to pass it, so any project using the
macro was limited to a bare `role_arn` trust policy. Add an optional
`external_id` argument, resolvable either per model or from the `vars`
S3 configuration, and emit it only when a `role_arn` is present.
Specifying `external_id` on its own raises, mirroring the existing
validation for a half-specified access key pair.

The new argument is appended to both signatures so existing positional
callers keep working.
* upstream/main:
  fix: quote S3 compression argument correctly (ClickHouse#695)

# Conflicts:
#	tests/unit/test_s3source_clause.py
@koletzilla
koletzilla self-requested a review August 7, 2026 14:40

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

Thanks for the PR!

@koletzilla
koletzilla merged commit c5c810b into ClickHouse:main Aug 7, 2026
14 of 15 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.

3 participants