Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/MainDistributionPipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.5-variegata
with:
extension_name: aws
duckdb_version: v1.5.3
duckdb_version: v1.5.4
ci_tools_version: v1.5-variegata
exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_rtools;windows_amd64_mingw' # Doesn't work anyway: env local file or env access possible

Expand All @@ -28,7 +28,7 @@ jobs:
secrets: inherit
with:
extension_name: aws
duckdb_version: v1.5.3
duckdb_version: v1.5.4
ci_tools_version: v1.5-variegata
exclude_archs: 'wasm_mvp;wasm_eh;wasm_threads;windows_amd64_rtools;windows_amd64_mingw' # Doesn't work anyway: env local file or env access possible
deploy_latest: ${{ startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main' }}
Expand Down
23 changes: 17 additions & 6 deletions src/aws_secret.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "duckdb/common/case_insensitive_map.hpp"
#include "duckdb/common/exception.hpp"
#include "duckdb/main/database.hpp"
#include "duckdb/main/extension/extension_loader.hpp"

#include <aws/core/Aws.h>
Expand Down Expand Up @@ -43,6 +44,12 @@ static struct {
const string default_ = "exists";
} CreateAwsSecretValidation;

//! Note: this is a partial duplication of duckdb's GetDefaultUserAgent()
//! It needs to be reimplemented since the format is not compatible
static string DuckDBAwsUserAgentAppId() {
return "duckdb/" + string(DuckDB::LibraryVersion());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use GetDefaultUserAgent()? link

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That is a non-static method on the config, so it would need config to be wired in the callstack. I think that might be appropriate with a rework, but for a bug fix this is I think the proper solution.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now I got the question, we can't use DBConfig::UserAgent, see above, and we could use GetDefaultUserAgent but: requires changes duckdb side, and more importantly '(' and ')' are not whitelisted, so depending on details they might become rendered as --, and then we can as well just skip them, and solve this properly.

}

//! Build a ClientConfiguration with the detected CA file path applied (if any).
//! This is required because libcurl is statically linked from a RHEL-based
//! manylinux image, so its default CA path is /etc/pki/tls/certs/ca-bundle.crt,
Expand All @@ -55,6 +62,12 @@ static Aws::Client::ClientConfiguration BuildClientConfigWithCa() {
if (!SELECTED_CURL_CERT_PATH.empty()) {
cfg.caFile = SELECTED_CURL_CERT_PATH;
}
// Identify DuckDB in the User-Agent, unless the user has already configured an app
// id (via AWS_SDK_UA_APP_ID or the sdk_ua_app_id profile key, which the
// ClientConfiguration constructor loads into cfg.appId): their value wins.
if (cfg.appId.empty()) {
cfg.appId = DuckDBAwsUserAgentAppId();
}
return cfg;
}

Expand Down Expand Up @@ -137,12 +150,10 @@ class DuckDBCustomAWSCredentialsProviderChain : public Aws::Auth::AWSCredentials
aws_profile.SetExternalId(external_id);
AddSTSProvider(aws_profile);
} else if (item == "sso") {
// Pass an explicit ClientConfiguration so the SSO portal HTTPS call
// uses the detected CA path. See BuildClientConfigWithCa() comment.
auto sso_config = Aws::MakeShared<Aws::Client::ClientConfiguration>("DuckDBAwsSSO");
if (!SELECTED_CURL_CERT_PATH.empty()) {
sso_config->caFile = SELECTED_CURL_CERT_PATH;
}
// Pass an explicit ClientConfiguration so the SSO portal HTTPS call uses
// the detected CA path and the DuckDB app id. See BuildClientConfigWithCa().
auto sso_config = Aws::MakeShared<Aws::Client::ClientConfiguration>("DuckDBAwsSSO",
BuildClientConfigWithCa());
if (profile.empty()) {
AddProvider(std::make_shared<Aws::Auth::SSOCredentialsProvider>(Aws::String(), sso_config));
} else {
Expand Down
Loading