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.

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

Jump to bottom

Add Config::login API. #5377

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+107 −45
Open

Add Config::login API. #5377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a2abf9
Add Config::login API.
bekadavis9 Nov 18, 2024
6406980
Add Config::set_internal API.
bekadavis9 Nov 21, 2024
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
22 changes: 15 additions & 7 deletions test/src/unit-capi-config.cc
View file
Open in desktop
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
View file
Open in desktop
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
109 changes: 75 additions & 34 deletions tiledb/sm/config/config.cc
View file
Open in desktop
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 All @@ -30,16 +30,20 @@
* This file implements class Config.
*/

#include <filesystem>
#include <fstream>
#include <iostream>
#include <sstream>

#include "config.h"
#include "external/include/nlohmann/json.hpp"
#include "tiledb/common/logger.h"
#include "tiledb/sm/enums/serialization_type.h"
#include "tiledb/sm/misc/constants.h"
#include "tiledb/sm/misc/parse_argument.h"

using json = nlohmann::json;

using namespace tiledb::common;

namespace {
Expand All @@ -48,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 All @@ -60,14 +63,12 @@ bool ignore_default_via_env(const std::string& param) {

namespace tiledb::sm {

/** Return a Config error class Status with a given message **/
inline Status Status_ConfigError(const std::string& msg) {
return {"[TileDB::Config] Error", msg};
}

void throw_config_exception(const std::string& msg) {
throw StatusException(Status_ConfigError(msg));
}
class ConfigException : public StatusException {
public:
explicit ConfigException(const std::string& msg)
: StatusException("Config", msg) {
}
};

/* ****************************** */
/* CONFIG DEFAULTS */
Expand Down Expand Up @@ -553,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 @@ -561,17 +563,55 @@ Config::~Config() = default;
/* API */
/* ****************************** */

void Config::login() {
// Retrieve $HOME path
std::string home_dir;
#ifdef _WIN32
home_dir =
std::string(std::getenv("HOMEDRIVE")) + std::string(getenv("HOMEPATH"));
#else
home_dir = std::string(std::getenv("HOME"));
#endif

// 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) {
Comment on lines +576 to +578
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this, dev will soon switch to 2.28 and and this will always be false.

// Find and parse the cloud.json file
std::string file = home_dir + "/.tiledb/cloud.json";
if (!std::filesystem::exists(file)) {
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")) {
set_internal("rest.token", data["api_key"]["X-TILEDB-REST-API-KEY"]);
}
if (data.contains("host")) {
set_internal("rest.server_address", data["host"]);
}
if (data.contains("password")) {
set_internal("rest.password", data["password"]);
}
if (data.contains("username")) {
set_internal("rest.username", data["username"]);
}
if (data.contains("verify_ssl")) {
set_internal("vfs.s3.verify_ssl", data["verify_ssl"] ? "true" : "false");
}
}
}

Status Config::load_from_file(const std::string& filename) {
// Do nothing if filename is empty
if (filename.empty())
return LOG_STATUS(
Status_ConfigError("Cannot load from file; Invalid filename"));
if (filename.empty()) {
throw ConfigException("Cannot load from file; Invalid filename");
}

std::ifstream ifs(filename);
if (!ifs.is_open()) {
std::stringstream msg;
msg << "Failed to open config file '" << filename << "'";
return LOG_STATUS(Status_ConfigError(msg.str()));
throw ConfigException("Failed to open config file '" + filename + "'");
}

size_t linenum = 0;
Expand All @@ -592,7 +632,7 @@ Status Config::load_from_file(const std::string& filename) {
std::stringstream msg;
msg << "Failed to parse config file '" << filename << "'; ";
msg << "Missing parameter value (line: " << linenum << ")";
return LOG_STATUS(Status_ConfigError(msg.str()));
throw ConfigException(msg.str());
}

// Parse extra
Expand All @@ -601,7 +641,7 @@ Status Config::load_from_file(const std::string& filename) {
std::stringstream msg;
msg << "Failed to parse config file '" << filename << "'; ";
msg << "Invalid line format (line: " << linenum << ")";
return LOG_STATUS(Status_ConfigError(msg.str()));
throw ConfigException(msg.str());
}

// Set param-value pair
Expand All @@ -614,15 +654,14 @@ Status Config::load_from_file(const std::string& filename) {

Status Config::save_to_file(const std::string& filename) {
// Do nothing if filename is empty
if (filename.empty())
return LOG_STATUS(
Status_ConfigError("Cannot save to file; Invalid filename"));
if (filename.empty()) {
throw ConfigException("Cannot save to file; Invalid filename");
}

std::ofstream ofs(filename);
if (!ofs.is_open()) {
std::stringstream msg;
msg << "Failed to open config file '" << filename << "' for writing";
return LOG_STATUS(Status_ConfigError(msg.str()));
throw ConfigException(
"Failed to open config file '" + filename + "' for writing");
}
for (auto& pv : param_values_) {
if (unserialized_params_.count(pv.first) != 0)
Expand Down Expand Up @@ -664,7 +703,7 @@ Status Config::get(const std::string& param, T* value, bool* found) const {
// Parameter found, retrieve value
auto status = utils::parse::convert(val, value);
if (!status.ok()) {
return Status_ConfigError(
throw ConfigException(
std::string("Failed to parse config value '") + std::string(val) +
std::string("' for key '") + param + "' due to: " + status.to_string());
}
Expand Down Expand Up @@ -751,8 +790,7 @@ Status Config::sanity_check(
RETURN_NOT_OK(utils::parse::convert(value, &v32));
} else if (param == "config.logging_format") {
if (value != "DEFAULT" && value != "JSON")
return LOG_STATUS(
Status_ConfigError("Invalid logging format parameter value"));
throw ConfigException("Invalid logging format parameter value");
} else if (param == "sm.allow_separate_attribute_writes") {
RETURN_NOT_OK(utils::parse::convert(value, &v));
} else if (param == "sm.allow_updates_experimental") {
Expand Down Expand Up @@ -799,8 +837,7 @@ Status Config::sanity_check(
RETURN_NOT_OK(utils::parse::convert(value, &v));
} else if (param == "sm.var_offsets.mode") {
if (value != "bytes" && value != "elements")
return LOG_STATUS(
Status_ConfigError("Invalid offsets format parameter value"));
throw ConfigException("Invalid offsets format parameter value");
} else if (param == "sm.fragment_info.preload_mbrs") {
RETURN_NOT_OK(utils::parse::convert(value, &v));
} else if (param == "ssl.verify") {
Expand All @@ -825,8 +862,7 @@ Status Config::sanity_check(
RETURN_NOT_OK(utils::parse::convert(value, &v32));
} else if (param == "vfs.s3.scheme") {
if (value != "http" && value != "https")
return LOG_STATUS(
Status_ConfigError("Invalid S3 scheme parameter value"));
throw ConfigException("Invalid S3 scheme parameter value");
} else if (param == "vfs.s3.use_virtual_addressing") {
RETURN_NOT_OK(utils::parse::convert(value, &v));
} else if (param == "vfs.s3.skit_init") {
Expand Down Expand Up @@ -866,7 +902,7 @@ Status Config::sanity_check(
(value == "bucket_owner_full_control"))))) {
std::stringstream msg;
msg << "value " << param << " invalid canned acl for " << param;
return Status_Error(msg.str());
throw ConfigException(msg.str());
}
}

Expand Down Expand Up @@ -970,7 +1006,7 @@ optional<T> Config::get_internal(const std::string& key) const {
if (status.ok()) {
return {converted_value};
}
throw_config_exception(
throw ConfigException(
"Failed to parse config value '" + std::string(value.value()) +
"' for key '" + key + "'. Reason: " + status.to_string());
}
Expand All @@ -988,11 +1024,16 @@ optional<std::string> Config::get_internal_string(
}

if constexpr (must_find_) {
throw_config_exception("Failed to get config value for key: " + key);
throw ConfigException("Failed to get config value for key: " + key);
}
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
15 changes: 14 additions & 1 deletion tiledb/sm/config/config.h
View file
Open in desktop
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 @@ -667,6 +667,9 @@ class Config {
/* API */
/* ********************************* */

/** Log in to TileDB Cloud, saving profile info to the config. */
void login();

/** Loads the config parameters from a configuration (local) file. */
Status load_from_file(const std::string& filename);

Expand Down Expand Up @@ -824,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
Loading