Skip to content

fix: unify all CreateSecret paths under single mutex to prevent write-write conflict #1113

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.

Sign up for GitHub

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

Closed
HaoXuAI wants to merge 3 commits into from
Closed

fix: unify all CreateSecret paths under single mutex to prevent write-write conflict #1113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d93ddf
Add credential auto-refresh support for web_identity/STS chains
HaoXuAI Jun 10, 2026
ced07a0
Add Rio CI pipeline for building iceberg extension
HaoXuAI Jun 10, 2026
e70a73c
fix: unify all CreateSecret paths under single mutex to prevent write…
HaoXuAI Jun 25, 2026
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
204 changes: 204 additions & 0 deletions rio.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
schemaVersion: 2.0

machine:
env: &DEFAULT_ENV
COMMIT: "$(git rev-parse --short HEAD)"
PUBLISH_TIME: '$(/usr/bin/env date +"%Y%m%d%H%M%S")'
GEN: ninja
VCPKG_COMMIT: "84bab45d415d22042bd0b9081aea57f362da3f35"
DUCKDB_COMMIT: "14eca11bd9d4a0de2ea0f078be588a9c1c5b279c"
CI_TOOLS_COMMIT: "4b3b37b0c9de00da54e1765d65abfea3f94617f4"

pipelines:
# Build DuckDB Iceberg Extension (linux_amd64)
- group: duckdb-iceberg-extension-build
branchName: feature/credential-auto-refresh-v1.5
trigger:
gitPush: true
machine:
env:
<<: *DEFAULT_ENV
baseImage: quay.io/pypa/manylinux_2_28_x86_64
build:
template: freestyle:v4:publish
steps:
# Install build dependencies
- yum groupinstall -y "Development Tools"
- yum install -y curl zip unzip tar ninja-build perl-IPC-Cmd perl-core ccache wget git jq

# Install CMake 4.x
- wget -q https://github.com/Kitware/CMake/releases/download/v4.0.2/cmake-4.0.2-linux-x86_64.sh
- chmod +x cmake-4.0.2-linux-x86_64.sh && ./cmake-4.0.2-linux-x86_64.sh --skip-license --prefix=/usr/local
- cmake --version

# Setup vcpkg
- git clone https://github.com/microsoft/vcpkg.git /vcpkg
- (cd /vcpkg && git checkout ${VCPKG_COMMIT} && ./bootstrap-vcpkg.sh)
- export VCPKG_ROOT=/vcpkg
- export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake

# Allow git operations
- git config --global --add safe.directory "*"

# Initialize submodules (fallback: clone explicitly if Rio didn't check them out)
- git submodule update --init --recursive || true
- |
if [ ! -f duckdb/CMakeLists.txt ]; then
echo "Submodule 'duckdb' missing, cloning explicitly..."
rm -rf duckdb
git clone --depth 1 https://github.com/duckdb/duckdb.git duckdb
(cd duckdb && git fetch --depth 1 origin ${DUCKDB_COMMIT} && git checkout ${DUCKDB_COMMIT})
fi
- |
if [ ! -f extension-ci-tools/makefiles/duckdb_extension.Makefile ]; then
echo "Submodule 'extension-ci-tools' missing, cloning explicitly..."
rm -rf extension-ci-tools
git clone --depth 1 https://github.com/duckdb/extension-ci-tools.git extension-ci-tools
(cd extension-ci-tools && git fetch --depth 1 origin ${CI_TOOLS_COMMIT} && git checkout ${CI_TOOLS_COMMIT})
fi

# Verify
- ls duckdb/CMakeLists.txt extension-ci-tools/makefiles/duckdb_extension.Makefile
- echo "PWD=$(pwd)" && ls Makefile CMakeLists.txt extension_config.cmake 2>&1 || true

# Build extension directly with cmake (Makefile may not be present in Rio checkout)
- |
export VCPKG_ROOT=/vcpkg
export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake
mkdir -p build/release
cmake -G Ninja \
-DEXTENSION_STATIC_BUILD=1 \
-DDUCKDB_EXTENSION_CONFIGS="$(pwd)/extension_config.cmake" \
-DVCPKG_BUILD=1 \
-DCMAKE_TOOLCHAIN_FILE=${VCPKG_TOOLCHAIN_PATH} \
-DVCPKG_MANIFEST_DIR="$(pwd)" \
-DCMAKE_BUILD_TYPE=Release \
-S ./duckdb/ \
-B build/release
cmake --build build/release --config Release

# Stage the built extension artifact
- export EXT_VERSION="v1.5.3-$(git rev-parse --short HEAD)"
- mkdir -p .out
- cp build/release/extension/iceberg/iceberg.duckdb_extension .out/iceberg.duckdb_extension
- echo "${EXT_VERSION}" > .out/VERSION
- ci stage-lib --label extension ".out/iceberg.duckdb_extension,com/apple/maps/data/infra/duckdb/iceberg-extension/${EXT_VERSION}/"
- ci stage-lib --label extension ".out/VERSION,com/apple/maps/data/infra/duckdb/iceberg-extension/${EXT_VERSION}/"
test:
sidecar: true
package:
release: false
freeform:
- publish:
- packageFilters: [extension]
repo: "m2:maps-libs-public"

# Release build (manually triggered)
- group: duckdb-iceberg-extension-release
branchRules:
includePatterns:
- "release/iceberg-extension/.*"
trigger:
gitPush: false
machine:
env:
<<: *DEFAULT_ENV
baseImage: quay.io/pypa/manylinux_2_28_x86_64
build:
template: freestyle:v4:publish
steps:
- yum groupinstall -y "Development Tools"
- yum install -y curl zip unzip tar ninja-build perl-IPC-Cmd perl-core ccache wget git jq
- wget -q https://github.com/Kitware/CMake/releases/download/v4.0.2/cmake-4.0.2-linux-x86_64.sh
- chmod +x cmake-4.0.2-linux-x86_64.sh && ./cmake-4.0.2-linux-x86_64.sh --skip-license --prefix=/usr/local
- git clone https://github.com/microsoft/vcpkg.git /vcpkg
- (cd /vcpkg && git checkout ${VCPKG_COMMIT} && ./bootstrap-vcpkg.sh)
- export VCPKG_ROOT=/vcpkg
- export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake
- git config --global --add safe.directory "*"
- git submodule update --init --recursive || true
- |
if [ ! -f duckdb/CMakeLists.txt ]; then
rm -rf duckdb && git clone --depth 1 https://github.com/duckdb/duckdb.git duckdb
(cd duckdb && git fetch --depth 1 origin ${DUCKDB_COMMIT} && git checkout ${DUCKDB_COMMIT})
fi
- |
if [ ! -f extension-ci-tools/makefiles/duckdb_extension.Makefile ]; then
rm -rf extension-ci-tools && git clone --depth 1 https://github.com/duckdb/extension-ci-tools.git extension-ci-tools
(cd extension-ci-tools && git fetch --depth 1 origin ${CI_TOOLS_COMMIT} && git checkout ${CI_TOOLS_COMMIT})
fi
- |
export VCPKG_ROOT=/vcpkg
export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake
mkdir -p build/release
cmake -G Ninja \
-DEXTENSION_STATIC_BUILD=1 \
-DDUCKDB_EXTENSION_CONFIGS="$(pwd)/extension_config.cmake" \
-DVCPKG_BUILD=1 \
-DCMAKE_TOOLCHAIN_FILE=${VCPKG_TOOLCHAIN_PATH} \
-DVCPKG_MANIFEST_DIR="$(pwd)" \
-DCMAKE_BUILD_TYPE=Release \
-S ./duckdb/ \
-B build/release
cmake --build build/release --config Release
- export EXT_VERSION="v1.5.3-$(git rev-parse --short HEAD)"
- mkdir -p .out
- cp build/release/extension/iceberg/iceberg.duckdb_extension .out/iceberg.duckdb_extension
- echo "${EXT_VERSION}" > .out/VERSION
- ci stage-lib --label extension ".out/iceberg.duckdb_extension,com/apple/maps/data/infra/duckdb/iceberg-extension/${EXT_VERSION}/"
- ci stage-lib --label extension ".out/VERSION,com/apple/maps/data/infra/duckdb/iceberg-extension/${EXT_VERSION}/"
test:
sidecar: true
package:
release: true
freeform:
- publish:
- packageFilters: [extension]
repo: "m2:maps-libs-public"

# Pull Request Build (compile check only)
- group: pull-request-build
branchName: main
build:
template: freestyle:v4:prb
steps:
- yum groupinstall -y "Development Tools"
- yum install -y curl zip unzip tar ninja-build perl-IPC-Cmd perl-core ccache wget git jq
- wget -q https://github.com/Kitware/CMake/releases/download/v4.0.2/cmake-4.0.2-linux-x86_64.sh
- chmod +x cmake-4.0.2-linux-x86_64.sh && ./cmake-4.0.2-linux-x86_64.sh --skip-license --prefix=/usr/local
- git clone https://github.com/microsoft/vcpkg.git /vcpkg
- (cd /vcpkg && git checkout ${VCPKG_COMMIT} && ./bootstrap-vcpkg.sh)
- export VCPKG_ROOT=/vcpkg
- export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake
- git config --global --add safe.directory "*"
- git submodule update --init --recursive || true
- |
if [ ! -f duckdb/CMakeLists.txt ]; then
rm -rf duckdb && git clone --depth 1 https://github.com/duckdb/duckdb.git duckdb
(cd duckdb && git fetch --depth 1 origin ${DUCKDB_COMMIT} && git checkout ${DUCKDB_COMMIT})
fi
- |
if [ ! -f extension-ci-tools/makefiles/duckdb_extension.Makefile ]; then
rm -rf extension-ci-tools && git clone --depth 1 https://github.com/duckdb/extension-ci-tools.git extension-ci-tools
(cd extension-ci-tools && git fetch --depth 1 origin ${CI_TOOLS_COMMIT} && git checkout ${CI_TOOLS_COMMIT})
fi
- |
export VCPKG_ROOT=/vcpkg
export VCPKG_TOOLCHAIN_PATH=/vcpkg/scripts/buildsystems/vcpkg.cmake
mkdir -p build/release
cmake -G Ninja \
-DEXTENSION_STATIC_BUILD=1 \
-DDUCKDB_EXTENSION_CONFIGS="$(pwd)/extension_config.cmake" \
-DVCPKG_BUILD=1 \
-DCMAKE_TOOLCHAIN_FILE=${VCPKG_TOOLCHAIN_PATH} \
-DVCPKG_MANIFEST_DIR="$(pwd)" \
-DCMAKE_BUILD_TYPE=Release \
-S ./duckdb/ \
-B build/release
cmake --build build/release --config Release
test:
sidecar: true
machine:
env:
<<: *DEFAULT_ENV
baseImage: quay.io/pypa/manylinux_2_28_x86_64
84 changes: 76 additions & 8 deletions src/catalog/rest/catalog_entry/table/iceberg_table_information.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,74 @@
#include "core/expression/iceberg_transform.hpp"
#include "duckdb/parser/column_definition.hpp"

#include <chrono>
#include <climits>
#include <mutex>

namespace duckdb {

//! Attempt to refresh the catalog's storage secret if it supports auto-refresh.
//! Uses SIGV4Authorization's shared mutex to prevent write-write conflicts with
//! MaybeRefreshSecret (both call CreateSecret on the same aws_secret).
static bool TryRefreshStorageSecret(ClientContext &context, const SecretEntry &secret_entry) {
auto &refresh_mutex = SIGV4Authorization::GetRefreshMutex();
auto &last_refresh_time = SIGV4Authorization::GetLastRefreshTime();
static constexpr int REFRESH_INTERVAL_SECONDS = 300;

// Fast path: skip if refreshed recently
auto now = std::chrono::steady_clock::now();
auto elapsed = std::chrono::duration_cast<std::chrono::seconds>(now - last_refresh_time).count();
if (elapsed < REFRESH_INTERVAL_SECONDS) {
return false;
}

std::unique_lock<std::mutex> lock(refresh_mutex, std::try_to_lock);
if (!lock.owns_lock()) {
return false;
}

// Double-check after lock
now = std::chrono::steady_clock::now();
elapsed = std::chrono::duration_cast<std::chrono::seconds>(now - last_refresh_time).count();
if (elapsed < REFRESH_INTERVAL_SECONDS) {
return false;
}

const auto &kv_secret = dynamic_cast<const KeyValueSecret &>(*secret_entry.secret);
Value refresh_info;
if (!kv_secret.TryGetValue("refresh_info", refresh_info)) {
last_refresh_time = std::chrono::steady_clock::now();
return false;
}

// Reconstruct a CreateSecretInput from refresh_info — this mirrors what httpfs does
CreateSecretInput refresh_input;
refresh_input.on_conflict = OnCreateConflict::REPLACE_ON_CONFLICT;
refresh_input.persist_type = SecretPersistType::TEMPORARY;
refresh_input.type = kv_secret.GetType();
refresh_input.name = kv_secret.GetName();
refresh_input.provider = kv_secret.GetProvider();
refresh_input.storage_type = secret_entry.storage_mode;
refresh_input.scope = kv_secret.GetScope();

auto child_count = StructType::GetChildCount(refresh_info.type());
auto children = StructValue::GetChildren(refresh_info);
for (idx_t i = 0; i < child_count; i++) {
auto &key = StructType::GetChildName(refresh_info.type(), i);
refresh_input.options[key] = children[i];
}

try {
auto &secret_manager = context.db->GetSecretManager();
(void)secret_manager.CreateSecret(context, refresh_input);
} catch (std::exception &) {
// Refresh failed — continue with existing credentials
}
// Always back off (prevents cascading retries on failure)
last_refresh_time = std::chrono::steady_clock::now();
return true;
}

const string &IcebergTableInformation::BaseFilePath() const {
return table_metadata.location;
}
Expand Down Expand Up @@ -173,17 +237,21 @@ IRCAPITableCredentials IcebergTableInformation::GetVendedCredentials(ClientConte
if (catalog.auth_handler->type == IcebergAuthorizationType::SIGV4) {
auto &sigv4_auth = catalog.auth_handler->Cast<SIGV4Authorization>();
auto catalog_credentials = IcebergCatalog::GetStorageSecret(context, sigv4_auth.secret);
// start with the credentials needed for the catalog and overwrite information contained
// in the vended credentials. We do it this way to maintain the region info from the catalog credentials
// Start with the credentials needed for the catalog and overwrite information contained
// in the vended credentials. We do it this way to maintain the region info from the catalog credentials.
if (catalog_credentials) {
// Attempt to refresh the catalog secret before snapshotting its values.
// This ensures we start with fresh credentials if the secret supports auto-refresh
// (e.g., web_identity or sts credential chains).
TryRefreshStorageSecret(context, *catalog_credentials);
// Re-fetch to get the (potentially) refreshed credentials
catalog_credentials = IcebergCatalog::GetStorageSecret(context, sigv4_auth.secret);

auto kv_secret = dynamic_cast<const KeyValueSecret &>(*catalog_credentials->secret);
for (auto &option : kv_secret.secret_map) {
// Ignore refresh info.
// if the credentials are the same as for the catalog, then refreshing the catalog secret is enough
// otherwise the vended credentials contain their own information for refreshing.
if (option.first != "refresh_info" && option.first != "refresh") {
user_defaults.emplace(option);
}
// Propagate refresh_info so that per-table secrets can be auto-refreshed by httpfs
// when credentials expire during long-running queries.
user_defaults.emplace(option);
}
}
if (!sigv4_auth.sigv4_region.empty()) {
Expand Down
Loading