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

minor secret manager fix #10600

Merged
merged 1 commit into from Feb 12, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/database.cpp
Expand Up @@ -34,6 +34,7 @@ DBConfig::DBConfig() {
cast_functions = make_uniq<CastFunctionSet>(*this);
index_types = make_uniq<IndexTypeSet>();
error_manager = make_uniq<ErrorManager>();
secret_manager = make_uniq<SecretManager>();
}

DBConfig::DBConfig(bool read_only) : DBConfig::DBConfig() {
Expand Down Expand Up @@ -328,8 +329,6 @@ void DatabaseInstance::Configure(DBConfig &new_config) {
}
if (new_config.secret_manager) {
config.secret_manager = std::move(new_config.secret_manager);
} else {
config.secret_manager = make_uniq<SecretManager>();
}
if (config.options.maximum_memory == (idx_t)-1) {
config.SetDefaultMaxMemory();
Expand Down
13 changes: 10 additions & 3 deletions src/main/secret/secret_manager.cpp
Expand Up @@ -158,6 +158,11 @@ unique_ptr<SecretEntry> SecretManager::RegisterSecretInternal(CatalogTransaction
//! Lookup which backend to store the secret in
auto backend = GetSecretStorage(resolved_storage);
if (!backend) {
if (!config.allow_persistent_secrets &&
(persist_type == SecretPersistType::PERSISTENT || storage == LOCAL_FILE_STORAGE_NAME)) {
throw InvalidInputException("Persistent secrets are disabled. Restart DuckDB and enable persistent secrets "
"through 'SET allow_persistent_secrets=true'");
}
throw InvalidInputException("Secret storage '%s' not found!", resolved_storage);
}

Expand Down Expand Up @@ -503,9 +508,11 @@ void SecretManager::InitializeSecrets(CatalogTransaction transaction) {
// load the tmp storage
LoadSecretStorageInternal(make_uniq<TemporarySecretStorage>(TEMPORARY_STORAGE_NAME, *transaction.db));

// load the persistent storage if enabled
LoadSecretStorageInternal(
make_uniq<LocalFileSecretStorage>(*this, *transaction.db, LOCAL_FILE_STORAGE_NAME, config.secret_path));
if (config.allow_persistent_secrets) {
// load the persistent storage if enabled
LoadSecretStorageInternal(
make_uniq<LocalFileSecretStorage>(*this, *transaction.db, LOCAL_FILE_STORAGE_NAME, config.secret_path));
}

initialized = true;
}
Expand Down
8 changes: 8 additions & 0 deletions test/secrets/test_custom_secret_storage.cpp
Expand Up @@ -118,6 +118,10 @@ TEST_CASE("Test adding a custom secret storage", "[secret][.]") {

REQUIRE_NO_FAIL(con.Query("set allow_persistent_secrets=true;"));

// Set custom secret path to prevent interference with other tests
auto secret_dir = TestCreatePath("custom_secret_storage_cpp_1");
REQUIRE_NO_FAIL(con.Query("set secret_directory='" + secret_dir + "'"));

REQUIRE_NO_FAIL(con.Query("CREATE SECRET s1 IN TEST_STORAGE (TYPE S3, SCOPE 's3://foo')"));
REQUIRE_NO_FAIL(con.Query("CREATE PERSISTENT SECRET s2 IN test_storage (TYPE S3, SCOPE 's3://')"));
REQUIRE_NO_FAIL(con.Query("CREATE TEMPORARY SECRET s2 (TYPE S3, SCOPE 's3://')"));
Expand Down Expand Up @@ -188,6 +192,10 @@ TEST_CASE("Test tie-break behaviour for custom secret storage", "[secret][.]") {

REQUIRE_NO_FAIL(con.Query("set allow_persistent_secrets=true;"));

// Set custom secret path to prevent interference with other tests
auto secret_dir = TestCreatePath("custom_secret_storage_cpp_2");
REQUIRE_NO_FAIL(con.Query("set secret_directory='" + secret_dir + "'"));

// Register custom secret storage
auto &secret_manager = duckdb::SecretManager::Get(*db.instance);

Expand Down
6 changes: 3 additions & 3 deletions test/sql/secrets/create_secret_storage_backends.test
Expand Up @@ -16,18 +16,18 @@ set allow_persistent_secrets=false;
statement error
CREATE TEMPORARY SECRET s1 IN LOCAL_FILE ( TYPE S3 )
----
Cannot create temporary secrets in a persistent secret storage!
Invalid Input Error: Persistent secrets are disabled. Restart DuckDB and enable persistent secrets through 'SET allow_persistent_secrets=true'

statement error
CREATE PERSISTENT SECRET s1 IN NON_EXISTENT_SECRET_STORAGE ( TYPE S3 )
----
Secret storage 'non_existent_secret_storage' not found!
Invalid Input Error: Persistent secrets are disabled. Restart DuckDB and enable persistent secrets through 'SET allow_persistent_secrets=true'

# We have disabled the permanent secrets, so this should fail
statement error
CREATE PERSISTENT SECRET perm_s1 ( TYPE S3 )
----
Persistent secrets are currently disabled. To enable them, restart duckdb and run 'SET allow_persistent_secrets=true'
Invalid Input Error: Persistent secrets are disabled. Restart DuckDB and enable persistent secrets through 'SET allow_persistent_secrets=true'

restart

Expand Down
5 changes: 5 additions & 0 deletions tools/pythonpkg/tests/fast/api/test_config.py
Expand Up @@ -74,3 +74,8 @@ def test_user_agent_custom(self, duckdb_cursor):
assert regex.match(con_regular.sql("PRAGMA user_agent").fetchone()[0]) is not None
custom_user_agent = con_regular.sql("SELECT current_setting('custom_user_agent')").fetchone()
assert custom_user_agent[0] == 'CUSTOM_STRING'

def test_secret_manager_option(self, duckdb_cursor):
con = duckdb.connect(':memory:', config={'allow_persistent_secrets': False})
result = con.execute('select count(*) from duckdb_secrets()').fetchall()
assert result == [(0,)]