wm: prevent empty has_app_name predicate (#46957)

3 files changed, 88 insertions, 36 deletions

diff --git a/ydb/core/kqp/workload_service/ut/kqp_has_app_name_ut.cpp b/ydb/core/kqp/workload_service/ut/kqp_has_app_name_ut.cpp
index a4ef96f6ac0..58298f71107 100644
--- a/ydb/core/kqp/workload_service/ut/kqp_has_app_name_ut.cpp
+++ b/ydb/core/kqp/workload_service/ut/kqp_has_app_name_ut.cpp
@@ -10,6 +10,34 @@ namespace NKikimr::NKqp {
 using namespace NWorkload;
 using namespace NYdb;
 
+namespace {
+
+constexpr TStringBuf USER_SID = "test@user";
+
+void CreatePoolAndClassifier(
+    TIntrusivePtr<IYdbSetup> ydb,
+    const TString& poolId,
+    const TString& classifierName,
+    const TString& appName)
+{
+    ydb->ExecuteSchemeQuery(TStringBuilder() << R"(
+        GRANT ALL ON `/)" << ydb->GetSettings().DomainName_ << R"(` TO `)" << USER_SID << R"(`;
+        CREATE RESOURCE POOL )" << poolId << R"( WITH (
+            CONCURRENT_QUERY_LIMIT=0
+        );
+        CREATE RESOURCE POOL CLASSIFIER )" << classifierName << R"( WITH (
+            RESOURCE_POOL=")" << poolId << R"(",
+            HAS_APP_NAME=")" << appName << R"("
+        );
+    )");
+}
+
+TQueryRunnerSettings QuerySettings(const TString& appName) {
+    return TQueryRunnerSettings().PoolId("").UserSID(TString(USER_SID)).ApplicationName(appName);
+}
+
+}  // anonymous namespace
+
 Y_UNIT_TEST_SUITE(TQueryClassifierHasAppName) {
 
     Y_UNIT_TEST(ShouldMatchAppName) {
@@ -44,45 +72,19 @@ Y_UNIT_TEST_SUITE(HasAppNameDdl) {
 
     Y_UNIT_TEST(TestHasAppNameClassifier) {
         auto ydb = TYdbSetupSettings().Create();
-
         const TString& poolId = "app_pool";
-        const TString& userSID = "test@user";
-        ydb->ExecuteSchemeQuery(TStringBuilder() << R"(
-            GRANT ALL ON `/)" << ydb->GetSettings().DomainName_ << R"(` TO `)" << userSID << R"(`;
-            CREATE RESOURCE POOL )" << poolId << R"( WITH (
-                CONCURRENT_QUERY_LIMIT=0
-            );
-            CREATE RESOURCE POOL CLASSIFIER app_classifier WITH (
-                RESOURCE_POOL=")" << poolId << R"(",
-                HAS_APP_NAME="my_app"
-            );
-        )");
-
-        auto matchSettings = TQueryRunnerSettings().PoolId("").UserSID(userSID).ApplicationName("my_app");
-        WaitForClassifierFail(ydb, matchSettings, poolId);
+        CreatePoolAndClassifier(ydb, poolId, "app_classifier", "my_app");
 
-        auto noMatchSettings = TQueryRunnerSettings().PoolId("").UserSID(userSID).ApplicationName("other_app");
-        WaitForClassifierSuccess(ydb, noMatchSettings);
+        WaitForClassifierFail(ydb, QuerySettings("my_app"), poolId);
+        WaitForClassifierSuccess(ydb, QuerySettings("other_app"));
     }
 
     Y_UNIT_TEST(TestAlterHasAppName) {
         auto ydb = TYdbSetupSettings().Create();
-
         const TString& poolId = "alter_pool";
-        const TString& userSID = "test@user";
-        ydb->ExecuteSchemeQuery(TStringBuilder() << R"(
-            GRANT ALL ON `/)" << ydb->GetSettings().DomainName_ << R"(` TO `)" << userSID << R"(`;
-            CREATE RESOURCE POOL )" << poolId << R"( WITH (
-                CONCURRENT_QUERY_LIMIT=0
-            );
-            CREATE RESOURCE POOL CLASSIFIER alter_classifier WITH (
-                RESOURCE_POOL=")" << poolId << R"(",
-                HAS_APP_NAME="old_app"
-            );
-        )");
+        CreatePoolAndClassifier(ydb, poolId, "alter_classifier", "old_app");
 
-        auto oldMatch = TQueryRunnerSettings().PoolId("").UserSID(userSID).ApplicationName("old_app");
-        WaitForClassifierFail(ydb, oldMatch, poolId);
+        WaitForClassifierFail(ydb, QuerySettings("old_app"), poolId);
 
         ydb->ExecuteSchemeQuery(R"(
             ALTER RESOURCE POOL CLASSIFIER alter_classifier SET (
@@ -90,11 +92,40 @@ Y_UNIT_TEST_SUITE(HasAppNameDdl) {
             );
         )");
 
-        auto newMatch = TQueryRunnerSettings().PoolId("").UserSID(userSID).ApplicationName("new_app");
-        WaitForClassifierFail(ydb, newMatch, poolId);
+        WaitForClassifierFail(ydb, QuerySettings("new_app"), poolId);
+        WaitForClassifierSuccess(ydb, QuerySettings("old_app"));
+    }
+
+    Y_UNIT_TEST(TestAlterHasAppNameEmptyClearsFilter) {
+        auto ydb = TYdbSetupSettings().Create();
+        const TString& poolId = "empty_pool";
+        CreatePoolAndClassifier(ydb, poolId, "empty_classifier", "old_app");
+
+        WaitForClassifierFail(ydb, QuerySettings("old_app"), poolId);
+
+        ydb->ExecuteSchemeQuery(R"(
+            ALTER RESOURCE POOL CLASSIFIER empty_classifier SET (
+                HAS_APP_NAME=""
+            );
+        )");
+
+        WaitForClassifierFail(ydb, QuerySettings("any_app"), poolId);
+        WaitForClassifierFail(ydb, QuerySettings(""), poolId);
+    }
+
+    Y_UNIT_TEST(TestResetHasAppNameClearsFilter) {
+        auto ydb = TYdbSetupSettings().Create();
+        const TString& poolId = "reset_pool";
+        CreatePoolAndClassifier(ydb, poolId, "reset_classifier", "old_app");
+
+        WaitForClassifierFail(ydb, QuerySettings("old_app"), poolId);
+
+        ydb->ExecuteSchemeQuery(R"(
+            ALTER RESOURCE POOL CLASSIFIER reset_classifier RESET (HAS_APP_NAME);
+        )");
 
-        auto oldNoMatch = TQueryRunnerSettings().PoolId("").UserSID(userSID).ApplicationName("old_app");
-        WaitForClassifierSuccess(ydb, oldNoMatch);
+        WaitForClassifierFail(ydb, QuerySettings("any_app"), poolId);
+        WaitForClassifierFail(ydb, QuerySettings(""), poolId);
     }
 }
 
diff --git a/ydb/core/resource_pools/resource_pool_classifier_settings.cpp b/ydb/core/resource_pools/resource_pool_classifier_settings.cpp
index 26df0373ded..c6d7cc211d7 100644
--- a/ydb/core/resource_pools/resource_pool_classifier_settings.cpp
+++ b/ydb/core/resource_pools/resource_pool_classifier_settings.cpp
@@ -23,7 +23,11 @@ void TClassifierSettings::TParser::operator()(TString* setting) const {
 }
 
 void TClassifierSettings::TParser::operator()(std::optional<TString>* setting) const {
-    *setting = Value;
+    if (Value.empty()) {
+        setting->reset();
+    } else {
+        *setting = Value;
+    }
 }
 
 void TClassifierSettings::TParser::operator()(std::optional<TRegexPredicate>* setting) const {
diff --git a/ydb/core/resource_pools/resource_pool_classifier_settings_ut.cpp b/ydb/core/resource_pools/resource_pool_classifier_settings_ut.cpp
index be23cb7bb7c..debd32c057f 100644
--- a/ydb/core/resource_pools/resource_pool_classifier_settings_ut.cpp
+++ b/ydb/core/resource_pools/resource_pool_classifier_settings_ut.cpp
@@ -85,6 +85,23 @@ Y_UNIT_TEST_SUITE(ResourcePoolClassifierTest) {
         UNIT_ASSERT_VALUES_EQUAL(std::visit(extractor, propertiesMap["has_path"]), "/Root/db/archive/*");
     }
 
+    Y_UNIT_TEST(OptionalStringEmptyResets) {
+        TClassifierSettings settings;
+        auto propertiesMap = settings.GetPropertiesMap();
+
+        std::visit(TClassifierSettings::TParser{"my_app"}, propertiesMap["has_app_name"]);
+        UNIT_ASSERT(settings.HasAppName.has_value());
+
+        std::visit(TClassifierSettings::TParser{""}, propertiesMap["has_app_name"]);
+        UNIT_ASSERT(!settings.HasAppName.has_value());
+
+        std::visit(TClassifierSettings::TParser{"test@user"}, propertiesMap["member_name"]);
+        UNIT_ASSERT(settings.MemberName.has_value());
+
+        std::visit(TClassifierSettings::TParser{""}, propertiesMap["member_name"]);
+        UNIT_ASSERT(!settings.MemberName.has_value());
+    }
+
     Y_UNIT_TEST(PredicateExtractingEmpty) {
         TClassifierSettings settings;
         auto propertiesMap = settings.GetPropertiesMap();