Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for secrets #23

Merged
merged 12 commits into from
Jan 2, 2024
Merged

Conversation

samansmink
Copy link
Collaborator

@samansmink samansmink commented Dec 22, 2023

This PR adds support for a new concept in duckdb: secrets (duckdb/duckdb#10042)

To summarize, secrets are, in the most generic sense, scoped sets of configuration that provide the necessary information to do something. In most cases this "something" will be an authorized request to some API.

For the AWS extension this means the what was previously configured through duckdb settings, will now be done through secrets. Check out the aforementioned PR in duckdb on what benefits this brings. This means that the load_aws_credentials() function call is now implemented as a Secret Provider called credential_chain.

credential_chain secret provider overview

The credential_chain provider uses the AWS SDK to create an S3 secret. This is done in exactly the same way as the load_aws_credentials function now does, except instead of setting the duckdb_settings, it creates a secret.

The easiest way to use it is:

CREATE SECRET (
    TYPE S3,
    PROVIDER credential_chain
)

This will create an S3 secret that basically does everything automatically, just like load_aws_credentials.

Note that all of the fields that you can normally set for s3 secrets are also implemented, including the ones set automatically. For example to automatically load the config, override the loaded region, and disable ssl:

CREATE SECRET (
    TYPE S3,
    PROVIDER credential_chain,
    REGION 'eu-west-1',
    USE_SSL false
)

Small feature addition

While implementing, I added a small feature similar to what already existed in the azure extension: the ability to specify the credential chain used. This means that you can change (the order in) which different places are searched for credentials by the aws sdk. For example, to search for credentials firstly in the config, and then the env:

CREATE SECRET (
    TYPE S3,
    PROVIDER credential_chain,
    CHAIN 'config;env'
)

This feature fixes #16

Also this should fix issue #14 and #10 by providing a way to pass the profile to the sso chain:

CREATE SECRET (
    TYPE S3,
    PROVIDER credential_chain,
    CHAIN 'sso',
    PROFILE 'my-profile'
)

Finally, the credential chain provider can take various arguments to configure the several other aws sdk providers, however, these are currently not yet tested properly. (see test/sql/aws_secret_chains.test for more info)

@samansmink samansmink changed the title Switch to secrets Add support for secrets Dec 22, 2023
@carlopi
Copy link
Contributor

carlopi commented Dec 25, 2023

Idea: would it make sense have load_aws_credentials also invoke CREATE SECRET ( TYPE S3, PROVIDER credential_chain ) behind the scenes?

So the idea would be that users that want default behaviours can keep using load_aws_credentials and that will keep working also post deprecations of the s3_* settings.

And users that will want more control can move to use explicit CREATE SECRET (or potentially yet to be introduced overloads for load_aws_credentials).

Obv, this can also added later after landing this PR.

@samansmink
Copy link
Collaborator Author

would it make sense have load_aws_credentials also invoke CREATE SECRET ( TYPE S3, PROVIDER credential_chain ) behind the scenes

Yea good point, I've considered this, but i think there's not really a point. If we're already deprecating the s3_* variables, why keep this legacy function around while it doesn't really add any functionality. I would say we deprecate it along with the s3_* settings to keep things uniform.

@Mytherin Mytherin merged commit bdee1e8 into duckdb:main Jan 2, 2024
16 checks passed
@Mytherin
Copy link
Contributor

Mytherin commented Jan 2, 2024

Thanks! LGTM.

A related question - previously this extension would be auto-loaded when CALL load_aws_credentials() was called. Is the extension now autoloaded when a credential_chain secret is created for S3? If not, perhaps it makes sense to add that behavior as well?

@samansmink
Copy link
Collaborator Author

@Mytherin Yea it is! We can autoload on secret load and creation https://github.com/duckdb/duckdb/blob/1e216074028c0aaef838ae2075e2e30705aa48e7/src/main/secret/secret_manager.cpp#L196

@Mytherin
Copy link
Contributor

Mytherin commented Jan 2, 2024

Cool, very nice!

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.

Support AWS_WEB_IDENTITY_TOKEN_FILE as credential provider
3 participants