Mark options that are allowed to be used in the database configuration (#14761)

Author: CyberROFL

ydb/core/cms/console/console_configs_manager.cpp

@@ -148,14 +148,30 @@ void TConfigsManager::ValidateDatabaseConfig(TUpdateDatabaseConfigOpContext& opC
             currentConfig = it->second.Config;
         }
         if (opCtx.UpdatedConfig != currentConfig) {
-            auto tree = NFyaml::TDocument::Parse(MainYamlConfig);
             auto databaseTree = NFyaml::TDocument::Parse(opCtx.UpdatedConfig);
-            NYamlConfig::AppendDatabaseConfig(tree, databaseTree);
-            auto resolved = NYamlConfig::ResolveAll(tree);
+            auto databaseConfig = NYamlConfig::ParseConfig(databaseTree);
 
             TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> unknownFieldsCollector = new NYamlConfig::TBasicUnknownFieldsCollector;
 
+            auto databaseCfg = NYamlConfig::YamlToProto(
+                databaseConfig.Config,
+                true,
+                false,
+                unknownFieldsCollector);
+
             std::vector<TString> errors;
+            NKikimr::NConfig::EValidationResult result = NKikimr::NConfig::ValidateDatabaseConfig(databaseCfg, errors);
+            if (result == NKikimr::NConfig::EValidationResult::Error) {
+                ythrow yexception() << errors.front();
+            }
+
+            // TODO: validate databaseConfig.AllowedLabels & databaseConfig.Selectors too
+
+            auto tree = NFyaml::TDocument::Parse(MainYamlConfig);
+            NYamlConfig::AppendDatabaseConfig(tree, databaseTree);
+            auto resolved = NYamlConfig::ResolveAll(tree);
+
+            errors.clear();
             for (auto& [_, config] : resolved.Configs) {
                 auto cfg = NYamlConfig::YamlToProto(
                     config.second,

ydb/core/config/protos/marker.proto

@@ -50,5 +50,8 @@ extend google.protobuf.FieldOptions {
     repeated string CopyTo = 82001;
     repeated string AsMap = 82002;
     repeated TAdvancedCopyTo AdvancedCopyTo = 82003;
+
+    // **Top-level** options marked with that label are allowed to be used in the database configuration.
+    optional bool AllowInDatabaseConfig = 82004;
 }
 

ydb/core/config/validation/validators.cpp

@@ -1,8 +1,13 @@
 #include "validators.h"
 
+#include <ydb/core/config/protos/marker.pb.h>
 #include <ydb/core/protos/blobstorage.pb.h>
 #include <ydb/core/protos/blobstorage_disk.pb.h>
 
+#include <library/cpp/protobuf/json/util.h>
+
+#include <util/string/builder.h>
+
 #include <map>
 #include <set>
 
@@ -161,6 +166,37 @@ EValidationResult ValidateStaticGroup(const NKikimrConfig::TAppConfig& current,
     return EValidationResult::Ok;
 }
 
+EValidationResult ValidateDatabaseConfig(const NKikimrConfig::TAppConfig& config, std::vector<TString>& msg) {
+    const auto* desc = config.GetDescriptor();
+    const auto* reflection = config.GetReflection();
+
+    for (int i = 0; i < desc->field_count(); i++) {
+        const auto* field = desc->field(i);
+
+        if (field->options().GetExtension(NKikimrConfig::NMarkers::AllowInDatabaseConfig)) {
+            continue;
+        }
+
+        if (!field->is_repeated()) {
+            if (!reflection->HasField(config, field)) {
+                continue;
+            }
+        } else {
+            if (!reflection->FieldSize(config, field)) {
+                continue;
+            }
+        }
+
+        auto fieldName = field->name();
+        NProtobufJson::ToSnakeCaseDense(&fieldName);
+        msg.push_back(TStringBuilder() << "'" << fieldName << "' "
+            << "is not allowed to be used in the database configuration");
+        return EValidationResult::Error;
+    }
+
+    return EValidationResult::Ok;
+}
+
 EValidationResult ValidateConfig(const NKikimrConfig::TAppConfig& config, std::vector<TString>& msg) {
     if (config.HasAuthConfig()) {
         NKikimr::NConfig::EValidationResult result = NKikimr::NConfig::ValidateAuthConfig(config.GetAuthConfig(), msg);

ydb/core/config/validation/validators.h

@@ -46,6 +46,10 @@ EValidationResult ValidateColumnShardConfig(
     const NKikimrConfig::TColumnShardConfig& columnShardConfig, 
     std::vector<TString>& msg);
 
+EValidationResult ValidateDatabaseConfig(
+    const NKikimrConfig::TAppConfig& config,
+    std::vector<TString>& msg);
+
 EValidationResult ValidateConfig(
     const NKikimrConfig::TAppConfig& config,
     std::vector<TString>& msg);

ydb/core/config/validation/validators_ut.cpp

@@ -2,6 +2,8 @@
 
 #include <ydb/core/protos/blobstorage.pb.h>
 #include <ydb/core/protos/blobstorage_disk.pb.h>
+#include <ydb/core/protos/feature_flags.pb.h>
+#include <ydb/core/protos/table_service_config.pb.h>
 
 #include <library/cpp/testing/unittest/registar.h>
 
@@ -406,3 +408,26 @@ Y_UNIT_TEST_SUITE(ConfigValidation) {
         UNIT_ASSERT_EQUAL(res, EValidationResult::Error);
     }
 }
+
+Y_UNIT_TEST_SUITE(DatabaseConfigValidation) {
+    Y_UNIT_TEST(AllowedFields) {
+        NKikimrConfig::TAppConfig config;
+        config.MutableFeatureFlags()->SetEnablePgSyntax(true);
+        config.MutableTableServiceConfig()->SetEnableStreamWrite(true);
+
+        std::vector<TString> err;
+        auto res = ValidateDatabaseConfig(config, err);
+        UNIT_ASSERT_VALUES_EQUAL(err.size(), 0);
+        UNIT_ASSERT_EQUAL(res, EValidationResult::Ok);
+    }
+
+    Y_UNIT_TEST(NotAllowedFields) {
+        auto [config, _] = PrepareStaticStorageTest();
+
+        std::vector<TString> err;
+        auto res = ValidateDatabaseConfig(config, err);
+        UNIT_ASSERT_VALUES_EQUAL(err.size(), 1);
+        UNIT_ASSERT_VALUES_EQUAL(err[0], "'blob_storage_config' is not allowed to be used in the database configuration");
+        UNIT_ASSERT_EQUAL(res, EValidationResult::Error);
+    }
+}

ydb/core/config/validation/ya.make

@@ -10,6 +10,7 @@ SRCS(
 PEERDIR(
     ydb/core/protos
     ydb/core/formats/arrow/serializer
+    library/cpp/protobuf/json
 )
 
 END()

ydb/core/protos/config.proto

@@ -2173,7 +2173,7 @@ message TAppConfig {
     //optional TLocalConfig LocalConfig = 23; DEPRECATED
     optional TDynamicNodeConfig DynamicNodeConfig = 24;
     optional NKikimrCms.TCmsConfig CmsConfig = 25;
-    optional TFeatureFlags FeatureFlags = 26;
+    optional TFeatureFlags FeatureFlags = 26 [(NMarkers.AllowInDatabaseConfig) = true];
     optional TSqsConfig SqsConfig = 27;
     optional NKikimrPQ.TPQConfig PQConfig = 28;
     optional NKikimrTenantPool.TTenantPoolConfig TenantPoolConfig = 29;
@@ -2184,7 +2184,7 @@ message TAppConfig {
     optional NKikimrProto.TKeyConfig KeyConfig = 35;
     optional NKikimrProto.TKeyConfig PDiskKeyConfig = 51;
     optional NKikimrNodeBroker.TConfig NodeBrokerConfig = 36;
-    optional TTableServiceConfig TableServiceConfig = 37;
+    optional TTableServiceConfig TableServiceConfig = 37 [(NMarkers.AllowInDatabaseConfig) = true];
     optional NKikimrSharedCache.TSharedCacheConfig SharedCacheConfig = 38; // dynamic configuration via cms
     optional TImmediateControlsConfig ImmediateControlsConfig = 39;
     optional TAllocatorConfig AllocatorConfig = 40;