Backport YDB #41435

Author: Aleksandr Usenko

contrib/ydb/core/cms/console/console__replace_yaml_config.cpp

@@ -307,7 +307,7 @@ class TConfigsManager::TTxReplaceDatabaseYamlConfig
                 DoInternalAudit(txc, ctx);
 
                 db.Table<Schema::DatabaseYamlConfigs>().Key(TargetDatabase, Version + 1)
-                    .Update<Schema::DatabaseYamlConfigs::Config>(Config);
+                    .Update<Schema::DatabaseYamlConfigs::Config>(UpdatedDatabaseConfig);
 
                 /* Later we shift this boundary to support rollback and history */
                 db.Table<Schema::DatabaseYamlConfigs>().Key(TargetDatabase, Version)

contrib/ydb/core/cms/console/console_configs_manager.cpp

@@ -74,10 +74,9 @@ void TConfigsManager::ReplaceMainConfigMetadata(const TString &config, bool forc
 
 void TConfigsManager::ValidateMainConfig(TUpdateConfigOpContext& opCtx) {
     try {
+        // Re-applying an unchanged body with the same version is silently accepted
+        // (idempotent fast path)
         if (opCtx.UpdatedConfig != MainYamlConfig || YamlDropped) {
-            auto tree = NFyaml::TDocument::Parse(opCtx.UpdatedConfig);
-            auto resolved = NYamlConfig::ResolveAll(tree);
-
             if (ClusterName != opCtx.Cluster) {
                 ythrow yexception() << "ClusterName mismatch"
                     << " expected " << ClusterName
@@ -90,6 +89,9 @@ void TConfigsManager::ValidateMainConfig(TUpdateConfigOpContext& opCtx) {
                     << " but got " << opCtx.Version;
             }
 
+            auto tree = NFyaml::TDocument::Parse(opCtx.UpdatedConfig);
+            auto resolved = NYamlConfig::ResolveAll(tree);
+
             TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> unknownFieldsCollector = new NYamlConfig::TBasicUnknownFieldsCollector;
 
             std::vector<TString> errors;
@@ -135,7 +137,12 @@ void TConfigsManager::ReplaceDatabaseConfigMetadata(const TString &config, bool
         if (!force) {
             opCtx.Version = metadata.Version.value_or(0);
         } else {
-            opCtx.Version = YamlVersion;
+            ui32 currentVersion = 0;
+            if (auto it = DatabaseYamlConfigs.find(opCtx.TargetDatabase); it != DatabaseYamlConfigs.end())
+            {
+                currentVersion = it->second.Version;
+            }
+            opCtx.Version = currentVersion;
         }
 
         opCtx.UpdatedConfig = NYamlConfig::ReplaceMetadata(config, NYamlConfig::TDatabaseMetadata{
@@ -150,10 +157,22 @@ void TConfigsManager::ReplaceDatabaseConfigMetadata(const TString &config, bool
 void TConfigsManager::ValidateDatabaseConfig(TUpdateDatabaseConfigOpContext& opCtx) {
     try {
         TString currentConfig;
+        ui32 currentVersion = 0;
+
         if (auto it = DatabaseYamlConfigs.find(opCtx.TargetDatabase); it != DatabaseYamlConfigs.end()) {
             currentConfig = it->second.Config;
+            currentVersion = it->second.Version;
         }
+
+        // Re-applying an unchanged body with the same version is silently accepted
+        // (idempotent fast path)
         if (opCtx.UpdatedConfig != currentConfig) {
+            if (opCtx.Version != currentVersion) {
+                ythrow yexception() << "Version mismatch"
+                    << " expected " << currentVersion
+                    << " but got " << opCtx.Version;
+            }
+
             auto databaseTree = NFyaml::TDocument::Parse(opCtx.UpdatedConfig);
             auto databaseConfig = NYamlConfig::ParseConfig(databaseTree);
 

contrib/ydb/core/cms/console/console_configs_provider.cpp

@@ -745,8 +745,7 @@ bool TConfigsProvider::CheckSubscription(TInMemorySubscription::TPtr subscriptio
     subscription->VolatileYamlConfigHashes = VolatileYamlConfigHashes;
 
     if (auto it = DatabaseYamlConfigs.find(subscription->Tenant); it != DatabaseYamlConfigs.end()) {
-        // FIXME: handle version change correctly, instead of always sending on first update
-        if (!subscription->DatabaseYamlConfigVersion || *subscription->DatabaseYamlConfigVersion != it->second.Version || !subscription->FirstUpdateSent) {
+        if (!subscription->DatabaseYamlConfigVersion || *subscription->DatabaseYamlConfigVersion != it->second.Version) {
             subscription->DatabaseYamlConfigVersion = it->second.Version;
             request->Record.SetDatabaseYamlConfig(it->second.Config);
         } else {

contrib/ydb/core/cms/console/console_ut_configs.cpp

@@ -874,6 +874,72 @@ const TString DATABASE_2_YAML_CONFIG_2_UPDATED = R"(
     some_removed_feature_flag_example: true
 )";
 
+const TString VERSIONED_MAIN_CONFIG_V0_STALE = R"(
+---
+metadata:
+  kind: MainConfig
+  cluster: ""
+  version: 0
+config:
+  log_config:
+    cluster_name: clusterA
+)";
+
+const TString VERSIONED_MAIN_CONFIG_V1 = R"(
+---
+metadata:
+  kind: MainConfig
+  cluster: ""
+  version: 1
+config:
+  log_config:
+    cluster_name: clusterA
+)";
+
+const TString VERSIONED_MAIN_CONFIG_V2 = R"(
+---
+metadata:
+  kind: MainConfig
+  cluster: ""
+  version: 2
+config:
+  log_config:
+    cluster_name: clusterB
+)";
+
+const TString VERSIONED_DATABASE_CONFIG_V0_STALE = R"(
+---
+metadata:
+  kind: DatabaseConfig
+  database: "/dc-1/users/tenant-1"
+  version: 0
+config:
+  feature_flags:
+    some_removed_feature_flag_example: true
+)";
+
+const TString VERSIONED_DATABASE_CONFIG_V1 = R"(
+---
+metadata:
+  kind: DatabaseConfig
+  database: "/dc-1/users/tenant-1"
+  version: 1
+config:
+  feature_flags:
+    some_removed_feature_flag_example: true
+)";
+
+const TString VERSIONED_DATABASE_CONFIG_V2 = R"(
+---
+metadata:
+  kind: DatabaseConfig
+  database: "/dc-1/users/tenant-1"
+  version: 2
+config:
+  feature_flags:
+    some_removed_feature_flag_example: false
+)";
+
 
 void InitializeTestConfigItems()
 {
@@ -1116,7 +1182,101 @@ TVector<ui64> CheckConfigureLogAffected(TTenantTestRuntime &runtime,
     return {reply->Record.GetAddedItemIds().begin(), reply->Record.GetAddedItemIds().end()};
 }
 
+void DoFetchMainConfigFromConsole(TTenantTestRuntime &runtime, TString &yamlConfig)
+{
+    auto *event = new TEvConsole::TEvGetAllConfigsRequest;
+    runtime.SendToConsole(event);
+
+    TAutoPtr<IEventHandle> handle;
+    auto response = runtime.GrabEdgeEventRethrow<TEvConsole::TEvGetAllConfigsResponse>(handle);
+
+    UNIT_ASSERT_C(response->Record.GetResponse().config_size() == 1,
+        "expected exactly one config in response");
+    UNIT_ASSERT_C(response->Record.GetResponse().identity_size() == 1,
+        "expected exactly one identity in response");
+    UNIT_ASSERT_C(response->Record.GetResponse().identity(0).type_case() == Ydb::DynamicConfig::ConfigIdentity::kCluster,
+        "expected kCluster identity in response");
+
+    yamlConfig = response->Record.GetResponse().config(0);
+}
+
+void DoFetchDatabaseConfigFromConsole(TTenantTestRuntime &runtime, const TString &databasePath, TString &yamlConfig)
+{
+    auto *event = new TEvConsole::TEvGetAllConfigsRequest;
+    event->Record.SetIngressDatabase(databasePath);
+    runtime.SendToConsole(event);
+
+    TAutoPtr<IEventHandle> handle;
+    auto response = runtime.GrabEdgeEventRethrow<TEvConsole::TEvGetAllConfigsResponse>(handle);
+
+    UNIT_ASSERT_C(response->Record.GetResponse().config_size() == 1,
+        "expected at least one config in response for database: " << databasePath);
+    UNIT_ASSERT_C(response->Record.GetResponse().identity_size() == 1,
+        "expected exactly one identity in response");
+    UNIT_ASSERT_C(response->Record.GetResponse().identity(0).type_case() == Ydb::DynamicConfig::ConfigIdentity::kDatabase,
+        "expected kDatabase identity in response");
+    UNIT_ASSERT_C(response->Record.GetResponse().identity(0).has_database(),
+        "kDatabase identity in response with no database set");
+    UNIT_ASSERT_EQUAL_C(response->Record.GetResponse().identity(0).database(), databasePath,
+        "database in response not match requested database");
+
+    yamlConfig = response->Record.GetResponse().config(0);
+}
+
+void DoReplaceYamlConfig(TTenantTestRuntime &runtime,
+                         const TString &yaml,
+                         Ydb::StatusIds::StatusCode expectedCode,
+                         const TString &errorSubstring = {},
+                         bool allowAbsentDatabase = false)
+{
+    auto *event = new TEvConsole::TEvReplaceYamlConfigRequest;
+    event->Record.MutableRequest()->set_config(yaml);
+    if (allowAbsentDatabase) {
+        event->Record.MutableRequest()->set_allow_absent_database(true);
+    }
+    runtime.SendToConsole(event);
+
+    TAutoPtr<IEventHandle> handle;
+    auto [success, error] = runtime.GrabEdgeEvents<
+        TEvConsole::TEvReplaceYamlConfigResponse,
+        TEvConsole::TEvGenericError>(handle);
+
+    if (expectedCode == Ydb::StatusIds::SUCCESS) {
+        UNIT_ASSERT_C(success != nullptr,
+            "expected success but got error: " <<
+            (error ? error->Record.ShortDebugString() : TString("no event")));
+    } else {
+        UNIT_ASSERT_C(error != nullptr,
+            "expected error " << static_cast<int>(expectedCode) << " but got success");
+        UNIT_ASSERT_VALUES_EQUAL(error->Record.GetYdbStatus(), expectedCode);
+        if (!errorSubstring.empty()) {
+            TString allMessages;
+            for (const auto& issue : error->Record.GetIssues()) {
+                allMessages += issue.message();
+                allMessages += ";";
+            }
+            UNIT_ASSERT_STRING_CONTAINS(allMessages, errorSubstring);
+        }
+    }
+}
+
+void DoEnsureMainConfigReplacedWith(TTenantTestRuntime &runtime, TString replaceConfig)
+{
+    TString consoleConfig;
+    TString expectedConfig = NYamlConfig::UpgradeMainConfigVersion(replaceConfig);
+
+    DoFetchMainConfigFromConsole(runtime, consoleConfig);
+    UNIT_ASSERT_VALUES_EQUAL_C(consoleConfig, expectedConfig, "CONSOLE config version not match replace config version");
+}
+
+void DoEnsureDatabaseConfigReplacedWith(TTenantTestRuntime &runtime, const TString& database, TString replaceConfig)
+{
+    TString consoleConfig;
+    TString expectedConfig = NYamlConfig::UpgradeDatabaseConfigVersion(replaceConfig);
 
+    DoFetchDatabaseConfigFromConsole(runtime, database, consoleConfig);
+    UNIT_ASSERT_VALUES_EQUAL_C(consoleConfig, expectedConfig, "CONSOLE config version not match replace config version");
+}
 
 } // anonymous namespace
 
@@ -4520,6 +4680,93 @@ Y_UNIT_TEST_SUITE(TConsoleInMemoryConfigSubscriptionTests) {
         UNIT_ASSERT(notification->Get()->Record.GetVolatileConfigs()[0].GetNotChanged());
         UNIT_ASSERT(notification->Get()->Record.GetVolatileConfigs()[1].GetNotChanged());
     }
+
+    Y_UNIT_TEST(TestReplaceMainYamlConfigVersionCheck) {
+        TTenantTestRuntime runtime(DefaultConsoleTestConfig());
+
+        // Fresh runtime: YamlVersion = 0. Per documentation, the client must send
+        // the *current* stored version. A version ahead of stored is rejected.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V1,
+            Ydb::StatusIds::BAD_REQUEST, "Version mismatch");
+
+        // version: 0 matches stored YamlVersion = 0 — accepted; YamlVersion becomes 1.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V0_STALE,
+            Ydb::StatusIds::SUCCESS);
+        DoEnsureMainConfigReplacedWith(runtime, VERSIONED_MAIN_CONFIG_V0_STALE);
+
+        // version: 2 with stored = 1 — rejected.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V2,
+            Ydb::StatusIds::BAD_REQUEST, "Version mismatch");
+
+        // Re-applying the same body with the stale version: 0 is silently
+        // accepted (documented idempotent behaviour: wire == stored - 1 with
+        // identical content). YamlVersion stays at 1.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V0_STALE,
+            Ydb::StatusIds::SUCCESS);
+        DoEnsureMainConfigReplacedWith(runtime, VERSIONED_MAIN_CONFIG_V0_STALE);
+
+        // version: 1 matches stored YamlVersion = 1 — accepted; YamlVersion becomes 2.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V1,
+            Ydb::StatusIds::SUCCESS);
+        DoEnsureMainConfigReplacedWith(runtime, VERSIONED_MAIN_CONFIG_V1);
+
+        // version: 2 with a differing body now matches stored — accepted; YamlVersion = 3.
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V2,
+            Ydb::StatusIds::SUCCESS);
+        DoEnsureMainConfigReplacedWith(runtime, VERSIONED_MAIN_CONFIG_V2);
+    }
+
+    Y_UNIT_TEST(TestReplaceDatabaseYamlConfigVersionCheck) {
+        NKikimrConfig::TAppConfig appcfg;
+        appcfg.MutableFeatureFlags()->SetDatabaseYamlConfigAllowed(true);
+        TTenantTestRuntime runtime(MultipleTenantsConsoleTestConfig(), appcfg);
+
+        const TString DATABASE = "/dc-1/users/tenant-1";
+
+        // ValidateDatabaseConfig appends the database config into the main config
+        // and re-validates the result, which requires a non-empty MainYamlConfig.
+        // Seed a minimal main config first (fresh YamlVersion = 0, so wire = 0).
+        DoReplaceYamlConfig(runtime, VERSIONED_MAIN_CONFIG_V0_STALE,
+            Ydb::StatusIds::SUCCESS);
+
+        // Fresh runtime: no per-database config stored, so currentVersion = 0.
+        // Per documentation, the wire version must equal the stored version.
+        // Sending version: 1 must be rejected.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V1,
+            Ydb::StatusIds::BAD_REQUEST, "Version mismatch",
+            /* allowAbsentDatabase = */ true);
+
+        // version: 0 matches stored currentVersion = 0 — accepted; stored version becomes 1.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V0_STALE,
+            Ydb::StatusIds::SUCCESS, {},
+            /* allowAbsentDatabase = */ true);
+        DoEnsureDatabaseConfigReplacedWith(runtime, DATABASE, VERSIONED_DATABASE_CONFIG_V0_STALE);
+
+        // version: 2 with stored = 1 — rejected.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V2,
+            Ydb::StatusIds::BAD_REQUEST, "Version mismatch",
+            /* allowAbsentDatabase = */ true);
+
+        // Re-applying the same body with the stale version: 0 is silently
+        // accepted (documented idempotent behaviour: wire == stored - 1 with
+        // identical content). Stored version stays at 1.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V0_STALE,
+            Ydb::StatusIds::SUCCESS, {},
+            /* allowAbsentDatabase = */ true);
+        DoEnsureDatabaseConfigReplacedWith(runtime, DATABASE, VERSIONED_DATABASE_CONFIG_V0_STALE);
+
+        // version: 1 matches stored = 1 — accepted; stored version becomes 2.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V1,
+            Ydb::StatusIds::SUCCESS, {},
+            /* allowAbsentDatabase = */ true);
+        DoEnsureDatabaseConfigReplacedWith(runtime, DATABASE, VERSIONED_DATABASE_CONFIG_V1);
+
+        // version: 2 with a differing body now matches stored = 2 — accepted; version = 3.
+        DoReplaceYamlConfig(runtime, VERSIONED_DATABASE_CONFIG_V2,
+            Ydb::StatusIds::SUCCESS, {},
+            /* allowAbsentDatabase = */ true);
+        DoEnsureDatabaseConfigReplacedWith(runtime, DATABASE, VERSIONED_DATABASE_CONFIG_V2);
+    }
 }
 
 Y_UNIT_TEST_SUITE(TConsoleConfigHelpersTests) {

contrib/ydb/library/yaml_config/public/yaml_config.cpp

@@ -895,6 +895,13 @@ TString UpgradeStorageConfigVersion(const TString& config) {
     return ReplaceMetadata(config, metadata);
 }
 
+TString UpgradeDatabaseConfigVersion(const TString& config) {
+    auto metadata = GetDatabaseMetadata(config);
+    Y_ENSURE(metadata.Version);
+    *metadata.Version = *metadata.Version + 1;
+    return ReplaceMetadata(config, metadata);
+}
+
 } // namespace NKikimr::NYamlConfig
 
 template <>

contrib/ydb/library/yaml_config/public/yaml_config.h

@@ -255,6 +255,11 @@ TString UpgradeMainConfigVersion(const TString& config);
  */
 TString UpgradeStorageConfigVersion(const TString& config);
 
+/**
+ * Takes valid DatabaseConfig and increases version exactly by one
+ */
+TString UpgradeDatabaseConfigVersion(const TString& config);
+
 /**
  * Replaces metadata in database config
  */