From f016fc18ee993a180b1bdb26f631dcc1a5e930f8 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Tue, 17 Mar 2026 15:17:56 +0000 Subject: [PATCH 1/8] Implemented automatic grpc loop with reconnection Signed-off-by: Marcin Olko --- providers/flagd/src/BUILD | 1 + providers/flagd/src/configuration.cpp | 181 ++++++++++++++++ providers/flagd/src/configuration.h | 24 +++ providers/flagd/src/sync/grpc/grpc_sync.cpp | 216 ++++++++++++++------ providers/flagd/src/sync/grpc/grpc_sync.h | 3 + providers/flagd/src/sync/sync.cpp | 9 + providers/flagd/src/sync/sync.h | 1 + 7 files changed, 375 insertions(+), 60 deletions(-) diff --git a/providers/flagd/src/BUILD b/providers/flagd/src/BUILD index 2ccd666..2050369 100644 --- a/providers/flagd/src/BUILD +++ b/providers/flagd/src/BUILD @@ -26,6 +26,7 @@ cc_library( deps = [ "@abseil-cpp//absl/strings", "@com_github_grpc_grpc//:grpc++", + "@nlohmann_json//:json", "@openfeature_cpp_sdk//openfeature", ], ) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 8192dee..6cb7a81 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -1,20 +1,28 @@ #include "configuration.h" +#include #include #include +#include #include #include +#include #include #include #include +#include +#include "absl/log/log.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" +#include "nlohmann/json.hpp" namespace flagd { namespace { +constexpr double kMsInSecond = 1000.0; + struct EnvVars { static constexpr std::string_view kHost = "FLAGD_HOST"; static constexpr std::string_view kPort = "FLAGD_PORT"; @@ -23,6 +31,17 @@ struct EnvVars { static constexpr std::string_view kSocketPath = "FLAGD_SOCKET_PATH"; static constexpr std::string_view kServerCertPath = "FLAGD_SERVER_CERT_PATH"; static constexpr std::string_view kDeadlineMs = "FLAGD_DEADLINE_MS"; + static constexpr std::string_view kStreamDeadlineMs = + "FLAGD_STREAM_DEADLINE_MS"; + static constexpr std::string_view kRetryBackoffMs = "FLAGD_RETRY_BACKOFF_MS"; + static constexpr std::string_view kRetryBackoffMaxMs = + "FLAGD_RETRY_BACKOFF_MAX_MS"; + static constexpr std::string_view kRetryGracePeriod = + "FLAGD_RETRY_GRACE_PERIOD"; + static constexpr std::string_view kKeepAliveTimeMs = + "FLAGD_KEEP_ALIVE_TIME_MS"; + static constexpr std::string_view kFatalStatusCodes = + "FLAGD_FATAL_STATUS_CODES"; static constexpr std::string_view kSourceSelector = "FLAGD_SOURCE_SELECTOR"; static constexpr std::string_view kProviderId = "FLAGD_PROVIDER_ID"; static constexpr std::string_view kOfflineFlagSourcePath = @@ -35,8 +54,33 @@ struct Defaults { static constexpr int kPortInProcess = 8015; static constexpr bool kTls = false; static constexpr int kDeadlineMs = 500; + static constexpr int kStreamDeadlineMs = 600000; + static constexpr int kRetryBackoffMs = 1000; + static constexpr int kRetryBackoffMaxMs = 12000; + static constexpr int kRetryGracePeriod = 5; + static constexpr uint64_t kKeepAliveTimeMs = 0; static constexpr int kOfflinePollMs = 5000; }; + +const std::map kStatusCodeMap = { + {"OK", 0}, + {"CANCELLED", 1}, + {"UNKNOWN", 2}, + {"INVALID_ARGUMENT", 3}, + {"DEADLINE_EXCEEDED", 4}, + {"NOT_FOUND", 5}, + {"ALREADY_EXISTS", 6}, + {"PERMISSION_DENIED", 7}, + {"UNAUTHENTICATED", 16}, + {"RESOURCE_EXHAUSTED", 8}, + {"FAILED_PRECONDITION", 9}, + {"ABORTED", 10}, + {"OUT_OF_RANGE", 11}, + {"UNIMPLEMENTED", 12}, + {"INTERNAL", 13}, + {"UNAVAILABLE", 14}, + {"DATA_LOSS", 15}, +}; } // namespace // --- Helpers --- @@ -58,6 +102,19 @@ static int GetEnvInt(const std::string_view name, int default_value) { return default_value; } +static uint64_t GetEnvLong(const std::string_view name, + uint64_t default_value) { + const char* val = std::getenv(std::string(name).c_str()); + if (val != nullptr) { + try { + return std::stoul(val); + } catch (...) { + return default_value; + } + } + return default_value; +} + static bool GetEnvBool(const std::string_view name, bool default_value) { const char* val = std::getenv(std::string(name).c_str()); if (val == nullptr) { @@ -69,11 +126,48 @@ static bool GetEnvBool(const std::string_view name, bool default_value) { return str == "true" || str == "1"; } +static std::vector ParseFatalStatusCodes(const std::string& str) { + std::vector result; + std::stringstream sstream(str); + std::string item; + while (std::getline(sstream, item, ',')) { + item.erase(0, item.find_first_not_of(" \t\n\r")); + item.erase(item.find_last_not_of(" \t\n\r") + 1); + + if (item.empty()) continue; + + try { + int code = std::stoi(item); + result.push_back(code); + continue; + } catch (...) { + } + + auto iter = kStatusCodeMap.find(item); + if (iter != kStatusCodeMap.end()) { + result.push_back(iter->second); + } else { + LOG(WARNING) << "Unknown gRPC status code: " << item; + } + } + return result; +} + FlagdProviderConfig::FlagdProviderConfig() : host_(GetEnvStr(EnvVars::kHost, Defaults::kHost)), port_(GetEnvInt(EnvVars::kPort, Defaults::kPortInProcess)), tls_(GetEnvBool(EnvVars::kTls, Defaults::kTls)), deadline_ms_(GetEnvInt(EnvVars::kDeadlineMs, Defaults::kDeadlineMs)), + stream_deadline_ms_( + GetEnvInt(EnvVars::kStreamDeadlineMs, Defaults::kStreamDeadlineMs)), + retry_backoff_ms_( + GetEnvInt(EnvVars::kRetryBackoffMs, Defaults::kRetryBackoffMs)), + retry_backoff_max_ms_( + GetEnvInt(EnvVars::kRetryBackoffMaxMs, Defaults::kRetryBackoffMaxMs)), + retry_grace_period_( + GetEnvInt(EnvVars::kRetryGracePeriod, Defaults::kRetryGracePeriod)), + keep_alive_time_ms_( + GetEnvLong(EnvVars::kKeepAliveTimeMs, Defaults::kKeepAliveTimeMs)), offline_poll_interval_ms_( GetEnvInt(EnvVars::kOfflinePollMs, Defaults::kOfflinePollMs)) { if (std::string val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { @@ -95,6 +189,9 @@ FlagdProviderConfig::FlagdProviderConfig() !val.empty()) { offline_flag_source_path_ = val; } + if (std::string val = GetEnvStr(EnvVars::kFatalStatusCodes); !val.empty()) { + fatal_status_codes_ = ParseFatalStatusCodes(val); + } } std::string FlagdProviderConfig::GetEffectiveTargetUri() const { @@ -150,6 +247,55 @@ std::optional FlagdProviderConfig::GetCertPath() const { return cert_path_; } int FlagdProviderConfig::GetDeadlineMs() const { return deadline_ms_; } +int FlagdProviderConfig::GetStreamDeadlineMs() const { + return stream_deadline_ms_; +} +int FlagdProviderConfig::GetRetryBackoffMs() const { return retry_backoff_ms_; } +int FlagdProviderConfig::GetRetryBackoffMaxMs() const { + return retry_backoff_max_ms_; +} +int FlagdProviderConfig::GetRetryGracePeriod() const { + return retry_grace_period_; +} +uint64_t FlagdProviderConfig::GetKeepAliveTimeMs() const { + return keep_alive_time_ms_; +} +const std::vector& FlagdProviderConfig::GetFatalStatusCodes() const { + return fatal_status_codes_; +} + +std::string FlagdProviderConfig::GetServiceConfigJson() const { + nlohmann::json config = nlohmann::json::object(); + nlohmann::json method_config = nlohmann::json::array(); + nlohmann::json rule = nlohmann::json::object(); + + nlohmann::json name = nlohmann::json::array(); + name.push_back( + nlohmann::json::object({{"service", "flagd.evaluation.v1.Service"}})); + name.push_back( + nlohmann::json::object({{"service", "flagd.sync.v1.FlagSyncService"}})); + rule["name"] = name; + + nlohmann::json retry_policy = nlohmann::json::object(); + retry_policy["maxAttempts"] = 4; + retry_policy["initialBackoff"] = + absl::StrCat(retry_backoff_ms_ / kMsInSecond, "s"); + retry_policy["maxBackoff"] = + absl::StrCat(retry_backoff_max_ms_ / kMsInSecond, "s"); + retry_policy["backoffMultiplier"] = 2; + + nlohmann::json retryable_status_codes = nlohmann::json::array(); + retryable_status_codes.push_back("UNAVAILABLE"); + retryable_status_codes.push_back("UNKNOWN"); + retry_policy["retryableStatusCodes"] = retryable_status_codes; + + rule["retryPolicy"] = retry_policy; + method_config.push_back(rule); + config["methodConfig"] = method_config; + + return config.dump(); +} + std::optional FlagdProviderConfig::GetSelector() const { return selector_; } @@ -198,6 +344,41 @@ FlagdProviderConfig& FlagdProviderConfig::SetDeadlineMs(int deadline_ms) { deadline_ms_ = deadline_ms; return *this; } +FlagdProviderConfig& FlagdProviderConfig::SetStreamDeadlineMs( + int stream_deadline_ms) { + stream_deadline_ms_ = stream_deadline_ms; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMs( + int retry_backoff_ms) { + retry_backoff_ms_ = retry_backoff_ms; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMaxMs( + int retry_backoff_max_ms) { + retry_backoff_max_ms_ = retry_backoff_max_ms; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetRetryGracePeriod( + int retry_grace_period) { + retry_grace_period_ = retry_grace_period; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetKeepAliveTimeMs( + uint64_t keep_alive_time_ms) { + keep_alive_time_ms_ = keep_alive_time_ms; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetFatalStatusCodes( + const std::vector& fatal_status_codes) { + fatal_status_codes_ = fatal_status_codes; + return *this; +} +FlagdProviderConfig& FlagdProviderConfig::SetFatalStatusCodes( + const std::string& fatal_status_codes_str) { + fatal_status_codes_ = ParseFatalStatusCodes(fatal_status_codes_str); + return *this; +} FlagdProviderConfig& FlagdProviderConfig::SetSelector( std::string_view selector) { selector_ = std::string(selector); diff --git a/providers/flagd/src/configuration.h b/providers/flagd/src/configuration.h index d15435b..efd1f12 100644 --- a/providers/flagd/src/configuration.h +++ b/providers/flagd/src/configuration.h @@ -27,6 +27,14 @@ class FlagdProviderConfig { std::optional GetCertPath() const; int GetDeadlineMs() const; + int GetStreamDeadlineMs() const; + int GetRetryBackoffMs() const; + int GetRetryBackoffMaxMs() const; + int GetRetryGracePeriod() const; + uint64_t GetKeepAliveTimeMs() const; + const std::vector& GetFatalStatusCodes() const; + + std::string GetServiceConfigJson() const; std::optional GetSelector() const; std::optional GetProviderId() const; @@ -54,6 +62,16 @@ class FlagdProviderConfig { FlagdProviderConfig& SetSocketPath(std::string_view path); FlagdProviderConfig& SetCertPath(std::string_view path); FlagdProviderConfig& SetDeadlineMs(int deadline_ms); + FlagdProviderConfig& SetStreamDeadlineMs(int stream_deadline_ms); + FlagdProviderConfig& SetRetryBackoffMs(int retry_backoff_ms); + FlagdProviderConfig& SetRetryBackoffMaxMs(int retry_backoff_max_ms); + FlagdProviderConfig& SetRetryGracePeriod(int retry_grace_period); + FlagdProviderConfig& SetKeepAliveTimeMs(uint64_t keep_alive_time_ms); + FlagdProviderConfig& SetFatalStatusCodes( + const std::vector& fatal_status_codes); + FlagdProviderConfig& SetFatalStatusCodes( + const std::string& fatal_status_codes_str); + FlagdProviderConfig& SetSelector(std::string_view selector); FlagdProviderConfig& SetProviderId(std::string_view provider_id); FlagdProviderConfig& SetOfflineFlagSourcePath(std::string_view path); @@ -69,6 +87,12 @@ class FlagdProviderConfig { std::optional cert_path_; int deadline_ms_; + int stream_deadline_ms_; + int retry_backoff_ms_; + int retry_backoff_max_ms_; + int retry_grace_period_; + uint64_t keep_alive_time_ms_; + std::vector fatal_status_codes_; std::optional selector_; std::optional provider_id_; diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index ebe2352..80b3f34 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -2,6 +2,8 @@ #include +#include +#include #include #include #include @@ -36,18 +38,31 @@ absl::Status GrpcSync::Init(const openfeature::EvaluationContext& ctx) { case State::kReady: return absl::OkStatus(); case State::kInitializing: - lifecycle_cv_.wait(lock, - [this] { return state_ != State::kInitializing; }); + if (!lifecycle_cv_.wait_for( + lock, std::chrono::milliseconds(config_.GetDeadlineMs()), + [this] { return state_ != State::kInitializing; })) { + init_timed_out_ = true; + init_result_ = absl::DeadlineExceededError("Initialization timed out"); + return init_result_; + } return (state_ == State::kReady) ? absl::OkStatus() : init_result_; + case State::kReconnecting: + if (init_timed_out_) { + return absl::DeadlineExceededError("Initialization timed out"); + } + return init_result_; case State::kShuttingDown: lifecycle_cv_.wait(lock, [this] { return state_ == State::kUninitialized; }); break; + case State::kFatal: + return init_result_; case State::kUninitialized: break; } state_ = State::kInitializing; + LOG(INFO) << "GrpcSync state changed to kInitializing"; init_result_ = absl::UnknownError("Initialization incomplete"); std::string target = config_.GetEffectiveTargetUri(); @@ -55,30 +70,37 @@ absl::Status GrpcSync::Init(const openfeature::EvaluationContext& ctx) { config_.GetEffectiveCredentials(); if (!creds.ok()) { state_ = State::kUninitialized; + LOG(INFO) << "GrpcSync state changed to kUninitialized"; return creds.status(); } - std::shared_ptr channel = grpc::CreateChannel(target, *creds); + std::shared_ptr channel; + grpc::ChannelArguments args; + args.SetServiceConfigJSON(config_.GetServiceConfigJson()); + if (config_.GetKeepAliveTimeMs() > 0) { + args.SetInt("grpc.keepalive_time_ms", config_.GetKeepAliveTimeMs()); + } + channel = grpc::CreateCustomChannel(target, *creds, args); stub_ = FlagSyncService::NewStub(channel); - context_ = std::make_shared(); - try { background_thread_ = std::thread(&GrpcSync::WaitForUpdates, this); } catch (const std::exception& e) { state_ = State::kUninitialized; + LOG(INFO) << "GrpcSync state changed to kUninitialized"; return absl::InternalError( absl::StrCat("Failed to spawn thread: ", e.what())); } - lifecycle_cv_.wait(lock, [this] { return state_ != State::kInitializing; }); + if (!lifecycle_cv_.wait_for( + lock, std::chrono::milliseconds(config_.GetDeadlineMs()), + [this] { return state_ != State::kInitializing; })) { + init_timed_out_ = true; + init_result_ = absl::DeadlineExceededError("Initialization timed out"); + return init_result_; + } if (state_ != State::kReady) { - if (background_thread_.joinable()) { - lock.unlock(); - background_thread_.join(); - lock.lock(); - } return init_result_; } @@ -100,6 +122,7 @@ absl::Status GrpcSync::Shutdown() { State previous_state = state_; state_ = State::kShuttingDown; + LOG(INFO) << "GrpcSync state changed to kShuttingDown"; if (context_) { context_->TryCancel(); @@ -114,6 +137,7 @@ absl::Status GrpcSync::Shutdown() { context_.reset(); stub_.reset(); state_ = State::kUninitialized; + LOG(INFO) << "GrpcSync state changed to kUninitialized"; if (previous_state == State::kInitializing) { init_result_ = @@ -126,68 +150,140 @@ absl::Status GrpcSync::Shutdown() { } void GrpcSync::WaitForUpdates() { - // TODO(#12) Add automatic reconection with exponential backoff - SyncFlagsRequest request; + auto last_connect_attempt = std::chrono::steady_clock::now(); + int retry_count = 0; - if (config_.GetProviderId().has_value() && - !config_.GetProviderId()->empty()) { - request.set_provider_id(*config_.GetProviderId()); - } - - std::shared_ptr local_ctx; - { - std::scoped_lock lock(lifecycle_mutex_); - local_ctx = context_; - } - - if (!local_ctx) return; + while (true) { + { + std::scoped_lock lock(lifecycle_mutex_); + if (state_ == State::kShuttingDown || state_ == State::kFatal) break; + } - auto reader = stub_->SyncFlags(local_ctx.get(), request); - SyncFlagsResponse response; + SyncFlagsRequest request; + if (config_.GetProviderId().has_value() && + !config_.GetProviderId()->empty()) { + request.set_provider_id(*config_.GetProviderId()); + } - bool first_read = true; - while (reader->Read(&response)) { - try { - Json raw = Json::parse(response.flag_configuration()); + std::shared_ptr local_ctx = + std::make_shared(); + { + std::scoped_lock lock(lifecycle_mutex_); + if (state_ == State::kShuttingDown) break; + context_ = local_ctx; + } + if (config_.GetStreamDeadlineMs() > 0) { + auto deadline = std::chrono::system_clock::now() + + std::chrono::milliseconds(config_.GetStreamDeadlineMs()); + local_ctx->set_deadline(deadline); + } - UpdateFlags(raw); + auto reader = stub_->SyncFlags(local_ctx.get(), request); - if (first_read) { - std::scoped_lock lock(lifecycle_mutex_); - if (state_ == State::kInitializing) { - state_ = State::kReady; - init_result_ = absl::OkStatus(); - lifecycle_cv_.notify_all(); - } - first_read = false; + if (!reader) { + LOG(ERROR) << "Failed to create sync stream"; + std::unique_lock lock(lifecycle_mutex_); + if (state_ == State::kInitializing) { + state_ = State::kReconnecting; + LOG(INFO) << "GrpcSync state changed to kReconnecting"; + init_result_ = absl::InternalError("Failed to create stream"); + lifecycle_cv_.notify_all(); } - } catch (const std::exception& e) { - LOG(ERROR) << "Failed to parse flag configuration: " << e.what(); - if (first_read) { - std::scoped_lock lock(lifecycle_mutex_); - if (state_ == State::kInitializing) { - state_ = State::kUninitialized; - init_result_ = - absl::InternalError(absl::StrCat("Parse error: ", e.what())); - lifecycle_cv_.notify_all(); + lifecycle_cv_.wait_for( + lock, std::chrono::milliseconds(config_.GetRetryBackoffMs()), [this] { + return state_ == State::kShuttingDown || state_ == State::kFatal; + }); + continue; + } + + SyncFlagsResponse response; + bool connected = false; + + while (reader->Read(&response)) { + try { + Json raw = Json::parse(response.flag_configuration()); + UpdateFlags(raw); + + if (!connected) { + connected = true; + retry_count = 0; + std::scoped_lock lock(lifecycle_mutex_); + if (state_ == State::kInitializing || + state_ == State::kReconnecting) { + state_ = State::kReady; + LOG(INFO) << "GrpcSync state changed to kReady"; + init_result_ = absl::OkStatus(); + lifecycle_cv_.notify_all(); + // TODO: emit PROVIDER_READY + // TODO: emit PROVIDER_CONFIGURATION_CHANGED + } } - // If we fail first read, we abort the stream - return; + // TODO: emit PROVIDER_CONFIGURATION_CHANGED + } catch (const std::exception& e) { + LOG(ERROR) << "Failed to parse flag configuration: " << e.what(); } } - } - grpc::Status status = reader->Finish(); + grpc::Status status = reader->Finish(); + LOG(WARNING) << "Sync stream closed: " << status.error_message() + << " (code: " << status.error_code() << ")"; - { - std::scoped_lock lock(lifecycle_mutex_); - if (state_ == State::kInitializing) { - state_ = State::kUninitialized; - init_result_ = status.ok() - ? absl::InternalError("Stream closed immediately") - : absl::InternalError(status.error_message()); + // Check fatal status codes + bool is_fatal = false; + for (int fatal_code : config_.GetFatalStatusCodes()) { + if (status.error_code() == fatal_code) { + is_fatal = true; + break; + } + } + + if (is_fatal) { + ClearFlags(); + std::scoped_lock lock(lifecycle_mutex_); + state_ = State::kFatal; + LOG(INFO) << "GrpcSync state changed to kFatal"; + init_result_ = absl::InternalError( + absl::StrCat("Fatal gRPC error: ", status.error_message())); lifecycle_cv_.notify_all(); + // TODO: emit PROVIDER_FATAL + break; + } + + { + std::scoped_lock lock(lifecycle_mutex_); + if (state_ == State::kInitializing) { + state_ = State::kReconnecting; + LOG(INFO) << "GrpcSync state changed to kReconnecting"; + init_result_ = absl::InternalError( + absl::StrCat("Stream failed: ", status.error_message())); + lifecycle_cv_.notify_all(); + } else if (state_ == State::kReady) { + state_ = State::kReconnecting; + LOG(INFO) << "GrpcSync state changed to kReconnecting"; + // TODO: emit PROVIDER_STALE + } + } + + int64_t backoff = config_.GetRetryBackoffMs() * (1 << retry_count); + backoff = std::min(backoff, config_.GetRetryBackoffMaxMs()); + retry_count++; + + auto now = std::chrono::steady_clock::now(); + auto disconnected_duration = + std::chrono::duration_cast(now - + last_connect_attempt); + if (disconnected_duration.count() > config_.GetRetryGracePeriod()) { + ClearFlags(); + // TODO: emit PROVIDER_ERROR + } + + { + std::unique_lock lock(lifecycle_mutex_); + lifecycle_cv_.wait_for(lock, std::chrono::milliseconds(backoff), [this] { + return state_ == State::kShuttingDown || state_ == State::kFatal; + }); } + last_connect_attempt = std::chrono::steady_clock::now(); } } diff --git a/providers/flagd/src/sync/grpc/grpc_sync.h b/providers/flagd/src/sync/grpc/grpc_sync.h index dc45abe..7538f83 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.h +++ b/providers/flagd/src/sync/grpc/grpc_sync.h @@ -33,7 +33,9 @@ class GrpcSync final : public FlagSync { kUninitialized, kInitializing, // Thread started, waiting for first connection kReady, // First sync complete, running normally + kReconnecting, // Connection lost or failed, trying to reconnect kShuttingDown, // Shutdown requested, cleaning up + kFatal, // Fatal error, gives up }; std::thread background_thread_; @@ -42,6 +44,7 @@ class GrpcSync final : public FlagSync { std::condition_variable lifecycle_cv_; State state_ = State::kUninitialized; absl::Status init_result_; + bool init_timed_out_ = false; FlagdProviderConfig config_; }; diff --git a/providers/flagd/src/sync/sync.cpp b/providers/flagd/src/sync/sync.cpp index 74a11cc..8297e67 100644 --- a/providers/flagd/src/sync/sync.cpp +++ b/providers/flagd/src/sync/sync.cpp @@ -85,6 +85,15 @@ void FlagSync::UpdateFlags(const nlohmann::json& new_json) { } } +void FlagSync::ClearFlags() { + { + std::scoped_lock lock(flags_mutex_); + current_flags_ = std::make_shared(nlohmann::json::object()); + global_metadata_ = + std::make_shared(nlohmann::json::object()); + } +} + std::shared_ptr FlagSync::GetFlags() const { std::shared_lock lock(flags_mutex_); return current_flags_; diff --git a/providers/flagd/src/sync/sync.h b/providers/flagd/src/sync/sync.h index 6e3f613..f0cfd4b 100644 --- a/providers/flagd/src/sync/sync.h +++ b/providers/flagd/src/sync/sync.h @@ -23,6 +23,7 @@ class FlagSync { protected: void UpdateFlags(const nlohmann::json& new_json); + void ClearFlags(); private: mutable std::shared_mutex flags_mutex_; From 73280da83dd1b5a66fc1f72fab2aec3910849008 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Tue, 17 Mar 2026 16:12:29 +0000 Subject: [PATCH 2/8] Applied gemini suggestions Signed-off-by: Marcin Olko --- providers/flagd/src/configuration.cpp | 56 ++++++++++----------- providers/flagd/src/sync/grpc/grpc_sync.cpp | 12 ++--- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 6cb7a81..930a676 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -108,7 +108,9 @@ static uint64_t GetEnvLong(const std::string_view name, if (val != nullptr) { try { return std::stoul(val); - } catch (...) { + } catch (const std::invalid_argument&) { + return default_value; + } catch (const std::out_of_range&) { return default_value; } } @@ -140,7 +142,10 @@ static std::vector ParseFatalStatusCodes(const std::string& str) { int code = std::stoi(item); result.push_back(code); continue; - } catch (...) { + } catch (const std::invalid_argument&) { + // Not an integer, try parsing as a status code string. + } catch (const std::out_of_range&) { + // Not a valid integer, try parsing as a status code string. } auto iter = kStatusCodeMap.find(item); @@ -265,35 +270,30 @@ const std::vector& FlagdProviderConfig::GetFatalStatusCodes() const { } std::string FlagdProviderConfig::GetServiceConfigJson() const { - nlohmann::json config = nlohmann::json::object(); - nlohmann::json method_config = nlohmann::json::array(); - nlohmann::json rule = nlohmann::json::object(); - - nlohmann::json name = nlohmann::json::array(); - name.push_back( - nlohmann::json::object({{"service", "flagd.evaluation.v1.Service"}})); - name.push_back( - nlohmann::json::object({{"service", "flagd.sync.v1.FlagSyncService"}})); - rule["name"] = name; - - nlohmann::json retry_policy = nlohmann::json::object(); - retry_policy["maxAttempts"] = 4; - retry_policy["initialBackoff"] = - absl::StrCat(retry_backoff_ms_ / kMsInSecond, "s"); - retry_policy["maxBackoff"] = - absl::StrCat(retry_backoff_max_ms_ / kMsInSecond, "s"); - retry_policy["backoffMultiplier"] = 2; + const auto names = nlohmann::json::array({ + nlohmann::json::object({{"service", "flagd.evaluation.v1.Service"}}), + nlohmann::json::object({{"service", "flagd.sync.v1.FlagSyncService"}}), + }); - nlohmann::json retryable_status_codes = nlohmann::json::array(); - retryable_status_codes.push_back("UNAVAILABLE"); - retryable_status_codes.push_back("UNKNOWN"); - retry_policy["retryableStatusCodes"] = retryable_status_codes; + const auto retry_policy = nlohmann::json::object({ + {"maxAttempts", 4}, + {"initialBackoff", absl::StrCat(retry_backoff_ms_ / kMsInSecond, "s")}, + {"maxBackoff", absl::StrCat(retry_backoff_max_ms_ / kMsInSecond, "s")}, + {"backoffMultiplier", 2}, + { + "retryableStatusCodes", + nlohmann::json::array({"UNAVAILABLE", "UNKNOWN"}), + }, + }); - rule["retryPolicy"] = retry_policy; - method_config.push_back(rule); - config["methodConfig"] = method_config; + const auto method_config = nlohmann::json::object({ + {"name", names}, + {"retryPolicy", retry_policy}, + }); - return config.dump(); + return nlohmann::json::object( + {{"methodConfig", nlohmann::json::array({method_config})}}) + .dump(); } std::optional FlagdProviderConfig::GetSelector() const { diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index 80b3f34..1b7642e 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -229,13 +229,9 @@ void GrpcSync::WaitForUpdates() { << " (code: " << status.error_code() << ")"; // Check fatal status codes - bool is_fatal = false; - for (int fatal_code : config_.GetFatalStatusCodes()) { - if (status.error_code() == fatal_code) { - is_fatal = true; - break; - } - } + const auto& fatal_codes = config_.GetFatalStatusCodes(); + bool is_fatal = std::find(fatal_codes.cbegin(), fatal_codes.cend(), + status.error_code()) != fatal_codes.cend(); if (is_fatal) { ClearFlags(); @@ -264,7 +260,7 @@ void GrpcSync::WaitForUpdates() { } } - int64_t backoff = config_.GetRetryBackoffMs() * (1 << retry_count); + int64_t backoff = config_.GetRetryBackoffMs() * (1LL << retry_count); backoff = std::min(backoff, config_.GetRetryBackoffMaxMs()); retry_count++; From 5c03d8a3b445d68fc6e1401dc5b3114fac07bd59 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Wed, 18 Mar 2026 10:56:51 +0000 Subject: [PATCH 3/8] Addressed gemini comments Signed-off-by: Marcin Olko --- providers/flagd/src/configuration.cpp | 23 ++++----------------- providers/flagd/src/configuration.h | 6 +++--- providers/flagd/src/sync/grpc/grpc_sync.cpp | 9 +++++--- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 930a676..5e460bb 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -58,7 +58,7 @@ struct Defaults { static constexpr int kRetryBackoffMs = 1000; static constexpr int kRetryBackoffMaxMs = 12000; static constexpr int kRetryGracePeriod = 5; - static constexpr uint64_t kKeepAliveTimeMs = 0; + static constexpr int kKeepAliveTimeMs = 0; static constexpr int kOfflinePollMs = 5000; }; @@ -102,21 +102,6 @@ static int GetEnvInt(const std::string_view name, int default_value) { return default_value; } -static uint64_t GetEnvLong(const std::string_view name, - uint64_t default_value) { - const char* val = std::getenv(std::string(name).c_str()); - if (val != nullptr) { - try { - return std::stoul(val); - } catch (const std::invalid_argument&) { - return default_value; - } catch (const std::out_of_range&) { - return default_value; - } - } - return default_value; -} - static bool GetEnvBool(const std::string_view name, bool default_value) { const char* val = std::getenv(std::string(name).c_str()); if (val == nullptr) { @@ -172,7 +157,7 @@ FlagdProviderConfig::FlagdProviderConfig() retry_grace_period_( GetEnvInt(EnvVars::kRetryGracePeriod, Defaults::kRetryGracePeriod)), keep_alive_time_ms_( - GetEnvLong(EnvVars::kKeepAliveTimeMs, Defaults::kKeepAliveTimeMs)), + GetEnvInt(EnvVars::kKeepAliveTimeMs, Defaults::kKeepAliveTimeMs)), offline_poll_interval_ms_( GetEnvInt(EnvVars::kOfflinePollMs, Defaults::kOfflinePollMs)) { if (std::string val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { @@ -262,7 +247,7 @@ int FlagdProviderConfig::GetRetryBackoffMaxMs() const { int FlagdProviderConfig::GetRetryGracePeriod() const { return retry_grace_period_; } -uint64_t FlagdProviderConfig::GetKeepAliveTimeMs() const { +int FlagdProviderConfig::GetKeepAliveTimeMs() const { return keep_alive_time_ms_; } const std::vector& FlagdProviderConfig::GetFatalStatusCodes() const { @@ -365,7 +350,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetRetryGracePeriod( return *this; } FlagdProviderConfig& FlagdProviderConfig::SetKeepAliveTimeMs( - uint64_t keep_alive_time_ms) { + int keep_alive_time_ms) { keep_alive_time_ms_ = keep_alive_time_ms; return *this; } diff --git a/providers/flagd/src/configuration.h b/providers/flagd/src/configuration.h index efd1f12..f276ba6 100644 --- a/providers/flagd/src/configuration.h +++ b/providers/flagd/src/configuration.h @@ -31,7 +31,7 @@ class FlagdProviderConfig { int GetRetryBackoffMs() const; int GetRetryBackoffMaxMs() const; int GetRetryGracePeriod() const; - uint64_t GetKeepAliveTimeMs() const; + int GetKeepAliveTimeMs() const; const std::vector& GetFatalStatusCodes() const; std::string GetServiceConfigJson() const; @@ -66,7 +66,7 @@ class FlagdProviderConfig { FlagdProviderConfig& SetRetryBackoffMs(int retry_backoff_ms); FlagdProviderConfig& SetRetryBackoffMaxMs(int retry_backoff_max_ms); FlagdProviderConfig& SetRetryGracePeriod(int retry_grace_period); - FlagdProviderConfig& SetKeepAliveTimeMs(uint64_t keep_alive_time_ms); + FlagdProviderConfig& SetKeepAliveTimeMs(int keep_alive_time_ms); FlagdProviderConfig& SetFatalStatusCodes( const std::vector& fatal_status_codes); FlagdProviderConfig& SetFatalStatusCodes( @@ -91,7 +91,7 @@ class FlagdProviderConfig { int retry_backoff_ms_; int retry_backoff_max_ms_; int retry_grace_period_; - uint64_t keep_alive_time_ms_; + int keep_alive_time_ms_; std::vector fatal_status_codes_; std::optional selector_; diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index 1b7642e..2371d2c 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -150,7 +150,7 @@ absl::Status GrpcSync::Shutdown() { } void GrpcSync::WaitForUpdates() { - auto last_connect_attempt = std::chrono::steady_clock::now(); + auto last_healthy_time = std::chrono::steady_clock::now(); int retry_count = 0; while (true) { @@ -228,6 +228,10 @@ void GrpcSync::WaitForUpdates() { LOG(WARNING) << "Sync stream closed: " << status.error_message() << " (code: " << status.error_code() << ")"; + if (connected) { + last_healthy_time = std::chrono::steady_clock::now(); + } + // Check fatal status codes const auto& fatal_codes = config_.GetFatalStatusCodes(); bool is_fatal = std::find(fatal_codes.cbegin(), fatal_codes.cend(), @@ -267,7 +271,7 @@ void GrpcSync::WaitForUpdates() { auto now = std::chrono::steady_clock::now(); auto disconnected_duration = std::chrono::duration_cast(now - - last_connect_attempt); + last_healthy_time); if (disconnected_duration.count() > config_.GetRetryGracePeriod()) { ClearFlags(); // TODO: emit PROVIDER_ERROR @@ -279,7 +283,6 @@ void GrpcSync::WaitForUpdates() { return state_ == State::kShuttingDown || state_ == State::kFatal; }); } - last_connect_attempt = std::chrono::steady_clock::now(); } } From 046e1d8780be94bb6a2c7b035a91baf79665af83 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Wed, 18 Mar 2026 14:00:35 +0000 Subject: [PATCH 4/8] Added checks for flagd configuration and mitigated int overflow risk Signed-off-by: Marcin Olko --- providers/flagd/src/configuration.cpp | 144 ++++++++++++++----- providers/flagd/src/sync/grpc/grpc_sync.cpp | 7 +- providers/flagd/tests/configuration_test.cpp | 50 +++++++ 3 files changed, 166 insertions(+), 35 deletions(-) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 5e460bb..63f9b1a 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -62,6 +61,14 @@ struct Defaults { static constexpr int kOfflinePollMs = 5000; }; +struct Validation { + static constexpr int kMinPort = 1; + static constexpr int kMaxPort = 65535; + static constexpr int kMinTimingMs = 0; + static constexpr int kMinStatusCode = 0; + static constexpr int kMaxStatusCode = 16; +}; + const std::map kStatusCodeMap = { {"OK", 0}, {"CANCELLED", 1}, @@ -113,6 +120,19 @@ static bool GetEnvBool(const std::string_view name, bool default_value) { return str == "true" || str == "1"; } +static bool IsValidPort(int port) { + return port >= Validation::kMinPort && port <= Validation::kMaxPort; +} + +static bool IsValidTiming(int timing_ms) { + return timing_ms >= Validation::kMinTimingMs; +} + +static bool IsValidStatusCode(int code) { + return code >= Validation::kMinStatusCode && + code <= Validation::kMaxStatusCode; +} + static std::vector ParseFatalStatusCodes(const std::string& str) { std::vector result; std::stringstream sstream(str); @@ -125,7 +145,11 @@ static std::vector ParseFatalStatusCodes(const std::string& str) { try { int code = std::stoi(item); - result.push_back(code); + if (IsValidStatusCode(code)) { + result.push_back(code); + } else { + LOG(WARNING) << "Invalid gRPC status code: " << code; + } continue; } catch (const std::invalid_argument&) { // Not an integer, try parsing as a status code string. @@ -144,43 +168,53 @@ static std::vector ParseFatalStatusCodes(const std::string& str) { } FlagdProviderConfig::FlagdProviderConfig() - : host_(GetEnvStr(EnvVars::kHost, Defaults::kHost)), - port_(GetEnvInt(EnvVars::kPort, Defaults::kPortInProcess)), - tls_(GetEnvBool(EnvVars::kTls, Defaults::kTls)), - deadline_ms_(GetEnvInt(EnvVars::kDeadlineMs, Defaults::kDeadlineMs)), - stream_deadline_ms_( - GetEnvInt(EnvVars::kStreamDeadlineMs, Defaults::kStreamDeadlineMs)), - retry_backoff_ms_( - GetEnvInt(EnvVars::kRetryBackoffMs, Defaults::kRetryBackoffMs)), - retry_backoff_max_ms_( - GetEnvInt(EnvVars::kRetryBackoffMaxMs, Defaults::kRetryBackoffMaxMs)), - retry_grace_period_( - GetEnvInt(EnvVars::kRetryGracePeriod, Defaults::kRetryGracePeriod)), - keep_alive_time_ms_( - GetEnvInt(EnvVars::kKeepAliveTimeMs, Defaults::kKeepAliveTimeMs)), - offline_poll_interval_ms_( - GetEnvInt(EnvVars::kOfflinePollMs, Defaults::kOfflinePollMs)) { - if (std::string val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { - target_uri_ = val; + : host_(std::string(Defaults::kHost)), + port_(Defaults::kPortInProcess), + tls_(Defaults::kTls), + deadline_ms_(Defaults::kDeadlineMs), + stream_deadline_ms_(Defaults::kStreamDeadlineMs), + retry_backoff_ms_(Defaults::kRetryBackoffMs), + retry_backoff_max_ms_(Defaults::kRetryBackoffMaxMs), + retry_grace_period_(Defaults::kRetryGracePeriod), + keep_alive_time_ms_(Defaults::kKeepAliveTimeMs), + offline_poll_interval_ms_(Defaults::kOfflinePollMs) { + SetHost(GetEnvStr(EnvVars::kHost, Defaults::kHost)); + SetPort(GetEnvInt(EnvVars::kPort, Defaults::kPortInProcess)); + SetTls(GetEnvBool(EnvVars::kTls, Defaults::kTls)); + SetDeadlineMs(GetEnvInt(EnvVars::kDeadlineMs, Defaults::kDeadlineMs)); + SetStreamDeadlineMs( + GetEnvInt(EnvVars::kStreamDeadlineMs, Defaults::kStreamDeadlineMs)); + SetRetryBackoffMs( + GetEnvInt(EnvVars::kRetryBackoffMs, Defaults::kRetryBackoffMs)); + SetRetryBackoffMaxMs( + GetEnvInt(EnvVars::kRetryBackoffMaxMs, Defaults::kRetryBackoffMaxMs)); + SetRetryGracePeriod( + GetEnvInt(EnvVars::kRetryGracePeriod, Defaults::kRetryGracePeriod)); + SetKeepAliveTimeMs( + GetEnvInt(EnvVars::kKeepAliveTimeMs, Defaults::kKeepAliveTimeMs)); + SetOfflinePollIntervalMs( + GetEnvInt(EnvVars::kOfflinePollMs, Defaults::kOfflinePollMs)); + + if (auto val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { + SetTargetUri(val); } - if (std::string val = GetEnvStr(EnvVars::kSocketPath); !val.empty()) { - socket_path_ = val; + if (auto val = GetEnvStr(EnvVars::kSocketPath); !val.empty()) { + SetSocketPath(val); } - if (std::string val = GetEnvStr(EnvVars::kServerCertPath); !val.empty()) { - cert_path_ = val; + if (auto val = GetEnvStr(EnvVars::kServerCertPath); !val.empty()) { + SetCertPath(val); } - if (std::string val = GetEnvStr(EnvVars::kSourceSelector); !val.empty()) { - selector_ = val; + if (auto val = GetEnvStr(EnvVars::kSourceSelector); !val.empty()) { + SetSelector(val); } - if (std::string val = GetEnvStr(EnvVars::kProviderId); !val.empty()) { - provider_id_ = val; + if (auto val = GetEnvStr(EnvVars::kProviderId); !val.empty()) { + SetProviderId(val); } - if (std::string val = GetEnvStr(EnvVars::kOfflineFlagSourcePath); - !val.empty()) { - offline_flag_source_path_ = val; + if (auto val = GetEnvStr(EnvVars::kOfflineFlagSourcePath); !val.empty()) { + SetOfflineFlagSourcePath(val); } - if (std::string val = GetEnvStr(EnvVars::kFatalStatusCodes); !val.empty()) { - fatal_status_codes_ = ParseFatalStatusCodes(val); + if (auto val = GetEnvStr(EnvVars::kFatalStatusCodes); !val.empty()) { + SetFatalStatusCodes(val); } } @@ -301,6 +335,10 @@ FlagdProviderConfig& FlagdProviderConfig::SetHost(std::string_view host) { return *this; } FlagdProviderConfig& FlagdProviderConfig::SetPort(int port) { + if (!IsValidPort(port)) { + LOG(WARNING) << "Invalid port: " << port << ". Ignoring."; + return *this; + } port_ = port; return *this; } @@ -326,36 +364,71 @@ FlagdProviderConfig& FlagdProviderConfig::SetCertPath(std::string_view path) { return *this; } FlagdProviderConfig& FlagdProviderConfig::SetDeadlineMs(int deadline_ms) { + if (!IsValidTiming(deadline_ms)) { + LOG(WARNING) << "Invalid deadline_ms: " << deadline_ms << ". Ignoring."; + return *this; + } deadline_ms_ = deadline_ms; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetStreamDeadlineMs( int stream_deadline_ms) { + if (!IsValidTiming(stream_deadline_ms)) { + LOG(WARNING) << "Invalid stream_deadline_ms: " << stream_deadline_ms + << ". Ignoring."; + return *this; + } stream_deadline_ms_ = stream_deadline_ms; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMs( int retry_backoff_ms) { + if (!IsValidTiming(retry_backoff_ms)) { + LOG(WARNING) << "Invalid retry_backoff_ms: " << retry_backoff_ms + << ". Ignoring."; + return *this; + } retry_backoff_ms_ = retry_backoff_ms; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMaxMs( int retry_backoff_max_ms) { + if (!IsValidTiming(retry_backoff_max_ms)) { + LOG(WARNING) << "Invalid retry_backoff_max_ms: " << retry_backoff_max_ms + << ". Ignoring."; + return *this; + } retry_backoff_max_ms_ = retry_backoff_max_ms; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetRetryGracePeriod( int retry_grace_period) { + if (!IsValidTiming(retry_grace_period)) { + LOG(WARNING) << "Invalid retry_grace_period: " << retry_grace_period + << ". Ignoring."; + return *this; + } retry_grace_period_ = retry_grace_period; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetKeepAliveTimeMs( int keep_alive_time_ms) { + if (!IsValidTiming(keep_alive_time_ms)) { + LOG(WARNING) << "Invalid keep_alive_time_ms: " << keep_alive_time_ms + << ". Ignoring."; + return *this; + } keep_alive_time_ms_ = keep_alive_time_ms; return *this; } FlagdProviderConfig& FlagdProviderConfig::SetFatalStatusCodes( const std::vector& fatal_status_codes) { + for (int code : fatal_status_codes) { + if (!IsValidStatusCode(code)) { + LOG(WARNING) << "Invalid gRPC status code: " << code << ". Ignoring."; + return *this; + } + } fatal_status_codes_ = fatal_status_codes; return *this; } @@ -381,6 +454,11 @@ FlagdProviderConfig& FlagdProviderConfig::SetOfflineFlagSourcePath( } FlagdProviderConfig& FlagdProviderConfig::SetOfflinePollIntervalMs( int interval_ms) { + if (!IsValidTiming(interval_ms)) { + LOG(WARNING) << "Invalid offline_poll_interval_ms: " << interval_ms + << ". Ignoring."; + return *this; + } offline_poll_interval_ms_ = interval_ms; return *this; } diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index 2371d2c..7dc5c09 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -265,8 +265,11 @@ void GrpcSync::WaitForUpdates() { } int64_t backoff = config_.GetRetryBackoffMs() * (1LL << retry_count); - backoff = std::min(backoff, config_.GetRetryBackoffMaxMs()); - retry_count++; + if (backoff >= config_.GetRetryBackoffMaxMs()) { + backoff = config_.GetRetryBackoffMaxMs(); + } else { + retry_count++; + } auto now = std::chrono::steady_clock::now(); auto disconnected_duration = diff --git a/providers/flagd/tests/configuration_test.cpp b/providers/flagd/tests/configuration_test.cpp index 63e20df..fe6a8c3 100644 --- a/providers/flagd/tests/configuration_test.cpp +++ b/providers/flagd/tests/configuration_test.cpp @@ -71,6 +71,56 @@ TEST_F(ConfigurationTest, EffectiveTargetUriPrecedence) { EXPECT_EQ(config.GetEffectiveTargetUri(), target_uri); } +TEST_F(ConfigurationTest, InvalidValues) { + FlagdProviderConfig config; + + // Invalid Port + int original_port = config.GetPort(); + config.SetPort(-1); + EXPECT_EQ(config.GetPort(), original_port); + config.SetPort(65536); + EXPECT_EQ(config.GetPort(), original_port); + + // Invalid Timings + int original_deadline = config.GetDeadlineMs(); + config.SetDeadlineMs(-1); + EXPECT_EQ(config.GetDeadlineMs(), original_deadline); + + int original_stream_deadline = config.GetStreamDeadlineMs(); + config.SetStreamDeadlineMs(-1); + EXPECT_EQ(config.GetStreamDeadlineMs(), original_stream_deadline); + + int original_retry_backoff = config.GetRetryBackoffMs(); + config.SetRetryBackoffMs(-1); + EXPECT_EQ(config.GetRetryBackoffMs(), original_retry_backoff); + + int original_retry_backoff_max = config.GetRetryBackoffMaxMs(); + config.SetRetryBackoffMaxMs(-1); + EXPECT_EQ(config.GetRetryBackoffMaxMs(), original_retry_backoff_max); + + int original_retry_grace = config.GetRetryGracePeriod(); + config.SetRetryGracePeriod(-1); + EXPECT_EQ(config.GetRetryGracePeriod(), original_retry_grace); + + int original_keep_alive = config.GetKeepAliveTimeMs(); + config.SetKeepAliveTimeMs(-1); + EXPECT_EQ(config.GetKeepAliveTimeMs(), original_keep_alive); + + int original_offline_poll = config.GetOfflinePollIntervalMs(); + config.SetOfflinePollIntervalMs(-1); + EXPECT_EQ(config.GetOfflinePollIntervalMs(), original_offline_poll); + + // Invalid Fatal Status Codes + config.SetFatalStatusCodes(std::vector{1, 100, 5}); // 100 is invalid + EXPECT_EQ(config.GetFatalStatusCodes().size(), + 0); // Should be empty as it was ignored + + config.SetFatalStatusCodes("1,INVALID,5"); // INVALID is invalid + EXPECT_EQ(config.GetFatalStatusCodes().size(), 2); + EXPECT_EQ(config.GetFatalStatusCodes()[0], 1); + EXPECT_EQ(config.GetFatalStatusCodes()[1], 5); +} + TEST_F(ConfigurationTest, GetEffectiveCredentialsInsecure) { FlagdProviderConfig config; config.SetTls(false); From 8f1123c0e318935da35805001cb833fab67960f3 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Wed, 18 Mar 2026 14:05:18 +0000 Subject: [PATCH 5/8] Fixed errors Signed-off-by: Marcin Olko --- providers/flagd/src/configuration.cpp | 40 ++++++--------------------- providers/flagd/src/configuration.h | 34 ++++++++++++++++------- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 63f9b1a..815724d 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -48,19 +48,6 @@ struct EnvVars { static constexpr std::string_view kOfflinePollMs = "FLAGD_OFFLINE_POLL_MS"; }; -struct Defaults { - static constexpr std::string_view kHost = "localhost"; - static constexpr int kPortInProcess = 8015; - static constexpr bool kTls = false; - static constexpr int kDeadlineMs = 500; - static constexpr int kStreamDeadlineMs = 600000; - static constexpr int kRetryBackoffMs = 1000; - static constexpr int kRetryBackoffMaxMs = 12000; - static constexpr int kRetryGracePeriod = 5; - static constexpr int kKeepAliveTimeMs = 0; - static constexpr int kOfflinePollMs = 5000; -}; - struct Validation { static constexpr int kMinPort = 1; static constexpr int kMaxPort = 65535; @@ -167,17 +154,7 @@ static std::vector ParseFatalStatusCodes(const std::string& str) { return result; } -FlagdProviderConfig::FlagdProviderConfig() - : host_(std::string(Defaults::kHost)), - port_(Defaults::kPortInProcess), - tls_(Defaults::kTls), - deadline_ms_(Defaults::kDeadlineMs), - stream_deadline_ms_(Defaults::kStreamDeadlineMs), - retry_backoff_ms_(Defaults::kRetryBackoffMs), - retry_backoff_max_ms_(Defaults::kRetryBackoffMaxMs), - retry_grace_period_(Defaults::kRetryGracePeriod), - keep_alive_time_ms_(Defaults::kKeepAliveTimeMs), - offline_poll_interval_ms_(Defaults::kOfflinePollMs) { +FlagdProviderConfig::FlagdProviderConfig() { SetHost(GetEnvStr(EnvVars::kHost, Defaults::kHost)); SetPort(GetEnvInt(EnvVars::kPort, Defaults::kPortInProcess)); SetTls(GetEnvBool(EnvVars::kTls, Defaults::kTls)); @@ -195,25 +172,26 @@ FlagdProviderConfig::FlagdProviderConfig() SetOfflinePollIntervalMs( GetEnvInt(EnvVars::kOfflinePollMs, Defaults::kOfflinePollMs)); - if (auto val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kTargetUri); !val.empty()) { SetTargetUri(val); } - if (auto val = GetEnvStr(EnvVars::kSocketPath); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kSocketPath); !val.empty()) { SetSocketPath(val); } - if (auto val = GetEnvStr(EnvVars::kServerCertPath); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kServerCertPath); !val.empty()) { SetCertPath(val); } - if (auto val = GetEnvStr(EnvVars::kSourceSelector); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kSourceSelector); !val.empty()) { SetSelector(val); } - if (auto val = GetEnvStr(EnvVars::kProviderId); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kProviderId); !val.empty()) { SetProviderId(val); } - if (auto val = GetEnvStr(EnvVars::kOfflineFlagSourcePath); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kOfflineFlagSourcePath); + !val.empty()) { SetOfflineFlagSourcePath(val); } - if (auto val = GetEnvStr(EnvVars::kFatalStatusCodes); !val.empty()) { + if (std::string val = GetEnvStr(EnvVars::kFatalStatusCodes); !val.empty()) { SetFatalStatusCodes(val); } } diff --git a/providers/flagd/src/configuration.h b/providers/flagd/src/configuration.h index f276ba6..ef35549 100644 --- a/providers/flagd/src/configuration.h +++ b/providers/flagd/src/configuration.h @@ -5,12 +5,26 @@ #include #include #include +#include #include "absl/status/statusor.h" #include "grpcpp/security/credentials.h" namespace flagd { +struct Defaults { + static constexpr std::string_view kHost = "localhost"; + static constexpr int kPortInProcess = 8015; + static constexpr bool kTls = false; + static constexpr int kDeadlineMs = 500; + static constexpr int kStreamDeadlineMs = 600000; + static constexpr int kRetryBackoffMs = 1000; + static constexpr int kRetryBackoffMaxMs = 12000; + static constexpr int kRetryGracePeriod = 5; + static constexpr int kKeepAliveTimeMs = 0; + static constexpr int kOfflinePollMs = 5000; +}; + class FlagdProviderConfig { public: // --- Constructor --- @@ -78,27 +92,27 @@ class FlagdProviderConfig { FlagdProviderConfig& SetOfflinePollIntervalMs(int interval_ms); private: - std::string host_; - int port_; + std::string host_ = std::string(Defaults::kHost); + int port_ = Defaults::kPortInProcess; std::optional target_uri_; - bool tls_; + bool tls_ = Defaults::kTls; std::shared_ptr channel_credentials_; std::optional socket_path_; std::optional cert_path_; - int deadline_ms_; - int stream_deadline_ms_; - int retry_backoff_ms_; - int retry_backoff_max_ms_; - int retry_grace_period_; - int keep_alive_time_ms_; + int deadline_ms_ = Defaults::kDeadlineMs; + int stream_deadline_ms_ = Defaults::kStreamDeadlineMs; + int retry_backoff_ms_ = Defaults::kRetryBackoffMs; + int retry_backoff_max_ms_ = Defaults::kRetryBackoffMaxMs; + int retry_grace_period_ = Defaults::kRetryGracePeriod; + int keep_alive_time_ms_ = Defaults::kKeepAliveTimeMs; std::vector fatal_status_codes_; std::optional selector_; std::optional provider_id_; std::optional offline_flag_source_path_; - int offline_poll_interval_ms_; + int offline_poll_interval_ms_ = Defaults::kOfflinePollMs; }; } // namespace flagd From b30b55795e6eae465a39057931e6c2335d5d4d4a Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Wed, 18 Mar 2026 14:38:34 +0000 Subject: [PATCH 6/8] Fixed inconsistent behavior of setting status codes and improved checks for timings Signed-off-by: Marcin Olko --- providers/flagd/src/configuration.cpp | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/providers/flagd/src/configuration.cpp b/providers/flagd/src/configuration.cpp index 815724d..f00b21f 100644 --- a/providers/flagd/src/configuration.cpp +++ b/providers/flagd/src/configuration.cpp @@ -51,7 +51,6 @@ struct EnvVars { struct Validation { static constexpr int kMinPort = 1; static constexpr int kMaxPort = 65535; - static constexpr int kMinTimingMs = 0; static constexpr int kMinStatusCode = 0; static constexpr int kMaxStatusCode = 16; }; @@ -111,10 +110,6 @@ static bool IsValidPort(int port) { return port >= Validation::kMinPort && port <= Validation::kMaxPort; } -static bool IsValidTiming(int timing_ms) { - return timing_ms >= Validation::kMinTimingMs; -} - static bool IsValidStatusCode(int code) { return code >= Validation::kMinStatusCode && code <= Validation::kMaxStatusCode; @@ -342,7 +337,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetCertPath(std::string_view path) { return *this; } FlagdProviderConfig& FlagdProviderConfig::SetDeadlineMs(int deadline_ms) { - if (!IsValidTiming(deadline_ms)) { + if (deadline_ms <= 0) { LOG(WARNING) << "Invalid deadline_ms: " << deadline_ms << ". Ignoring."; return *this; } @@ -351,7 +346,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetDeadlineMs(int deadline_ms) { } FlagdProviderConfig& FlagdProviderConfig::SetStreamDeadlineMs( int stream_deadline_ms) { - if (!IsValidTiming(stream_deadline_ms)) { + if (stream_deadline_ms <= 0) { LOG(WARNING) << "Invalid stream_deadline_ms: " << stream_deadline_ms << ". Ignoring."; return *this; @@ -361,7 +356,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetStreamDeadlineMs( } FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMs( int retry_backoff_ms) { - if (!IsValidTiming(retry_backoff_ms)) { + if (retry_backoff_ms <= 0) { LOG(WARNING) << "Invalid retry_backoff_ms: " << retry_backoff_ms << ". Ignoring."; return *this; @@ -371,7 +366,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMs( } FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMaxMs( int retry_backoff_max_ms) { - if (!IsValidTiming(retry_backoff_max_ms)) { + if (retry_backoff_max_ms <= 0) { LOG(WARNING) << "Invalid retry_backoff_max_ms: " << retry_backoff_max_ms << ". Ignoring."; return *this; @@ -381,7 +376,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetRetryBackoffMaxMs( } FlagdProviderConfig& FlagdProviderConfig::SetRetryGracePeriod( int retry_grace_period) { - if (!IsValidTiming(retry_grace_period)) { + if (retry_grace_period < 0) { LOG(WARNING) << "Invalid retry_grace_period: " << retry_grace_period << ". Ignoring."; return *this; @@ -391,7 +386,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetRetryGracePeriod( } FlagdProviderConfig& FlagdProviderConfig::SetKeepAliveTimeMs( int keep_alive_time_ms) { - if (!IsValidTiming(keep_alive_time_ms)) { + if (keep_alive_time_ms < 0) { LOG(WARNING) << "Invalid keep_alive_time_ms: " << keep_alive_time_ms << ". Ignoring."; return *this; @@ -401,13 +396,15 @@ FlagdProviderConfig& FlagdProviderConfig::SetKeepAliveTimeMs( } FlagdProviderConfig& FlagdProviderConfig::SetFatalStatusCodes( const std::vector& fatal_status_codes) { + std::vector valid_codes; for (int code : fatal_status_codes) { - if (!IsValidStatusCode(code)) { + if (IsValidStatusCode(code)) { + valid_codes.push_back(code); + } else { LOG(WARNING) << "Invalid gRPC status code: " << code << ". Ignoring."; - return *this; } } - fatal_status_codes_ = fatal_status_codes; + fatal_status_codes_ = std::move(valid_codes); return *this; } FlagdProviderConfig& FlagdProviderConfig::SetFatalStatusCodes( @@ -432,7 +429,7 @@ FlagdProviderConfig& FlagdProviderConfig::SetOfflineFlagSourcePath( } FlagdProviderConfig& FlagdProviderConfig::SetOfflinePollIntervalMs( int interval_ms) { - if (!IsValidTiming(interval_ms)) { + if (interval_ms <= 0) { LOG(WARNING) << "Invalid offline_poll_interval_ms: " << interval_ms << ". Ignoring."; return *this; From e6d2677a8f538a9351ad3f8d041f85db311efeae Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Wed, 18 Mar 2026 15:00:17 +0000 Subject: [PATCH 7/8] Fixed tests and improved cleanup Signed-off-by: Marcin Olko --- providers/flagd/src/sync/grpc/grpc_sync.cpp | 28 +++++++++++++++----- providers/flagd/src/sync/grpc/grpc_sync.h | 10 ++++--- providers/flagd/tests/configuration_test.cpp | 9 ++++--- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index 7dc5c09..e3d9fc4 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -43,6 +43,8 @@ absl::Status GrpcSync::Init(const openfeature::EvaluationContext& ctx) { [this] { return state_ != State::kInitializing; })) { init_timed_out_ = true; init_result_ = absl::DeadlineExceededError("Initialization timed out"); + lock.unlock(); + static_cast(ShutdownInternal(State::kUninitialized)); return init_result_; } return (state_ == State::kReady) ? absl::OkStatus() : init_result_; @@ -97,10 +99,15 @@ absl::Status GrpcSync::Init(const openfeature::EvaluationContext& ctx) { [this] { return state_ != State::kInitializing; })) { init_timed_out_ = true; init_result_ = absl::DeadlineExceededError("Initialization timed out"); + lock.unlock(); + static_cast(ShutdownInternal(State::kUninitialized)); return init_result_; } if (state_ != State::kReady) { + State final_state = state_; + lock.unlock(); + static_cast(ShutdownInternal(final_state)); return init_result_; } @@ -108,6 +115,10 @@ absl::Status GrpcSync::Init(const openfeature::EvaluationContext& ctx) { } absl::Status GrpcSync::Shutdown() { + return ShutdownInternal(State::kUninitialized); +} + +absl::Status GrpcSync::ShutdownInternal(State target_state) { std::unique_lock lock(lifecycle_mutex_); if (state_ == State::kUninitialized) { @@ -121,11 +132,13 @@ absl::Status GrpcSync::Shutdown() { } State previous_state = state_; - state_ = State::kShuttingDown; - LOG(INFO) << "GrpcSync state changed to kShuttingDown"; + if (state_ != State::kFatal) { + state_ = State::kShuttingDown; + LOG(INFO) << "GrpcSync state changed to kShuttingDown"; - if (context_) { - context_->TryCancel(); + if (context_) { + context_->TryCancel(); + } } lock.unlock(); @@ -136,10 +149,11 @@ absl::Status GrpcSync::Shutdown() { context_.reset(); stub_.reset(); - state_ = State::kUninitialized; - LOG(INFO) << "GrpcSync state changed to kUninitialized"; + state_ = target_state; + LOG(INFO) << "GrpcSync state changed (ShutdownInternal)"; - if (previous_state == State::kInitializing) { + if (previous_state == State::kInitializing && + target_state == State::kUninitialized) { init_result_ = absl::CancelledError("Shutdown called during initialization"); } diff --git a/providers/flagd/src/sync/grpc/grpc_sync.h b/providers/flagd/src/sync/grpc/grpc_sync.h index 7538f83..ccf88d5 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.h +++ b/providers/flagd/src/sync/grpc/grpc_sync.h @@ -25,10 +25,6 @@ class GrpcSync final : public FlagSync { absl::Status Shutdown() override; private: - void WaitForUpdates(); - - std::unique_ptr stub_; - std::shared_ptr context_; enum class State : uint8_t { kUninitialized, kInitializing, // Thread started, waiting for first connection @@ -38,6 +34,12 @@ class GrpcSync final : public FlagSync { kFatal, // Fatal error, gives up }; + void WaitForUpdates(); + absl::Status ShutdownInternal(State target_state); + + std::unique_ptr stub_; + std::shared_ptr context_; + std::thread background_thread_; std::mutex lifecycle_mutex_; diff --git a/providers/flagd/tests/configuration_test.cpp b/providers/flagd/tests/configuration_test.cpp index fe6a8c3..b689668 100644 --- a/providers/flagd/tests/configuration_test.cpp +++ b/providers/flagd/tests/configuration_test.cpp @@ -112,13 +112,14 @@ TEST_F(ConfigurationTest, InvalidValues) { // Invalid Fatal Status Codes config.SetFatalStatusCodes(std::vector{1, 100, 5}); // 100 is invalid - EXPECT_EQ(config.GetFatalStatusCodes().size(), - 0); // Should be empty as it was ignored - - config.SetFatalStatusCodes("1,INVALID,5"); // INVALID is invalid EXPECT_EQ(config.GetFatalStatusCodes().size(), 2); EXPECT_EQ(config.GetFatalStatusCodes()[0], 1); EXPECT_EQ(config.GetFatalStatusCodes()[1], 5); + + config.SetFatalStatusCodes("2,INVALID,6"); // INVALID is invalid + EXPECT_EQ(config.GetFatalStatusCodes().size(), 2); + EXPECT_EQ(config.GetFatalStatusCodes()[0], 2); + EXPECT_EQ(config.GetFatalStatusCodes()[1], 6); } TEST_F(ConfigurationTest, GetEffectiveCredentialsInsecure) { From 10288f17bad7c32871a576a51e6da5d3116167e5 Mon Sep 17 00:00:00 2001 From: Marcin Olko Date: Thu, 19 Mar 2026 09:55:21 +0000 Subject: [PATCH 8/8] Improved tests and added issue number to TODO Signed-off-by: Marcin Olko --- providers/flagd/src/sync/grpc/grpc_sync.cpp | 26 +++++++++++--------- providers/flagd/tests/configuration_test.cpp | 21 +++++++++++++++- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index e3d9fc4..ca65df9 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -187,12 +187,14 @@ void GrpcSync::WaitForUpdates() { context_ = local_ctx; } if (config_.GetStreamDeadlineMs() > 0) { - auto deadline = std::chrono::system_clock::now() + - std::chrono::milliseconds(config_.GetStreamDeadlineMs()); + std::chrono::time_point deadline = + std::chrono::system_clock::now() + + std::chrono::milliseconds(config_.GetStreamDeadlineMs()); local_ctx->set_deadline(deadline); } - auto reader = stub_->SyncFlags(local_ctx.get(), request); + std::unique_ptr> reader = + stub_->SyncFlags(local_ctx.get(), request); if (!reader) { LOG(ERROR) << "Failed to create sync stream"; @@ -228,11 +230,11 @@ void GrpcSync::WaitForUpdates() { LOG(INFO) << "GrpcSync state changed to kReady"; init_result_ = absl::OkStatus(); lifecycle_cv_.notify_all(); - // TODO: emit PROVIDER_READY - // TODO: emit PROVIDER_CONFIGURATION_CHANGED + // TODO(#89): emit PROVIDER_READY + // TODO(#89): emit PROVIDER_CONFIGURATION_CHANGED } } - // TODO: emit PROVIDER_CONFIGURATION_CHANGED + // TODO(#89): emit PROVIDER_CONFIGURATION_CHANGED } catch (const std::exception& e) { LOG(ERROR) << "Failed to parse flag configuration: " << e.what(); } @@ -247,7 +249,7 @@ void GrpcSync::WaitForUpdates() { } // Check fatal status codes - const auto& fatal_codes = config_.GetFatalStatusCodes(); + const std::vector& fatal_codes = config_.GetFatalStatusCodes(); bool is_fatal = std::find(fatal_codes.cbegin(), fatal_codes.cend(), status.error_code()) != fatal_codes.cend(); @@ -259,7 +261,7 @@ void GrpcSync::WaitForUpdates() { init_result_ = absl::InternalError( absl::StrCat("Fatal gRPC error: ", status.error_message())); lifecycle_cv_.notify_all(); - // TODO: emit PROVIDER_FATAL + // TODO(#89): emit PROVIDER_FATAL break; } @@ -274,7 +276,7 @@ void GrpcSync::WaitForUpdates() { } else if (state_ == State::kReady) { state_ = State::kReconnecting; LOG(INFO) << "GrpcSync state changed to kReconnecting"; - // TODO: emit PROVIDER_STALE + // TODO(#89): emit PROVIDER_STALE } } @@ -285,13 +287,13 @@ void GrpcSync::WaitForUpdates() { retry_count++; } - auto now = std::chrono::steady_clock::now(); - auto disconnected_duration = + std::chrono::time_point now = std::chrono::steady_clock::now(); + std::chrono::duration disconnected_duration = std::chrono::duration_cast(now - last_healthy_time); if (disconnected_duration.count() > config_.GetRetryGracePeriod()) { ClearFlags(); - // TODO: emit PROVIDER_ERROR + // TODO(#89): emit PROVIDER_ERROR } { diff --git a/providers/flagd/tests/configuration_test.cpp b/providers/flagd/tests/configuration_test.cpp index b689668..02b741b 100644 --- a/providers/flagd/tests/configuration_test.cpp +++ b/providers/flagd/tests/configuration_test.cpp @@ -109,17 +109,36 @@ TEST_F(ConfigurationTest, InvalidValues) { int original_offline_poll = config.GetOfflinePollIntervalMs(); config.SetOfflinePollIntervalMs(-1); EXPECT_EQ(config.GetOfflinePollIntervalMs(), original_offline_poll); +} + +TEST_F(ConfigurationTest, FatalStatusCodes) { + FlagdProviderConfig config; - // Invalid Fatal Status Codes + // Invalid Fatal Status Codes (Integers) config.SetFatalStatusCodes(std::vector{1, 100, 5}); // 100 is invalid EXPECT_EQ(config.GetFatalStatusCodes().size(), 2); EXPECT_EQ(config.GetFatalStatusCodes()[0], 1); EXPECT_EQ(config.GetFatalStatusCodes()[1], 5); + // Invalid Fatal Status Codes (Strings) config.SetFatalStatusCodes("2,INVALID,6"); // INVALID is invalid EXPECT_EQ(config.GetFatalStatusCodes().size(), 2); EXPECT_EQ(config.GetFatalStatusCodes()[0], 2); EXPECT_EQ(config.GetFatalStatusCodes()[1], 6); + + // String names for fatal status codes + config.SetFatalStatusCodes("DEADLINE_EXCEEDED,NOT_FOUND,INVALID_ARGUMENT"); + EXPECT_EQ(config.GetFatalStatusCodes().size(), 3); + EXPECT_EQ(config.GetFatalStatusCodes()[0], 4); + EXPECT_EQ(config.GetFatalStatusCodes()[1], 5); + EXPECT_EQ(config.GetFatalStatusCodes()[2], 3); + + // Mixed string names and integers + config.SetFatalStatusCodes("1,INTERNAL,3"); + EXPECT_EQ(config.GetFatalStatusCodes().size(), 3); + EXPECT_EQ(config.GetFatalStatusCodes()[0], 1); + EXPECT_EQ(config.GetFatalStatusCodes()[1], 13); + EXPECT_EQ(config.GetFatalStatusCodes()[2], 3); } TEST_F(ConfigurationTest, GetEffectiveCredentialsInsecure) {