From 4701ab13e528535cc06fb1f6b481fec3cf6f6add Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 15:00:50 -0700 Subject: [PATCH 1/9] feat: implement DynamoDBDataSource + tests --- .github/workflows/server-dynamodb.yml | 19 +- .../server-sdk-dynamodb-source/CMakeLists.txt | 4 + libs/server-sdk-dynamodb-source/README.md | 3 +- .../integrations/dynamodb/dynamodb_source.hpp | 79 ++- .../integrations/dynamodb/options.hpp | 36 ++ .../src/CMakeLists.txt | 2 + .../src/aws_sdk_guard.cpp | 18 + .../src/aws_sdk_guard.hpp | 38 ++ .../src/client_factory.cpp | 58 ++ .../src/client_factory.hpp | 21 + .../src/dynamodb_attributes.hpp | 22 + .../src/dynamodb_source.cpp | 142 ++++- .../server-sdk-dynamodb-source/src/prefix.hpp | 15 + .../tests/CMakeLists.txt | 37 ++ .../tests/dynamodb_source_test.cpp | 509 ++++++++++++++++++ .../tests/prefixed_dynamodb_client.hpp | 148 +++++ 16 files changed, 1131 insertions(+), 20 deletions(-) create mode 100644 libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/options.hpp create mode 100644 libs/server-sdk-dynamodb-source/src/aws_sdk_guard.cpp create mode 100644 libs/server-sdk-dynamodb-source/src/aws_sdk_guard.hpp create mode 100644 libs/server-sdk-dynamodb-source/src/client_factory.cpp create mode 100644 libs/server-sdk-dynamodb-source/src/client_factory.hpp create mode 100644 libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp create mode 100644 libs/server-sdk-dynamodb-source/src/prefix.hpp create mode 100644 libs/server-sdk-dynamodb-source/tests/CMakeLists.txt create mode 100644 libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp create mode 100644 libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp diff --git a/.github/workflows/server-dynamodb.yml b/.github/workflows/server-dynamodb.yml index 85f46b2ed..6fc39fdee 100644 --- a/.github/workflows/server-dynamodb.yml +++ b/.github/workflows/server-dynamodb.yml @@ -14,19 +14,22 @@ on: - cron: '0 8 * * *' jobs: - build-dynamodb: + build-test-dynamodb: runs-on: ubuntu-22.04 + services: + dynamodb: + image: amazon/dynamodb-local + ports: + - 8000:8000 steps: # https://github.com/actions/checkout/releases/tag/v4.3.0 - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 - uses: ./.github/actions/ci with: cmake_target: launchdarkly-cpp-server-dynamodb-source - # No tests yet; PR 1 is scaffold-only and proves the AWS SDK builds. - run_tests: false # AWS C++ SDK requires libcurl at link time on Linux/macOS. install_curl: true - simulate_release: false + simulate_release: true build-dynamodb-mac: runs-on: macos-15 steps: @@ -36,9 +39,9 @@ jobs: with: cmake_target: launchdarkly-cpp-server-dynamodb-source platform_version: 12 - run_tests: false + run_tests: false # TODO: figure out how to run dynamodb-local on Mac install_curl: true - simulate_release: false + simulate_release: true build-dynamodb-windows: runs-on: windows-2022 steps: @@ -55,6 +58,6 @@ jobs: cmake_target: launchdarkly-cpp-server-dynamodb-source platform_version: 2022 toolset: msvc - run_tests: false + run_tests: false # TODO: figure out how to run dynamodb-local on Windows install_curl: true - simulate_windows_release: false + simulate_windows_release: true diff --git a/libs/server-sdk-dynamodb-source/CMakeLists.txt b/libs/server-sdk-dynamodb-source/CMakeLists.txt index bdba030a8..5fc2eefec 100644 --- a/libs/server-sdk-dynamodb-source/CMakeLists.txt +++ b/libs/server-sdk-dynamodb-source/CMakeLists.txt @@ -27,3 +27,7 @@ include(FetchContent) include(${CMAKE_FILES}/aws-sdk-cpp.cmake) add_subdirectory(src) + +if (LD_BUILD_UNIT_TESTS) + add_subdirectory(tests) +endif () diff --git a/libs/server-sdk-dynamodb-source/README.md b/libs/server-sdk-dynamodb-source/README.md index 5f0346fec..8732b1e8a 100644 --- a/libs/server-sdk-dynamodb-source/README.md +++ b/libs/server-sdk-dynamodb-source/README.md @@ -10,8 +10,7 @@ This component will allow the Server-Side SDK to retrieve feature flag configura from LaunchDarkly. > [!NOTE] -> This library currently contains only scaffolding. The functional `DynamoDBDataSource` and Big Segments store -> implementation will land in subsequent releases. +> The Big Segments store implementation will land in a subsequent release. LaunchDarkly overview ------------------------- diff --git a/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp index 936b36dd9..6a436bd6f 100644 --- a/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp +++ b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp @@ -1,10 +1,81 @@ +/** @file dynamodb_source.hpp + * @brief Server-Side DynamoDB Source + */ + #pragma once +#include +#include + +#include + +#include +#include + +namespace Aws::DynamoDB { +class DynamoDBClient; +} + namespace launchdarkly::server_side::integrations { -// Scaffold-only entry point. The real DynamoDBDataSource class will replace -// this in a subsequent PR; this declaration exists so the smoke .cpp has -// something to define and the library produces a non-empty archive. -void DynamoDBSourceLinkSmoke(); +/** + * @brief DynamoDBDataSource represents a data source for the Server-Side SDK + * backed by Amazon DynamoDB. It is meant to be used in place of the standard + * LaunchDarkly Streaming or Polling data sources. + * + * Call DynamoDBDataSource::Create to obtain a new instance. This instance can + * be passed into the SDK's DataSystem configuration via the LazyLoad builder. + * + * The DynamoDB table must already exist and follow the LaunchDarkly schema: + * a String partition key named `namespace` and a String sort key named `key`. + * The LaunchDarkly Relay Proxy populates the table with this schema; this + * class only reads from it. + * + * This implementation is backed by the AWS SDK for C++. + */ +class DynamoDBDataSource final : public ISerializedDataReader { + public: + /** + * @brief Creates a new DynamoDBDataSource, or returns an error if + * construction failed. + * + * @param table_name Name of the DynamoDB table to read from. The table + * must already exist; this class does not create it. + * + * @param prefix Optional namespace prefix. When non-empty, the source + * reads rows whose partition key is `:features`, + * `:segments`, etc. This allows multiple LaunchDarkly + * environments to share a single table. + * + * @param options Optional AWS DynamoDB client configuration. See + * @ref DynamoDBClientOptions. When defaulted, the AWS SDK resolves + * region, endpoint, and credentials from the standard provider chain + * (environment variables, shared config files, instance metadata). + * + * @return A DynamoDBDataSource, or an error if construction failed. + */ + static tl::expected, std::string> + Create(std::string table_name, + std::string prefix, + DynamoDBClientOptions options = {}); + + [[nodiscard]] GetResult Get(ISerializedItemKind const& kind, + std::string const& itemKey) const override; + [[nodiscard]] AllResult All(ISerializedItemKind const& kind) const override; + [[nodiscard]] std::string const& Identity() const override; + [[nodiscard]] bool Initialized() const override; + + ~DynamoDBDataSource() override; + + private: + DynamoDBDataSource(std::shared_ptr client, + std::string table_name, + std::string prefix); + + std::shared_ptr client_; + std::string const table_name_; + std::string const prefix_; + std::string const inited_namespace_; +}; } // namespace launchdarkly::server_side::integrations diff --git a/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/options.hpp b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/options.hpp new file mode 100644 index 000000000..c7d846388 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/options.hpp @@ -0,0 +1,36 @@ +/** @file options.hpp + * @brief Options for constructing a DynamoDB-backed integration. + */ + +#pragma once + +#include +#include + +namespace launchdarkly::server_side::integrations { + +/** + * @brief Optional knobs for constructing the AWS DynamoDB client used by + * @ref DynamoDBDataSource (and other DynamoDB-backed integrations). + * + * When unset, fields fall through to the AWS SDK's defaults: + * + * - @ref region resolves via the SDK region provider chain (environment, + * shared config file, instance metadata). + * - @ref endpoint defaults to the standard AWS DynamoDB endpoint for the + * resolved region. Set it to point at DynamoDB Local or LocalStack, e.g. + * `http://localhost:8000`. + * - If none of @ref aws_access_key_id / @ref aws_secret_access_key / + * @ref aws_session_token are set, the SDK's default credential provider + * chain is used (environment variables, shared credentials file, EC2/ECS + * roles). + */ +struct DynamoDBClientOptions { + std::optional region; + std::optional endpoint; + std::optional aws_access_key_id; + std::optional aws_secret_access_key; + std::optional aws_session_token; +}; + +} // namespace launchdarkly::server_side::integrations diff --git a/libs/server-sdk-dynamodb-source/src/CMakeLists.txt b/libs/server-sdk-dynamodb-source/src/CMakeLists.txt index 6b55f0a1e..af94ae3b4 100644 --- a/libs/server-sdk-dynamodb-source/src/CMakeLists.txt +++ b/libs/server-sdk-dynamodb-source/src/CMakeLists.txt @@ -14,6 +14,8 @@ target_sources(${LIBNAME} PRIVATE ${HEADER_LIST} dynamodb_source.cpp + aws_sdk_guard.cpp + client_factory.cpp ) diff --git a/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.cpp b/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.cpp new file mode 100644 index 000000000..b767fe9eb --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.cpp @@ -0,0 +1,18 @@ +#include "aws_sdk_guard.hpp" + +namespace launchdarkly::server_side::integrations::detail { + +void AwsSdkGuard::Ensure() { + static AwsSdkGuard instance; + (void)instance; +} + +AwsSdkGuard::AwsSdkGuard() { + Aws::InitAPI(options_); +} + +AwsSdkGuard::~AwsSdkGuard() { + Aws::ShutdownAPI(options_); +} + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.hpp b/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.hpp new file mode 100644 index 000000000..5ed649ca2 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/aws_sdk_guard.hpp @@ -0,0 +1,38 @@ +#pragma once + +#include + +namespace launchdarkly::server_side::integrations::detail { + +// AwsSdkGuard owns the process-wide Aws::InitAPI / Aws::ShutdownAPI lifecycle +// for this library. Multiple DynamoDB-backed integrations within the same +// process share the single static instance; the API is initialized lazily on +// first use and torn down during normal program termination via C++ static +// destruction. +// +// Static-destruction ordering caveat: if a caller stashes a raw AWS SDK +// pointer in their own static and that static is destroyed AFTER this guard, +// AWS SDK calls during that destructor will be undefined. The standard usage +// pattern (holding the data source / store via a unique_ptr or shared_ptr in +// regular program scope, not in another static) is unaffected because those +// smart pointers destruct before the guard. +class AwsSdkGuard { + public: + // Idempotent. First call constructs the singleton, which runs + // Aws::InitAPI in its constructor. Subsequent calls are no-ops. Safe to + // call from any thread. + static void Ensure(); + + AwsSdkGuard(AwsSdkGuard const&) = delete; + AwsSdkGuard(AwsSdkGuard&&) = delete; + AwsSdkGuard& operator=(AwsSdkGuard const&) = delete; + AwsSdkGuard& operator=(AwsSdkGuard&&) = delete; + + private: + AwsSdkGuard(); + ~AwsSdkGuard(); + + Aws::SDKOptions options_; +}; + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.cpp b/libs/server-sdk-dynamodb-source/src/client_factory.cpp new file mode 100644 index 000000000..9068dc71f --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/client_factory.cpp @@ -0,0 +1,58 @@ +#include "client_factory.hpp" + +#include +#include +#include + +namespace launchdarkly::server_side::integrations::detail { + +namespace { + +bool HasExplicitCredentials(DynamoDBClientOptions const& options) { + return options.aws_access_key_id.has_value() || + options.aws_secret_access_key.has_value() || + options.aws_session_token.has_value(); +} + +Aws::Client::ClientConfiguration BuildConfig( + DynamoDBClientOptions const& options) { + Aws::Client::ClientConfiguration config; + + if (options.region) { + config.region = *options.region; + } + + if (options.endpoint) { + config.endpointOverride = *options.endpoint; + // Use HTTP if the endpoint starts with "http://"; otherwise default + // to HTTPS. Endpoint overrides are commonly DynamoDB Local or + // LocalStack on plain HTTP for development. + std::string const& ep = *options.endpoint; + if (ep.rfind("http://", 0) == 0) { + config.scheme = Aws::Http::Scheme::HTTP; + config.verifySSL = false; + } + } + + return config; +} + +} // namespace + +std::shared_ptr BuildDynamoDBClient( + DynamoDBClientOptions const& options) { + auto const config = BuildConfig(options); + + if (HasExplicitCredentials(options)) { + Aws::Auth::AWSCredentials credentials{ + options.aws_access_key_id.value_or(""), + options.aws_secret_access_key.value_or(""), + options.aws_session_token.value_or("")}; + return std::make_shared(credentials, + config); + } + + return std::make_shared(config); +} + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.hpp b/libs/server-sdk-dynamodb-source/src/client_factory.hpp new file mode 100644 index 000000000..a93fb85d9 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/client_factory.hpp @@ -0,0 +1,21 @@ +#pragma once + +#include + +#include + +#include + +namespace launchdarkly::server_side::integrations::detail { + +// Builds an Aws::DynamoDB::DynamoDBClient configured from the supplied +// DynamoDBClientOptions. Caller is responsible for ensuring AwsSdkGuard::Ensure() +// has been called first. +// +// The shared_ptr return enables future sharing of a single client across +// multiple DynamoDB-backed stores in the same process (e.g. data source + +// big-segment store); today each consumer constructs its own. +std::shared_ptr BuildDynamoDBClient( + DynamoDBClientOptions const& options); + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp b/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp new file mode 100644 index 000000000..2057fe1a1 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp @@ -0,0 +1,22 @@ +#pragma once + +namespace launchdarkly::server_side::integrations::detail { + +// Schema constants for the LaunchDarkly DynamoDB table layout, shared with +// other DynamoDB-backed integrations in this library. +// +// The schema is defined in sdk-specs/specs/PS-persistent-store/README.md ยง +// DynamoDB schema and is the same layout that the LaunchDarkly Relay Proxy +// writes; mismatching any of these strings means the SDK and Relay will not +// agree on what's in the table. +inline constexpr char kPartitionKey[] = "namespace"; +inline constexpr char kSortKey[] = "key"; +inline constexpr char kVersionAttribute[] = "version"; +inline constexpr char kItemAttribute[] = "item"; + +// Sentinel namespace marking the table as initialized. When a prefix is set, +// both the partition-key value AND the sort-key value are prefixed: +// {namespace: "myprefix:$inited", key: "myprefix:$inited"}. +inline constexpr char kInitedNamespace[] = "$inited"; + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp index a764141ac..3d276d3ad 100644 --- a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp @@ -1,15 +1,145 @@ #include +#include "aws_sdk_guard.hpp" +#include "client_factory.hpp" +#include "dynamodb_attributes.hpp" +#include "prefix.hpp" + +#include #include +#include +#include +#include + +#include +#include namespace launchdarkly::server_side::integrations { -// Touch an Aws::DynamoDB type so the linker actually pulls in the AWS SDK and -// we prove the dependency wires up. This function is intentionally never -// called; it exists solely to validate the CMake/CI scaffolding. -void DynamoDBSourceLinkSmoke() { - Aws::DynamoDB::DynamoDBClient* unused = nullptr; - (void)unused; +namespace { + +using detail::kInitedNamespace; +using detail::kItemAttribute; +using detail::kPartitionKey; +using detail::kSortKey; +using detail::PrefixedNamespace; + +} // namespace + +tl::expected, std::string> +DynamoDBDataSource::Create(std::string table_name, + std::string prefix, + DynamoDBClientOptions options) { + try { + detail::AwsSdkGuard::Ensure(); + auto client = detail::BuildDynamoDBClient(options); + return std::unique_ptr(new DynamoDBDataSource( + std::move(client), std::move(table_name), std::move(prefix))); + } catch (std::exception const& e) { + return tl::make_unexpected(e.what()); + } +} + +DynamoDBDataSource::DynamoDBDataSource( + std::shared_ptr client, + std::string table_name, + std::string prefix) + : client_(std::move(client)), + table_name_(std::move(table_name)), + prefix_(std::move(prefix)), + inited_namespace_(PrefixedNamespace(prefix_, kInitedNamespace)) {} + +DynamoDBDataSource::~DynamoDBDataSource() = default; + +ISerializedDataReader::GetResult DynamoDBDataSource::Get( + ISerializedItemKind const& kind, + std::string const& itemKey) const { + Aws::DynamoDB::Model::GetItemRequest request; + request.SetTableName(table_name_); + request.SetConsistentRead(true); + request.AddKey(kPartitionKey, + Aws::DynamoDB::Model::AttributeValue{ + PrefixedNamespace(prefix_, kind.Namespace())}); + request.AddKey(kSortKey, Aws::DynamoDB::Model::AttributeValue{itemKey}); + + auto outcome = client_->GetItem(request); + if (!outcome.IsSuccess()) { + return tl::make_unexpected(Error{outcome.GetError().GetMessage()}); + } + + auto const& item = outcome.GetResult().GetItem(); + if (item.empty()) { + return std::nullopt; + } + + auto const it = item.find(kItemAttribute); + if (it == item.end()) { + return std::nullopt; + } + + return SerializedItemDescriptor::Present(0, it->second.GetS()); +} + +ISerializedDataReader::AllResult DynamoDBDataSource::All( + ISerializedItemKind const& kind) const { + AllResult::value_type items; + + Aws::DynamoDB::Model::QueryRequest request; + request.SetTableName(table_name_); + request.SetConsistentRead(true); + request.SetKeyConditionExpression("#ns = :ns"); + request.AddExpressionAttributeNames("#ns", kPartitionKey); + request.AddExpressionAttributeValues( + ":ns", Aws::DynamoDB::Model::AttributeValue{ + PrefixedNamespace(prefix_, kind.Namespace())}); + + while (true) { + auto outcome = client_->Query(request); + if (!outcome.IsSuccess()) { + return tl::make_unexpected(Error{outcome.GetError().GetMessage()}); + } + + auto const& result = outcome.GetResult(); + for (auto const& row : result.GetItems()) { + auto const key_it = row.find(kSortKey); + auto const item_it = row.find(kItemAttribute); + if (key_it == row.end() || item_it == row.end()) { + continue; + } + items.emplace( + key_it->second.GetS(), + SerializedItemDescriptor::Present(0, item_it->second.GetS())); + } + + auto const& last_key = result.GetLastEvaluatedKey(); + if (last_key.empty()) { + break; + } + request.SetExclusiveStartKey(last_key); + } + + return items; +} + +std::string const& DynamoDBDataSource::Identity() const { + static std::string const identity = "dynamodb"; + return identity; +} + +bool DynamoDBDataSource::Initialized() const { + Aws::DynamoDB::Model::GetItemRequest request; + request.SetTableName(table_name_); + request.SetConsistentRead(true); + request.AddKey(kPartitionKey, + Aws::DynamoDB::Model::AttributeValue{inited_namespace_}); + request.AddKey(kSortKey, + Aws::DynamoDB::Model::AttributeValue{inited_namespace_}); + + auto outcome = client_->GetItem(request); + if (!outcome.IsSuccess()) { + return false; + } + return !outcome.GetResult().GetItem().empty(); } } // namespace launchdarkly::server_side::integrations diff --git a/libs/server-sdk-dynamodb-source/src/prefix.hpp b/libs/server-sdk-dynamodb-source/src/prefix.hpp new file mode 100644 index 000000000..b79319e01 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/src/prefix.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include + +namespace launchdarkly::server_side::integrations::detail { + +inline std::string PrefixedNamespace(std::string const& prefix, + std::string const& base) { + if (prefix.empty()) { + return base; + } + return prefix + ":" + base; +} + +} // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/tests/CMakeLists.txt b/libs/server-sdk-dynamodb-source/tests/CMakeLists.txt new file mode 100644 index 000000000..c4d91d7e3 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/tests/CMakeLists.txt @@ -0,0 +1,37 @@ +cmake_minimum_required(VERSION 3.19) +include(GoogleTest) + +include_directories("${PROJECT_SOURCE_DIR}/include") +include_directories("${PROJECT_SOURCE_DIR}/src") + +file(GLOB tests "${PROJECT_SOURCE_DIR}/tests/*.cpp") + +set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) + +if (WIN32) + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG "${CMAKE_BINARY_DIR}../") + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE "${CMAKE_BINARY_DIR}../") +endif () + +add_executable(gtest_${LIBNAME} + ${tests} +) + +set_target_properties(gtest PROPERTIES COMPILE_WARNING_AS_ERROR OFF) + +set(LIBS + launchdarkly::server_dynamodb_source + # Needed for access to the data models (Flag, Segment) and their + # JSON serializers, which the fixture uses to seed test data. + launchdarkly::internal + # Needed because the source doesn't expose the AWS SDK publicly, but + # the test fixture talks to DynamoDB directly to put rows. + aws-cpp-sdk-dynamodb + aws-cpp-sdk-core + GTest::gtest_main + GTest::gmock +) + +target_link_libraries(gtest_${LIBNAME} PRIVATE ${LIBS}) + +gtest_discover_tests(gtest_${LIBNAME}) diff --git a/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp new file mode 100644 index 000000000..c139f5284 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp @@ -0,0 +1,509 @@ +#include + +#include +#include + +#include +#include +#include + +#include "aws_sdk_guard.hpp" +#include "prefixed_dynamodb_client.hpp" + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +using namespace launchdarkly::server_side::integrations; +using namespace launchdarkly::data_model; +using namespace launchdarkly::server_side; + +namespace { + +std::string EnvOr(char const* name, std::string const& fallback) { + char const* value = std::getenv(name); + if (value && *value) { + return value; + } + return fallback; +} + +DynamoDBClientOptions LocalOptions() { + DynamoDBClientOptions options; + options.endpoint = EnvOr("LD_DYNAMODB_TEST_ENDPOINT", "http://localhost:8000"); + options.region = EnvOr("LD_DYNAMODB_TEST_REGION", "us-east-1"); + // DynamoDB Local accepts any non-empty credentials. + options.aws_access_key_id = "dummy"; + options.aws_secret_access_key = "dummy"; + return options; +} + +} // namespace + +class DynamoDBTests : public ::testing::Test { + public: + DynamoDBTests() + : table_name_("ld-dynamodb-source-test"), + prefix_("testprefix"), + options_(LocalOptions()), + client_(MakeRawClient()) {} + + void SetUp() override { + // Reset table state between tests. + PrefixedDynamoDBClient::DeleteTable(*client_, table_name_); + PrefixedDynamoDBClient::CreateTable(*client_, table_name_); + + auto maybe_source = + DynamoDBDataSource::Create(table_name_, prefix_, options_); + ASSERT_TRUE(maybe_source) << maybe_source.error(); + source = std::move(*maybe_source); + } + + void TearDown() override { + source.reset(); + PrefixedDynamoDBClient::DeleteTable(*client_, table_name_); + } + + void Init() { + PrefixedDynamoDBClient(*client_, prefix_, table_name_).Init(); + } + + void PutFlag(Flag const& flag) { + PrefixedDynamoDBClient(*client_, prefix_, table_name_).PutFlag(flag); + } + + void PutSegment(Segment const& segment) { + PrefixedDynamoDBClient(*client_, prefix_, table_name_) + .PutSegment(segment); + } + + void PutDeletedFlag(std::string const& key, std::string const& ts) { + PrefixedDynamoDBClient(*client_, prefix_, table_name_) + .PutDeletedFlag(key, ts); + } + + void PutDeletedSegment(std::string const& key, std::string const& ts) { + PrefixedDynamoDBClient(*client_, prefix_, table_name_) + .PutDeletedSegment(key, ts); + } + + void WithPrefixedClient( + std::string const& prefix, + std::function const& f) { + f(PrefixedDynamoDBClient(*client_, prefix, table_name_)); + } + + void WithPrefixedSource( + std::string const& prefix, + std::function const& f) const { + auto maybe_source = + DynamoDBDataSource::Create(table_name_, prefix, options_); + ASSERT_TRUE(maybe_source) << maybe_source.error(); + f(**maybe_source); + } + + protected: + std::shared_ptr source; + std::string const table_name_; + std::string const prefix_; + DynamoDBClientOptions const options_; + + private: + std::unique_ptr MakeRawClient() const { + detail::AwsSdkGuard::Ensure(); + Aws::Client::ClientConfiguration config; + config.region = *options_.region; + config.endpointOverride = *options_.endpoint; + if (options_.endpoint->rfind("http://", 0) == 0) { + config.scheme = Aws::Http::Scheme::HTTP; + config.verifySSL = false; + } + Aws::Auth::AWSCredentials creds{*options_.aws_access_key_id, + *options_.aws_secret_access_key}; + return std::make_unique(creds, config); + } + + std::unique_ptr client_; +}; + +TEST_F(DynamoDBTests, EmptyIsNotInitialized) { + ASSERT_FALSE(source->Initialized()); + + auto all_flags = source->All(FlagKind{}); + ASSERT_TRUE(all_flags.has_value()); + ASSERT_TRUE(all_flags->empty()); + + auto all_segments = source->All(SegmentKind{}); + ASSERT_TRUE(all_segments.has_value()); + ASSERT_TRUE(all_segments->empty()); +} + +TEST_F(DynamoDBTests, ChecksInitialized) { + ASSERT_FALSE(source->Initialized()); + Init(); + ASSERT_TRUE(source->Initialized()); +} + +TEST_F(DynamoDBTests, GetFlag) { + Flag const flag{"foo", 1, true}; + PutFlag(flag); + + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_TRUE(result); + + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, serialize(boost::json::value_from(flag))); + } else { + FAIL() << "expected flag to be found"; + } +} + +TEST_F(DynamoDBTests, GetSegment) { + Segment const segment{"foo", 1}; + PutSegment(segment); + + auto const result = source->Get(SegmentKind{}, "foo"); + ASSERT_TRUE(result); + + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, + serialize(boost::json::value_from(segment))); + } else { + FAIL() << "expected segment to be found"; + } +} + +TEST_F(DynamoDBTests, GetMissingFlag) { + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_TRUE(result); + ASSERT_FALSE(*result); +} + +TEST_F(DynamoDBTests, GetMissingSegment) { + auto const result = source->Get(SegmentKind{}, "foo"); + ASSERT_TRUE(result); + ASSERT_FALSE(*result); +} + +TEST_F(DynamoDBTests, GetDeletedFlag) { + PutDeletedFlag("foo", "foo_tombstone"); + + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_TRUE(result); + + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, "foo_tombstone"); + } else { + FAIL() << "expected tombstone to be present"; + } +} + +TEST_F(DynamoDBTests, GetDeletedSegment) { + PutDeletedSegment("foo", "foo_tombstone"); + + auto const result = source->Get(SegmentKind{}, "foo"); + ASSERT_TRUE(result); + + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, "foo_tombstone"); + } else { + FAIL() << "expected tombstone to be present"; + } +} + +TEST_F(DynamoDBTests, GetFlagDoesNotFindSegment) { + PutSegment(Segment{"foo", 1}); + + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_TRUE(result); + ASSERT_FALSE(*result); +} + +TEST_F(DynamoDBTests, GetSegmentDoesNotFindFlag) { + PutFlag(Flag{"foo", 1, true}); + + auto const result = source->Get(SegmentKind{}, "foo"); + ASSERT_TRUE(result); + ASSERT_FALSE(*result); +} + +TEST_F(DynamoDBTests, GetAllFlagsWhenEmpty) { + auto const result = source->All(FlagKind{}); + ASSERT_TRUE(result); + ASSERT_TRUE(result->empty()); +} + +TEST_F(DynamoDBTests, GetAllSegmentsWhenEmpty) { + auto const result = source->All(SegmentKind{}); + ASSERT_TRUE(result); + ASSERT_TRUE(result->empty()); +} + +TEST_F(DynamoDBTests, GetAllFlags) { + Flag const flag1{"foo", 1, true}; + Flag const flag2{"bar", 2, false}; + + PutFlag(flag1); + PutFlag(flag2); + PutDeletedFlag("baz", "baz_tombstone"); + + auto const result = source->All(FlagKind{}); + ASSERT_TRUE(result); + ASSERT_EQ(result->size(), 3); + + auto const& flags = *result; + auto const flag1_it = flags.find("foo"); + ASSERT_NE(flag1_it, flags.end()); + ASSERT_EQ(flag1_it->second.serializedItem, + serialize(boost::json::value_from(flag1))); + + auto const flag2_it = flags.find("bar"); + ASSERT_NE(flag2_it, flags.end()); + ASSERT_EQ(flag2_it->second.serializedItem, + serialize(boost::json::value_from(flag2))); + + auto const flag3_it = flags.find("baz"); + ASSERT_NE(flag3_it, flags.end()); + ASSERT_EQ(flag3_it->second.serializedItem, "baz_tombstone"); +} + +TEST_F(DynamoDBTests, InitializedPrefixIndependence) { + WithPrefixedClient("not_our_prefix", [&](auto const& client) { + client.Init(); + ASSERT_FALSE(source->Initialized()); + }); + + WithPrefixedClient("TestPrefix", [&](auto const& client) { + client.Init(); + ASSERT_FALSE(source->Initialized()); + }); + + WithPrefixedClient("stillnotprefix", [&](auto const& client) { + client.Init(); + ASSERT_FALSE(source->Initialized()); + }); +} + +TEST_F(DynamoDBTests, SegmentPrefixIndependence) { + auto MakeSegment = [](std::uint64_t const version) { + return Segment{"foo", version}; + }; + + auto PrefixName = [](std::uint64_t const version) { + return "prefix" + std::to_string(version); + }; + + auto ValidateSegment = [&](ISerializedDataReader::GetResult const& result, + std::size_t i) { + ASSERT_TRUE(result); + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, + serialize(boost::json::value_from(MakeSegment(i)))); + } else { + FAIL() << "expected segment to be found under " << PrefixName(i); + } + }; + + constexpr std::size_t kPrefixCount = 10; + + for (std::size_t i = 0; i < kPrefixCount; i++) { + WithPrefixedClient(PrefixName(i), [&](auto const& client) { + client.PutSegment(MakeSegment(i)); + }); + } + + for (std::size_t i = 0; i < kPrefixCount; i++) { + WithPrefixedSource(PrefixName(i), [&](auto const& src) { + ValidateSegment(src.Get(SegmentKind{}, "foo"), i); + auto all = src.All(SegmentKind{}); + ASSERT_TRUE(all); + ASSERT_EQ(all->size(), 1); + }); + } +} + +TEST_F(DynamoDBTests, FlagPrefixIndependence) { + auto MakeFlag = [](std::uint64_t const version) { + return Flag{"foo", version, true}; + }; + + auto PrefixName = [](std::uint64_t const version) { + return "prefix" + std::to_string(version); + }; + + auto ValidateFlag = [&](ISerializedDataReader::GetResult const& result, + std::size_t i) { + ASSERT_TRUE(result); + if (auto const f = *result) { + ASSERT_EQ(f->serializedItem, + serialize(boost::json::value_from(MakeFlag(i)))); + } else { + FAIL() << "expected flag to be found under " << PrefixName(i); + } + }; + + constexpr std::size_t kPrefixCount = 10; + + for (std::size_t i = 0; i < kPrefixCount; i++) { + WithPrefixedClient(PrefixName(i), [&](auto const& client) { + client.PutFlag(MakeFlag(i)); + }); + } + + for (std::size_t i = 0; i < kPrefixCount; i++) { + WithPrefixedSource(PrefixName(i), [&](auto const& src) { + ValidateFlag(src.Get(FlagKind{}, "foo"), i); + auto all = src.All(FlagKind{}); + ASSERT_TRUE(all); + ASSERT_EQ(all->size(), 1); + }); + } +} + +TEST_F(DynamoDBTests, FlagAndSegmentCanCoexistWithSameKey) { + Flag const flag_in{"foo", 1, true}; + Segment const segment_in{"foo", 1}; + + PutFlag(flag_in); + PutSegment(segment_in); + + auto flag = source->Get(FlagKind{}, "foo"); + ASSERT_TRUE(flag); + ASSERT_EQ((*flag)->serializedItem, + serialize(boost::json::value_from(flag_in))); + + auto segment = source->Get(SegmentKind{}, "foo"); + ASSERT_TRUE(segment); + ASSERT_EQ((*segment)->serializedItem, + serialize(boost::json::value_from(segment_in))); +} + +TEST_F(DynamoDBTests, EmptyPrefixUsesBareNamespaces) { + // A source with no prefix should successfully read rows whose + // namespace partition keys are bare "features" / "segments" / "$inited". + WithPrefixedClient("", [&](auto const& client) { + client.Init(); + client.PutFlag(Flag{"foo", 1, true}); + }); + + WithPrefixedSource("", [&](auto const& src) { + ASSERT_TRUE(src.Initialized()); + auto const got = src.Get(FlagKind{}, "foo"); + ASSERT_TRUE(got); + ASSERT_TRUE(*got); + }); +} + +TEST_F(DynamoDBTests, AllPaginatesAcrossMultiplePages) { + // DynamoDB Query responses cap at 1MB. Insert enough large flags to + // force at least two pages, then verify All() returns every one. + constexpr std::size_t kFlagCount = 40; + constexpr std::size_t kPayloadBytes = 100 * 1024; // 100 KiB per flag + + for (std::size_t i = 0; i < kFlagCount; ++i) { + // Use the deleted-flag path to write an arbitrary opaque payload + // without forcing the data model to accept giant strings. + PutDeletedFlag("flag_" + std::to_string(i), + std::string(kPayloadBytes, 'x')); + } + + auto const result = source->All(FlagKind{}); + ASSERT_TRUE(result); + ASSERT_EQ(result->size(), kFlagCount); +} + +TEST_F(DynamoDBTests, IdentityReturnsDynamodb) { + ASSERT_EQ(source->Identity(), "dynamodb"); +} + +TEST_F(DynamoDBTests, CanConvertDataSourceToDataReader) { + auto maybe_source = + DynamoDBDataSource::Create(table_name_, "prefix", LocalOptions()); + ASSERT_TRUE(maybe_source); + + std::shared_ptr reader = std::move(*maybe_source); +} + +TEST_F(DynamoDBTests, CanUseAsSDKLazyLoadDataSource) { + Flag flag_a{"foo", 1, false, std::nullopt, {true, false}}; + flag_a.offVariation = 0; // variation: true + Flag flag_b{"bar", 1, false, std::nullopt, {true, false}}; + flag_b.offVariation = 1; // variation: false + + PutFlag(flag_a); + PutFlag(flag_b); + Init(); + + auto cfg_builder = ConfigBuilder("sdk-123"); + cfg_builder.DataSystem().Method( + config::builders::LazyLoadBuilder().Source(source)); + cfg_builder.Events().Disable(); + auto config = cfg_builder.Build(); + + ASSERT_TRUE(config); + + auto client = Client(std::move(*config)); + client.StartAsync(); + + auto const context = + launchdarkly::ContextBuilder().Kind("cat", "shadow").Build(); + + auto const all_flags = client.AllFlagsState(context); + auto const expected = std::unordered_map{ + {"foo", true}, {"bar", false}}; + + ASSERT_TRUE(all_flags.Valid()); + ASSERT_EQ(all_flags.Values(), expected); +} + +TEST(DynamoDBErrorTests, NonExistentTableReturnsErrorFromGet) { + auto maybe_source = DynamoDBDataSource::Create( + "table-that-does-not-exist", "prefix", LocalOptions()); + ASSERT_TRUE(maybe_source); + + auto const src = std::move(*maybe_source); + + ASSERT_FALSE(src->Initialized()); + + auto const get_flag = src->Get(FlagKind{}, "foo"); + ASSERT_FALSE(get_flag); + + auto const get_segment = src->Get(SegmentKind{}, "foo"); + ASSERT_FALSE(get_segment); + + auto const get_all_flag = src->All(FlagKind{}); + ASSERT_FALSE(get_all_flag); + + auto const get_all_segment = src->All(SegmentKind{}); + ASSERT_FALSE(get_all_segment); +} + +TEST(DynamoDBErrorTests, UnreachableEndpointReturnsErrorFromGet) { + DynamoDBClientOptions options; + options.endpoint = "http://127.0.0.1:1"; // nothing listening + options.region = "us-east-1"; + options.aws_access_key_id = "dummy"; + options.aws_secret_access_key = "dummy"; + + auto maybe_source = + DynamoDBDataSource::Create("any-table", "prefix", options); + ASSERT_TRUE(maybe_source); + + auto const src = std::move(*maybe_source); + + ASSERT_FALSE(src->Initialized()); + + auto const get_flag = src->Get(FlagKind{}, "foo"); + ASSERT_FALSE(get_flag); + + auto const get_all = src->All(FlagKind{}); + ASSERT_FALSE(get_all); +} diff --git a/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp new file mode 100644 index 000000000..b168a9be2 --- /dev/null +++ b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp @@ -0,0 +1,148 @@ +#pragma once + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include + +#include + +// PrefixedDynamoDBClient is a test fixture helper that writes flags and +// segments directly into a DynamoDB table using the LaunchDarkly schema +// (namespace + key), mirroring the Redis source's PrefixedClient. +class PrefixedDynamoDBClient { + public: + PrefixedDynamoDBClient(Aws::DynamoDB::DynamoDBClient& client, + std::string prefix, + std::string table_name) + : client_(client), + prefix_(std::move(prefix)), + table_name_(std::move(table_name)) {} + + static void CreateTable(Aws::DynamoDB::DynamoDBClient& client, + std::string const& table_name) { + Aws::DynamoDB::Model::CreateTableRequest request; + request.SetTableName(table_name); + + Aws::DynamoDB::Model::KeySchemaElement partition; + partition.SetAttributeName("namespace"); + partition.SetKeyType(Aws::DynamoDB::Model::KeyType::HASH); + request.AddKeySchema(partition); + + Aws::DynamoDB::Model::KeySchemaElement sort; + sort.SetAttributeName("key"); + sort.SetKeyType(Aws::DynamoDB::Model::KeyType::RANGE); + request.AddKeySchema(sort); + + Aws::DynamoDB::Model::AttributeDefinition partition_def; + partition_def.SetAttributeName("namespace"); + partition_def.SetAttributeType( + Aws::DynamoDB::Model::ScalarAttributeType::S); + request.AddAttributeDefinitions(partition_def); + + Aws::DynamoDB::Model::AttributeDefinition sort_def; + sort_def.SetAttributeName("key"); + sort_def.SetAttributeType( + Aws::DynamoDB::Model::ScalarAttributeType::S); + request.AddAttributeDefinitions(sort_def); + + Aws::DynamoDB::Model::ProvisionedThroughput throughput; + throughput.SetReadCapacityUnits(5); + throughput.SetWriteCapacityUnits(5); + request.SetProvisionedThroughput(throughput); + + auto outcome = client.CreateTable(request); + if (!outcome.IsSuccess()) { + FAIL() << "couldn't create DynamoDB table " << table_name << ": " + << outcome.GetError().GetMessage(); + } + } + + static void DeleteTable(Aws::DynamoDB::DynamoDBClient& client, + std::string const& table_name) { + Aws::DynamoDB::Model::DeleteTableRequest request; + request.SetTableName(table_name); + auto outcome = client.DeleteTable(request); + if (!outcome.IsSuccess()) { + // Ignore the not-found case (test may not have created the table). + return; + } + } + + static bool TableExists(Aws::DynamoDB::DynamoDBClient& client, + std::string const& table_name) { + Aws::DynamoDB::Model::DescribeTableRequest request; + request.SetTableName(table_name); + return client.DescribeTable(request).IsSuccess(); + } + + void Init() const { + std::string const inited_key = Prefixed("$inited"); + PutRaw(inited_key, inited_key, /*item_attribute=*/std::nullopt); + } + + void PutFlag(launchdarkly::data_model::Flag const& flag) const { + PutRaw(Prefixed("features"), flag.key, + serialize(boost::json::value_from(flag))); + } + + void PutSegment(launchdarkly::data_model::Segment const& segment) const { + PutRaw(Prefixed("segments"), segment.key, + serialize(boost::json::value_from(segment))); + } + + void PutDeletedFlag(std::string const& key, std::string const& ts) const { + PutRaw(Prefixed("features"), key, ts); + } + + void PutDeletedSegment(std::string const& key, + std::string const& ts) const { + PutRaw(Prefixed("segments"), key, ts); + } + + private: + std::string Prefixed(std::string const& base) const { + if (prefix_.empty()) { + return base; + } + return prefix_ + ":" + base; + } + + void PutRaw(std::string const& ns, + std::string const& key, + std::optional const& item_attribute) const { + Aws::DynamoDB::Model::PutItemRequest request; + request.SetTableName(table_name_); + request.AddItem("namespace", Aws::DynamoDB::Model::AttributeValue{ns}); + request.AddItem("key", Aws::DynamoDB::Model::AttributeValue{key}); + if (item_attribute) { + request.AddItem("item", + Aws::DynamoDB::Model::AttributeValue{*item_attribute}); + } + auto outcome = client_.PutItem(request); + if (!outcome.IsSuccess()) { + FAIL() << "couldn't put DynamoDB item ns=" << ns + << " key=" << key << ": " + << outcome.GetError().GetMessage(); + } + } + + Aws::DynamoDB::DynamoDBClient& client_; + std::string const prefix_; + std::string const table_name_; +}; From f070f2a62feb916cab9d837f9634b8a979716ebe Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 16:11:33 -0700 Subject: [PATCH 2/9] fix: return errors for malformed rows and partial credentials in DynamoDB source --- .../src/client_factory.cpp | 43 +++++++++++++++---- .../src/client_factory.hpp | 17 +++++--- .../src/dynamodb_source.cpp | 21 ++++++--- .../tests/dynamodb_source_test.cpp | 19 ++++++++ .../tests/prefixed_dynamodb_client.hpp | 17 +++++++- 5 files changed, 93 insertions(+), 24 deletions(-) diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.cpp b/libs/server-sdk-dynamodb-source/src/client_factory.cpp index 9068dc71f..f2c6e589f 100644 --- a/libs/server-sdk-dynamodb-source/src/client_factory.cpp +++ b/libs/server-sdk-dynamodb-source/src/client_factory.cpp @@ -8,10 +8,32 @@ namespace launchdarkly::server_side::integrations::detail { namespace { -bool HasExplicitCredentials(DynamoDBClientOptions const& options) { - return options.aws_access_key_id.has_value() || - options.aws_secret_access_key.has_value() || - options.aws_session_token.has_value(); +// Verifies that the credential fields in `options` form a valid combination. +// Returns an empty optional on success, or an error string describing what's +// wrong. The valid combinations are: +// +// - none of the three set (fall through to AWS default credential chain) +// - access_key_id + secret_access_key (long-lived IAM keys) +// - access_key_id + secret_access_key + session_token (STS temporary creds) +// +// All other partial combinations would build a misconfigured AWS client that +// fails opaquely at request time; catching them here surfaces the +// misconfiguration up front. +std::optional ValidateCredentials( + DynamoDBClientOptions const& options) { + bool const has_key = options.aws_access_key_id.has_value(); + bool const has_secret = options.aws_secret_access_key.has_value(); + bool const has_token = options.aws_session_token.has_value(); + + if (has_key != has_secret) { + return "aws_access_key_id and aws_secret_access_key must both be set " + "or both unset"; + } + if (has_token && !has_key) { + return "aws_session_token requires aws_access_key_id and " + "aws_secret_access_key"; + } + return std::nullopt; } Aws::Client::ClientConfiguration BuildConfig( @@ -39,14 +61,17 @@ Aws::Client::ClientConfiguration BuildConfig( } // namespace -std::shared_ptr BuildDynamoDBClient( - DynamoDBClientOptions const& options) { +tl::expected, std::string> +BuildDynamoDBClient(DynamoDBClientOptions const& options) { + if (auto err = ValidateCredentials(options)) { + return tl::make_unexpected(std::move(*err)); + } + auto const config = BuildConfig(options); - if (HasExplicitCredentials(options)) { + if (options.aws_access_key_id) { Aws::Auth::AWSCredentials credentials{ - options.aws_access_key_id.value_or(""), - options.aws_secret_access_key.value_or(""), + *options.aws_access_key_id, *options.aws_secret_access_key, options.aws_session_token.value_or("")}; return std::make_shared(credentials, config); diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.hpp b/libs/server-sdk-dynamodb-source/src/client_factory.hpp index a93fb85d9..2dbab8921 100644 --- a/libs/server-sdk-dynamodb-source/src/client_factory.hpp +++ b/libs/server-sdk-dynamodb-source/src/client_factory.hpp @@ -4,18 +4,21 @@ #include +#include + #include +#include namespace launchdarkly::server_side::integrations::detail { // Builds an Aws::DynamoDB::DynamoDBClient configured from the supplied -// DynamoDBClientOptions. Caller is responsible for ensuring AwsSdkGuard::Ensure() -// has been called first. +// DynamoDBClientOptions, or returns an error string if the options are +// internally inconsistent (e.g. only one of aws_access_key_id / +// aws_secret_access_key is set). // -// The shared_ptr return enables future sharing of a single client across -// multiple DynamoDB-backed stores in the same process (e.g. data source + -// big-segment store); today each consumer constructs its own. -std::shared_ptr BuildDynamoDBClient( - DynamoDBClientOptions const& options); +// Caller is responsible for ensuring AwsSdkGuard::Ensure() has been called +// first. +tl::expected, std::string> +BuildDynamoDBClient(DynamoDBClientOptions const& options); } // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp index 3d276d3ad..f7547ab91 100644 --- a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp @@ -32,9 +32,13 @@ DynamoDBDataSource::Create(std::string table_name, DynamoDBClientOptions options) { try { detail::AwsSdkGuard::Ensure(); - auto client = detail::BuildDynamoDBClient(options); - return std::unique_ptr(new DynamoDBDataSource( - std::move(client), std::move(table_name), std::move(prefix))); + auto maybe_client = detail::BuildDynamoDBClient(options); + if (!maybe_client) { + return tl::make_unexpected(std::move(maybe_client.error())); + } + return std::unique_ptr( + new DynamoDBDataSource(std::move(*maybe_client), + std::move(table_name), std::move(prefix))); } catch (std::exception const& e) { return tl::make_unexpected(e.what()); } @@ -74,7 +78,8 @@ ISerializedDataReader::GetResult DynamoDBDataSource::Get( auto const it = item.find(kItemAttribute); if (it == item.end()) { - return std::nullopt; + return tl::make_unexpected( + Error{"DynamoDB row missing expected 'item' attribute"}); } return SerializedItemDescriptor::Present(0, it->second.GetS()); @@ -102,10 +107,14 @@ ISerializedDataReader::AllResult DynamoDBDataSource::All( auto const& result = outcome.GetResult(); for (auto const& row : result.GetItems()) { auto const key_it = row.find(kSortKey); - auto const item_it = row.find(kItemAttribute); - if (key_it == row.end() || item_it == row.end()) { + if (key_it == row.end()) { continue; } + auto const item_it = row.find(kItemAttribute); + if (item_it == row.end()) { + return tl::make_unexpected( + Error{"DynamoDB row missing expected 'item' attribute"}); + } items.emplace( key_it->second.GetS(), SerializedItemDescriptor::Present(0, item_it->second.GetS())); diff --git a/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp index c139f5284..9578ce73f 100644 --- a/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp +++ b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp @@ -420,6 +420,25 @@ TEST_F(DynamoDBTests, AllPaginatesAcrossMultiplePages) { ASSERT_EQ(result->size(), kFlagCount); } +TEST_F(DynamoDBTests, GetReturnsErrorWhenRowIsMissingItemAttribute) { + WithPrefixedClient(prefix_, [&](auto const& client) { + client.PutRowWithoutItem("features", "foo"); + }); + + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_FALSE(result); +} + +TEST_F(DynamoDBTests, AllReturnsErrorWhenRowIsMissingItemAttribute) { + WithPrefixedClient(prefix_, [&](auto const& client) { + client.PutFlag(Flag{"foo", 1, true}); + client.PutRowWithoutItem("features", "bar"); + }); + + auto const result = source->All(FlagKind{}); + ASSERT_FALSE(result); +} + TEST_F(DynamoDBTests, IdentityReturnsDynamodb) { ASSERT_EQ(source->Identity(), "dynamodb"); } diff --git a/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp index b168a9be2..397cb3ddb 100644 --- a/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp +++ b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -79,8 +80,13 @@ class PrefixedDynamoDBClient { request.SetTableName(table_name); auto outcome = client.DeleteTable(request); if (!outcome.IsSuccess()) { - // Ignore the not-found case (test may not have created the table). - return; + // Tolerate not-found so setup can call this unconditionally. + if (outcome.GetError().GetErrorType() == + Aws::DynamoDB::DynamoDBErrors::RESOURCE_NOT_FOUND) { + return; + } + FAIL() << "couldn't delete DynamoDB table " << table_name << ": " + << outcome.GetError().GetMessage(); } } @@ -115,6 +121,13 @@ class PrefixedDynamoDBClient { PutRaw(Prefixed("segments"), key, ts); } + // Writes a row with only namespace + key, no `item` attribute. Used to + // simulate corrupted/malformed rows in the table. + void PutRowWithoutItem(std::string const& ns_suffix, + std::string const& key) const { + PutRaw(Prefixed(ns_suffix), key, std::nullopt); + } + private: std::string Prefixed(std::string const& base) const { if (prefix_.empty()) { From e7ffabb73e5c3832962c060e83bc8721cfdd51da Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 16:23:36 -0700 Subject: [PATCH 3/9] refactor: use unique_ptr for DynamoDB client ownership --- .../server_side/integrations/dynamodb/dynamodb_source.hpp | 4 ++-- libs/server-sdk-dynamodb-source/src/client_factory.cpp | 6 +++--- libs/server-sdk-dynamodb-source/src/client_factory.hpp | 2 +- libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp index 6a436bd6f..68cf8507b 100644 --- a/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp +++ b/libs/server-sdk-dynamodb-source/include/launchdarkly/server_side/integrations/dynamodb/dynamodb_source.hpp @@ -68,11 +68,11 @@ class DynamoDBDataSource final : public ISerializedDataReader { ~DynamoDBDataSource() override; private: - DynamoDBDataSource(std::shared_ptr client, + DynamoDBDataSource(std::unique_ptr client, std::string table_name, std::string prefix); - std::shared_ptr client_; + std::unique_ptr client_; std::string const table_name_; std::string const prefix_; std::string const inited_namespace_; diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.cpp b/libs/server-sdk-dynamodb-source/src/client_factory.cpp index f2c6e589f..c06a3b005 100644 --- a/libs/server-sdk-dynamodb-source/src/client_factory.cpp +++ b/libs/server-sdk-dynamodb-source/src/client_factory.cpp @@ -61,7 +61,7 @@ Aws::Client::ClientConfiguration BuildConfig( } // namespace -tl::expected, std::string> +tl::expected, std::string> BuildDynamoDBClient(DynamoDBClientOptions const& options) { if (auto err = ValidateCredentials(options)) { return tl::make_unexpected(std::move(*err)); @@ -73,11 +73,11 @@ BuildDynamoDBClient(DynamoDBClientOptions const& options) { Aws::Auth::AWSCredentials credentials{ *options.aws_access_key_id, *options.aws_secret_access_key, options.aws_session_token.value_or("")}; - return std::make_shared(credentials, + return std::make_unique(credentials, config); } - return std::make_shared(config); + return std::make_unique(config); } } // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/client_factory.hpp b/libs/server-sdk-dynamodb-source/src/client_factory.hpp index 2dbab8921..9cf55cf36 100644 --- a/libs/server-sdk-dynamodb-source/src/client_factory.hpp +++ b/libs/server-sdk-dynamodb-source/src/client_factory.hpp @@ -18,7 +18,7 @@ namespace launchdarkly::server_side::integrations::detail { // // Caller is responsible for ensuring AwsSdkGuard::Ensure() has been called // first. -tl::expected, std::string> +tl::expected, std::string> BuildDynamoDBClient(DynamoDBClientOptions const& options); } // namespace launchdarkly::server_side::integrations::detail diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp index f7547ab91..f9fc83d12 100644 --- a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp @@ -45,7 +45,7 @@ DynamoDBDataSource::Create(std::string table_name, } DynamoDBDataSource::DynamoDBDataSource( - std::shared_ptr client, + std::unique_ptr client, std::string table_name, std::string prefix) : client_(std::move(client)), From a3078b55e76781ff4a50378a49d19618718ccb32 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 16:46:05 -0700 Subject: [PATCH 4/9] fix: pass LD_BUILD_DYNAMODB_SUPPORT through release scripts --- scripts/build-release.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/build-release.sh b/scripts/build-release.sh index 38332ecf6..535a58c4b 100755 --- a/scripts/build-release.sh +++ b/scripts/build-release.sh @@ -9,6 +9,7 @@ set -e # Parse arguments TARGET="$1" build_redis="OFF" +build_dynamodb="OFF" build_curl="OFF" # Special case: unlike the other targets, enabling redis support will pull in redis++ and hiredis dependencies at @@ -17,6 +18,11 @@ if [ "$TARGET" == "launchdarkly-cpp-server-redis-source" ]; then build_redis="ON" fi +# Special case: enabling DynamoDB support fetches the AWS C++ SDK at configuration time. Only enable when asked. +if [ "$TARGET" == "launchdarkly-cpp-server-dynamodb-source" ]; then + build_dynamodb="ON" +fi + # Check for --with-curl flag for arg in "$@"; do if [ "$arg" == "--with-curl" ]; then @@ -35,7 +41,7 @@ fi # Build a static release. mkdir -p "build-static${suffix}" && cd "build-static${suffix}" mkdir -p release -cmake -G Ninja -D CMAKE_BUILD_TYPE=Release -D LD_BUILD_REDIS_SUPPORT="$build_redis" -D LD_CURL_NETWORKING="$build_curl" -D BUILD_TESTING=OFF -D CMAKE_INSTALL_PREFIX=./release .. +cmake -G Ninja -D CMAKE_BUILD_TYPE=Release -D LD_BUILD_REDIS_SUPPORT="$build_redis" -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" -D LD_CURL_NETWORKING="$build_curl" -D BUILD_TESTING=OFF -D CMAKE_INSTALL_PREFIX=./release .. cmake --build . --target "$TARGET" cmake --install . @@ -44,7 +50,7 @@ cd .. # Build a dynamic release. mkdir -p "build-dynamic${suffix}" && cd "build-dynamic${suffix}" mkdir -p release -cmake -G Ninja -D CMAKE_BUILD_TYPE=Release -D LD_BUILD_REDIS_SUPPORT="$build_redis" -D LD_CURL_NETWORKING="$build_curl" -D BUILD_TESTING=OFF -D LD_BUILD_SHARED_LIBS=ON -D CMAKE_INSTALL_PREFIX=./release .. +cmake -G Ninja -D CMAKE_BUILD_TYPE=Release -D LD_BUILD_REDIS_SUPPORT="$build_redis" -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" -D LD_CURL_NETWORKING="$build_curl" -D BUILD_TESTING=OFF -D LD_BUILD_SHARED_LIBS=ON -D CMAKE_INSTALL_PREFIX=./release .. cmake --build . --target "$TARGET" cmake --install . From 69b9a8744407d1a523e95a73fcbf9f4cd8651fea Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 19:50:18 -0700 Subject: [PATCH 5/9] fix: pass LD_BUILD_DYNAMODB_SUPPORT through Windows release script --- scripts/build-release-windows.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/build-release-windows.sh b/scripts/build-release-windows.sh index 5f5d5427f..e039b8bb1 100755 --- a/scripts/build-release-windows.sh +++ b/scripts/build-release-windows.sh @@ -9,6 +9,7 @@ set -e # Parse arguments TARGET="$1" build_redis="OFF" +build_dynamodb="OFF" build_curl="OFF" # Special case: unlike the other targets, enabling redis support will pull in redis++ and hiredis dependencies at @@ -17,6 +18,11 @@ if [ "$TARGET" == "launchdarkly-cpp-server-redis-source" ]; then build_redis="ON" fi +# Special case: enabling DynamoDB support fetches the AWS C++ SDK at configuration time. Only enable when asked. +if [ "$TARGET" == "launchdarkly-cpp-server-dynamodb-source" ]; then + build_dynamodb="ON" +fi + # Check for --with-curl flag for arg in "$@"; do if [ "$arg" == "--with-curl" ]; then @@ -37,6 +43,7 @@ mkdir -p "build-static${suffix}" && cd "build-static${suffix}" mkdir -p release cmake -G Ninja -D CMAKE_BUILD_TYPE=Release \ -D LD_BUILD_REDIS_SUPPORT="$build_redis" \ + -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" \ -D LD_CURL_NETWORKING="$build_curl" \ -D BUILD_TESTING=OFF \ -D CMAKE_INSTALL_PREFIX=./release .. @@ -50,6 +57,7 @@ mkdir -p "build-dynamic${suffix}" && cd "build-dynamic${suffix}" mkdir -p release cmake -G Ninja -D CMAKE_BUILD_TYPE=Release \ -D LD_BUILD_REDIS_SUPPORT="$build_redis" \ + -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" \ -D LD_CURL_NETWORKING="$build_curl" \ -D BUILD_TESTING=OFF \ -D LD_BUILD_SHARED_LIBS=ON \ @@ -66,6 +74,7 @@ mkdir -p release cmake -G Ninja -D CMAKE_BUILD_TYPE=Debug \ -D BUILD_TESTING=OFF \ -D LD_BUILD_REDIS_SUPPORT="$build_redis" \ + -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" \ -D LD_CURL_NETWORKING="$build_curl" \ -D CMAKE_INSTALL_PREFIX=./release .. @@ -80,6 +89,7 @@ mkdir -p release cmake -G Ninja -D CMAKE_BUILD_TYPE=Debug \ -D BUILD_TESTING=OFF \ -D LD_BUILD_REDIS_SUPPORT="$build_redis" \ + -D LD_BUILD_DYNAMODB_SUPPORT="$build_dynamodb" \ -D LD_CURL_NETWORKING="$build_curl" \ -D LD_BUILD_SHARED_LIBS=ON \ -D LD_DYNAMIC_LINK_BOOST=OFF \ From 54f65e27300cb5cc1a6077ea8bf2d9a71fdf9a6e Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 19:59:59 -0700 Subject: [PATCH 6/9] fix: build AWS SDK statically to avoid macOS shared-link failure --- cmake/aws-sdk-cpp.cmake | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/cmake/aws-sdk-cpp.cmake b/cmake/aws-sdk-cpp.cmake index 3c06c6b83..84c4f56b5 100644 --- a/cmake/aws-sdk-cpp.cmake +++ b/cmake/aws-sdk-cpp.cmake @@ -23,4 +23,22 @@ FetchContent_Declare(aws-sdk-cpp OVERRIDE_FIND_PACKAGE ) -FetchContent_MakeAvailable(aws-sdk-cpp) +# Always build the AWS SDK as static archives, even when our own library is +# being built shared (LD_BUILD_SHARED_LIBS=ON). Building aws-sdk-cpp as +# BUILD_SHARED_LIBS=ON produces dylibs on macOS whose dynamodb component +# fails to find AWS Core symbols at link time (the AWS SDK's visibility +# configuration doesn't export them consistently across its FetchContent +# build). Linking aws-sdk-cpp statically into our shared wrapper sidesteps +# the issue and matches the redis pattern of pinning dep build shape via +# a scoped variable rather than overwriting the global BUILD_SHARED_LIBS. +# +# Using a function for scope: `set(BUILD_SHARED_LIBS OFF)` inside the +# function shadows any cached/parent value during FetchContent_MakeAvailable +# without persisting after the call, so user `-D LD_BUILD_SHARED_LIBS=ON` +# for the rest of the project remains untouched. +function(_ld_fetch_aws_sdk_cpp_static) + set(BUILD_SHARED_LIBS OFF) + FetchContent_MakeAvailable(aws-sdk-cpp) +endfunction() + +_ld_fetch_aws_sdk_cpp_static() From d936b5d784cc621dd9e9a658bed566dca3ff00a0 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 20:50:33 -0700 Subject: [PATCH 7/9] fix: save and restore BUILD_SHARED_LIBS around AWS SDK FetchContent --- cmake/aws-sdk-cpp.cmake | 44 ++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/cmake/aws-sdk-cpp.cmake b/cmake/aws-sdk-cpp.cmake index 84c4f56b5..d2123a294 100644 --- a/cmake/aws-sdk-cpp.cmake +++ b/cmake/aws-sdk-cpp.cmake @@ -29,16 +29,36 @@ FetchContent_Declare(aws-sdk-cpp # fails to find AWS Core symbols at link time (the AWS SDK's visibility # configuration doesn't export them consistently across its FetchContent # build). Linking aws-sdk-cpp statically into our shared wrapper sidesteps -# the issue and matches the redis pattern of pinning dep build shape via -# a scoped variable rather than overwriting the global BUILD_SHARED_LIBS. +# the issue. # -# Using a function for scope: `set(BUILD_SHARED_LIBS OFF)` inside the -# function shadows any cached/parent value during FetchContent_MakeAvailable -# without persisting after the call, so user `-D LD_BUILD_SHARED_LIBS=ON` -# for the rest of the project remains untouched. -function(_ld_fetch_aws_sdk_cpp_static) - set(BUILD_SHARED_LIBS OFF) - FetchContent_MakeAvailable(aws-sdk-cpp) -endfunction() - -_ld_fetch_aws_sdk_cpp_static() +# This needs cache manipulation rather than a function-scoped `set()`: +# aws-sdk-cpp's top-level CMakeLists pins `cmake_policy(SET CMP0077 OLD)`, +# which makes its `option(BUILD_SHARED_LIBS ... ON)` ignore non-cache +# variables and unconditionally write the cache. The only way to stop +# `option()` from setting the cache to ON is to pre-populate the cache +# with OFF before FetchContent_MakeAvailable runs. +# +# The prior cache state is saved before the FORCE-write and restored +# immediately after FetchContent finishes, so other subprojects (e.g. +# libs/server-sdk-otel) see whatever BUILD_SHARED_LIBS value was in the +# cache when this file was first included. An earlier attempt that +# FORCE-wrote OFF without restoring leaked into those subprojects; the +# save/restore here is the fix for that. +if (DEFINED CACHE{BUILD_SHARED_LIBS}) + set(_LD_AWS_BSL_PREV "${BUILD_SHARED_LIBS}") + set(_LD_AWS_BSL_WAS_CACHED TRUE) +else () + set(_LD_AWS_BSL_WAS_CACHED FALSE) +endif () + +set(BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE) + +FetchContent_MakeAvailable(aws-sdk-cpp) + +if (_LD_AWS_BSL_WAS_CACHED) + set(BUILD_SHARED_LIBS "${_LD_AWS_BSL_PREV}" CACHE BOOL "" FORCE) +else () + unset(BUILD_SHARED_LIBS CACHE) +endif () +unset(_LD_AWS_BSL_PREV) +unset(_LD_AWS_BSL_WAS_CACHED) From e1dafd88d78beaab1de3940bcb1fc60ea43d86a9 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 14 May 2026 21:12:36 -0700 Subject: [PATCH 8/9] chore: drop unused kVersionAttribute constant --- libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp b/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp index 2057fe1a1..84ffbc31a 100644 --- a/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_attributes.hpp @@ -11,7 +11,6 @@ namespace launchdarkly::server_side::integrations::detail { // agree on what's in the table. inline constexpr char kPartitionKey[] = "namespace"; inline constexpr char kSortKey[] = "key"; -inline constexpr char kVersionAttribute[] = "version"; inline constexpr char kItemAttribute[] = "item"; // Sentinel namespace marking the table as initialized. When a prefix is set, From c4f97d6a6311decf37fe9ba4d61f687f63825edb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 15 May 2026 16:11:36 -0700 Subject: [PATCH 9/9] fix: surface non-String 'item' attribute as structured error DynamoDB does not enforce a type schema on non-key attributes, so a non-Relay writer (manual put-item, schema-migration tool, etc.) can produce a row whose 'item' attribute is stored as Number/Binary/Null/ Bool/List/Map instead of String. AttributeValue::GetS() on such a value silently returns an empty string. Without a check, the source returns Present(0, "") and the empty string flows into JsonDeserializer::DeserializeJsonDescriptor's throwing boost::json::parse, which propagates an uncaught exception out of the SDK's evaluation entry point. Treat an empty GetS() result as a structured error in both Get() and All(). The sort-key GetS() (All() line 119) is left untouched -- the DynamoDB service enforces the table's key schema at insert time, so the sort key is guaranteed type S by table-schema invariant. Adds PutRowWithNumericItem test helper that stores 'item' as a Number via AttributeValue::SetN, and two bug-proving tests (GetReturnsErrorWhenItemAttributeIsNotString, AllReturnsErrorWhenItemAttributeIsNotString) that fail without the fix and pass with it. Full dynamodb suite is 28/28 green. Per https://github.com/launchdarkly/cpp-sdks/pull/534#issuecomment-4464227563 --- .../src/dynamodb_source.cpp | 24 ++++++++++++++-- .../tests/dynamodb_source_test.cpp | 28 +++++++++++++++++++ .../tests/prefixed_dynamodb_client.hpp | 26 +++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp index f9fc83d12..03c259017 100644 --- a/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp +++ b/libs/server-sdk-dynamodb-source/src/dynamodb_source.cpp @@ -82,7 +82,20 @@ ISerializedDataReader::GetResult DynamoDBDataSource::Get( Error{"DynamoDB row missing expected 'item' attribute"}); } - return SerializedItemDescriptor::Present(0, it->second.GetS()); + // AttributeValue::GetS() silently returns an empty string when the + // attribute is stored as any non-String type (N/B/NULL/BOOL/L/M/SS/etc.). + // DynamoDB does not enforce a type schema on non-key attributes, so a + // non-Relay writer (manual put-item, schema-migration tool, etc.) can + // produce a row whose 'item' is the wrong type. The downstream JSON + // deserializer would then attempt boost::json::parse(""), which throws + // out of the evaluation path. + auto const& serialized = it->second.GetS(); + if (serialized.empty()) { + return tl::make_unexpected( + Error{"DynamoDB 'item' attribute is empty or not of type S"}); + } + + return SerializedItemDescriptor::Present(0, serialized); } ISerializedDataReader::AllResult DynamoDBDataSource::All( @@ -115,9 +128,16 @@ ISerializedDataReader::AllResult DynamoDBDataSource::All( return tl::make_unexpected( Error{"DynamoDB row missing expected 'item' attribute"}); } + // See note in Get(): a non-String 'item' attribute silently + // produces an empty GetS() and a downstream JSON parse throw. + auto const& serialized = item_it->second.GetS(); + if (serialized.empty()) { + return tl::make_unexpected( + Error{"DynamoDB 'item' attribute is empty or not of type S"}); + } items.emplace( key_it->second.GetS(), - SerializedItemDescriptor::Present(0, item_it->second.GetS())); + SerializedItemDescriptor::Present(0, serialized)); } auto const& last_key = result.GetLastEvaluatedKey(); diff --git a/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp index 9578ce73f..22f0de8f0 100644 --- a/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp +++ b/libs/server-sdk-dynamodb-source/tests/dynamodb_source_test.cpp @@ -439,6 +439,34 @@ TEST_F(DynamoDBTests, AllReturnsErrorWhenRowIsMissingItemAttribute) { ASSERT_FALSE(result); } +// Bug-proving: DynamoDB does not enforce a type schema on non-key attributes, +// so a row whose `item` attribute was written as a Number (not a String) is +// readable. AttributeValue::GetS() on a Number-typed value silently returns +// the default-constructed empty string. Without a type check, the source +// returns Present(0, "") and the empty string flows into +// JsonDeserializer::DeserializeJsonDescriptor's boost::json::parse, which +// throws boost::system::system_error -- the throw is uncaught all the way to +// the SDK's evaluation entry point. The source must surface this as a +// structured error instead. +TEST_F(DynamoDBTests, GetReturnsErrorWhenItemAttributeIsNotString) { + WithPrefixedClient(prefix_, [&](auto const& client) { + client.PutRowWithNumericItem("features", "foo"); + }); + + auto const result = source->Get(FlagKind{}, "foo"); + ASSERT_FALSE(result); +} + +TEST_F(DynamoDBTests, AllReturnsErrorWhenItemAttributeIsNotString) { + WithPrefixedClient(prefix_, [&](auto const& client) { + client.PutFlag(Flag{"foo", 1, true}); + client.PutRowWithNumericItem("features", "bar"); + }); + + auto const result = source->All(FlagKind{}); + ASSERT_FALSE(result); +} + TEST_F(DynamoDBTests, IdentityReturnsDynamodb) { ASSERT_EQ(source->Identity(), "dynamodb"); } diff --git a/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp index 397cb3ddb..1a1b9cc71 100644 --- a/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp +++ b/libs/server-sdk-dynamodb-source/tests/prefixed_dynamodb_client.hpp @@ -128,6 +128,32 @@ class PrefixedDynamoDBClient { PutRaw(Prefixed(ns_suffix), key, std::nullopt); } + // Writes a row where the `item` attribute is stored as a DynamoDB Number + // (type `N`), not a String (type `S`). DynamoDB does not enforce a type + // schema on non-key attributes, so a non-Relay writer (manual put-item, + // schema-migration tool, compromised writer) can produce rows with the + // wrong attribute type. Used to verify the source surfaces this as a + // structured error instead of silently producing an empty payload. + void PutRowWithNumericItem(std::string const& ns_suffix, + std::string const& key) const { + Aws::DynamoDB::Model::PutItemRequest request; + request.SetTableName(table_name_); + request.AddItem("namespace", Aws::DynamoDB::Model::AttributeValue{ + Prefixed(ns_suffix)}); + request.AddItem("key", Aws::DynamoDB::Model::AttributeValue{key}); + + Aws::DynamoDB::Model::AttributeValue numeric_item; + numeric_item.SetN("12345"); + request.AddItem("item", numeric_item); + + auto outcome = client_.PutItem(request); + if (!outcome.IsSuccess()) { + FAIL() << "couldn't put DynamoDB item ns=" << Prefixed(ns_suffix) + << " key=" << key << ": " + << outcome.GetError().GetMessage(); + } + } + private: std::string Prefixed(std::string const& base) const { if (prefix_.empty()) {