Skip to content

Commit

Permalink
Add Config::set_internal API.
Browse files Browse the repository at this point in the history
  • Loading branch information
bekadavis9 committed Nov 21, 2024
1 parent 3a2abf9 commit b87a31c
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 34 deletions.
22 changes: 15 additions & 7 deletions test/src/unit-capi-config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* The MIT License
*
* @copyright Copyright (c) 2017-2023 TileDB Inc.
* @copyright Copyright (c) 2017-2024 TileDB Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -40,6 +40,7 @@
#include <map>
#include <sstream>
#include <thread>
#include <unordered_set>

void remove_file(const std::string& filename) {
// Remove file
Expand Down Expand Up @@ -98,9 +99,7 @@ void check_load_incorrect_file_cannot_open() {
rc = tiledb_config_load_from_file(config, "non_existent_file", &error);
CHECK(rc == TILEDB_ERR);
CHECK(error != nullptr);
check_error(
error,
"[TileDB::Config] Error: Failed to open config file 'non_existent_file'");
check_error(error, "Config: Failed to open config file 'non_existent_file'");
tiledb_error_free(&error);
tiledb_config_free(&config);
CHECK(config == nullptr);
Expand All @@ -127,7 +126,7 @@ void check_load_incorrect_file_missing_value() {
CHECK(error != nullptr);
check_error(
error,
"[TileDB::Config] Error: Failed to parse config file 'test_config.txt'; "
"Config: Failed to parse config file 'test_config.txt'; "
"Missing parameter value (line: 1)");
tiledb_error_free(&error);
CHECK(error == nullptr);
Expand Down Expand Up @@ -157,7 +156,7 @@ void check_load_incorrect_file_extra_word() {
CHECK(error != nullptr);
check_error(
error,
"[TileDB::Config] Error: Failed to parse config file 'test_config.txt'; "
"Config: Failed to parse config file 'test_config.txt'; "
"Invalid line format (line: 3)");
tiledb_error_free(&error);
tiledb_config_free(&config);
Expand Down Expand Up @@ -906,6 +905,12 @@ TEST_CASE("C API: Test config iter", "[capi][config]") {
s3_param_values["config_source"] = "auto";
s3_param_values["install_sigpipe_handler"] = "true";

// A list of "sensitive" parameters, whose key-values should not be exposed.
std::unordered_set<std::string> sensitive_param_values;
sensitive_param_values.emplace("rest.token");
sensitive_param_values.emplace("rest.username");
sensitive_param_values.emplace("rest.password");

// Create an iterator and iterate over all parameters
tiledb_config_iter_t* config_iter = nullptr;
rc = tiledb_config_iter_alloc(config, nullptr, &config_iter, &error);
Expand All @@ -924,7 +929,10 @@ TEST_CASE("C API: Test config iter", "[capi][config]") {
CHECK(error == nullptr);
CHECK(param != nullptr);
CHECK(value != nullptr);
all_iter_map[std::string(param)] = std::string(value);
// Skip checks for sensitive params to avoid exposing their values.
if (!sensitive_param_values.contains(std::string(param))) {
all_iter_map[std::string(param)] = std::string(value);
}
rc = tiledb_config_iter_next(config_iter, &error);
CHECK(rc == TILEDB_OK);
CHECK(error == nullptr);
Expand Down
6 changes: 3 additions & 3 deletions test/src/unit-curl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ TEST_CASE(
"RestClient: Remove trailing slash from rest_server_", "[rest-client]") {
std::string rest_server =
GENERATE("http://localhost:8080/", "http://localhost:8080//");
SECTION("rest.server_address set in environment") {
setenv_local("TILEDB_REST_SERVER_ADDRESS", rest_server.c_str());
}
tiledb::sm::Config cfg;
SECTION("rest.server_address set in Config") {
cfg.set("rest.server_address", rest_server).ok();
}
SECTION("rest.server_address set in environment") {
setenv_local("TILEDB_REST_SERVER_ADDRESS", rest_server.c_str());
}
SECTION("rest.server_address set by loaded config file") {
std::string cfg_file = "tiledb_config.txt";
std::ofstream file(cfg_file);
Expand Down
26 changes: 15 additions & 11 deletions tiledb/sm/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ bool ignore_default_via_env(const std::string& param) {
// We should not use the default value for `vfs.s3.region` if the user
// has set either AWS_REGION or AWS_DEFAULT_REGION in their environment.
// We defer to the SDK to interpret these values.

if ((std::getenv("AWS_REGION") != nullptr) ||
(std::getenv("AWS_DEFAULT_REGION") != nullptr)) {
return true;
Expand Down Expand Up @@ -555,6 +554,7 @@ const std::set<std::string> Config::unserialized_params_ = {
Config::Config() {
// Set config values
param_values_ = default_config_values;
login();
}

Config::~Config() = default;
Expand All @@ -573,33 +573,32 @@ void Config::login() {
home_dir = std::string(std::getenv("HOME"));
#endif

// For library versions 22 and older, simply parse the local .json file
if (constants::format_version <= 22) {
// For library versions 2.27.0 and older, simply parse the local .json file
auto version = constants::library_version;
if (version[0] <= 2 && version[1] <= 27) {
// Find and parse the cloud.json file
std::string file = home_dir + "/.tiledb/cloud.json";
if (!std::filesystem::exists(file)) {
throw ConfigException("Cannot login; cloud.json file does not exist.");
return;
}
json data = json::parse(std::ifstream(file));

// Set the config values that have been saved to the file
if (data.contains("api_key") &&
data["api_key"].contains("X-TILEDB-REST-API-KEY")) {
throw_if_not_ok(
set("rest.token", data["api_key"]["X-TILEDB-REST-API-KEY"]));
set_internal("rest.token", data["api_key"]["X-TILEDB-REST-API-KEY"]);
}
if (data.contains("host")) {
throw_if_not_ok(set("rest.server_address", data["host"]));
set_internal("rest.server_address", data["host"]);
}
if (data.contains("password")) {
throw_if_not_ok(set("rest.password", data["password"]));
set_internal("rest.password", data["password"]);
}
if (data.contains("username")) {
throw_if_not_ok(set("rest.username", data["username"]));
set_internal("rest.username", data["username"]);
}
if (data.contains("verify_ssl")) {
throw_if_not_ok(
set("vfs.s3.verify_ssl", data["verify_ssl"] ? "true" : "false"));
set_internal("vfs.s3.verify_ssl", data["verify_ssl"] ? "true" : "false");
}
}
}
Expand Down Expand Up @@ -1030,6 +1029,11 @@ optional<std::string> Config::get_internal_string(
return {nullopt};
}

void Config::set_internal(const std::string& param, const std::string& value) {
throw_if_not_ok(sanity_check(param, value));
param_values_[param] = value;
}

/*
* Explicit instantiations
*/
Expand Down
10 changes: 10 additions & 0 deletions tiledb/sm/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,16 @@ class Config {

/** Returns the param -> value map. */
const std::map<std::string, std::string>& param_values() const;

/**
* Internally sets the given config parameter.
*
* @note For internal use only; This API does not update the user-set params.
*
* @param param The config parameter to set.
* @param value The value of the parameter.
*/
void set_internal(const std::string& param, const std::string& value);
};

/**
Expand Down
13 changes: 0 additions & 13 deletions tiledb/sm/config/test/unit_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,3 @@ TEST_CASE("Config::get<std::string> - found and matched", "[config]") {
CHECK(c.set(key, expected_value).ok());
TestConfig<std::string>::check_expected(expected_value, c, key);
}

TEST_CASE("Config::login", "[config][login]") {
Config c{};

// Upon initial Config construction, no rest token is set
auto initial_token = c.get<std::string>("rest.token");
CHECK(!initial_token.has_value());

// After login, the rest token is set from the cloud.json file
c.login();
auto login_token = c.get<std::string>("rest.token");
CHECK(login_token.has_value());
}

0 comments on commit b87a31c

Please sign in to comment.