Merge 67f3063 into d1693e4

Enjection · web-flow · commit 97798d961fff · 2024-08-22T12:31:33.000Z
diff --git a/ydb/core/cms/console/console__replace_yaml_config.cpp b/ydb/core/cms/console/console__replace_yaml_config.cpp
@@ -1,5 +1,6 @@
 #include "console_configs_manager.h"
 #include "console_configs_provider.h"
+#include "console_audit.h"
 
 #include <ydb/core/tablet_flat/tablet_flat_executed.h>
 #include <ydb/library/aclib/aclib.h>
@@ -16,6 +17,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
                          bool force)
         : TBase(self)
         , Config(ev->Get()->Record.GetRequest().config())
+        , Peer(ev->Get()->Record.GetPeerName())
         , Sender(ev->Sender)
         , UserSID(NACLib::TUserToken(ev->Get()->Record.GetUserToken()).GetUserSID())
         , Force(force)
@@ -146,6 +148,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
             auto *issue = ev->Record.AddIssues();
             issue->set_severity(NYql::TSeverityIds::S_ERROR);
             issue->set_message(ex.what());
+            ErrorReason = ex.what();
             Response = MakeHolder<NActors::IEventHandle>(Sender, ctx.SelfID, ev.Release());
         }
 
@@ -159,6 +162,14 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
         ctx.Send(Response.Release());
 
         if (!Error && Modify && !DryRun) {
+            AuditLogReplaceConfigTransaction(
+                /* peer = */ Peer,
+                /* userSID = */ UserSID,
+                /* oldConfig = */ Self->YamlConfig,
+                /* newConfig = */ Config,
+                /* reason = */ {},
+                /* success = */ true);
+
             Self->YamlVersion = Version + 1;
             Self->YamlConfig = UpdatedConfig;
             Self->YamlDropped = false;
@@ -167,20 +178,30 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
 
             auto resp = MakeHolder<TConfigsProvider::TEvPrivate::TEvUpdateYamlConfig>(Self->YamlConfig);
             ctx.Send(Self->ConfigsProvider, resp.Release());
+        } else if (Error && !DryRun) {
+            AuditLogReplaceConfigTransaction(
+                /* peer = */ Peer,
+                /* userSID = */ UserSID,
+                /* oldConfig = */ Self->YamlConfig,
+                /* newConfig = */ Config,
+                /* reason = */ ErrorReason,
+                /* success = */ false);
         }
 
         Self->TxProcessor->TxCompleted(this, ctx);
     }
 
 private:
     const TString Config;
+    const TString Peer;
     const TActorId Sender;
     const TString UserSID;
     const bool Force = false;
     const bool AllowUnknownFields = false;
     const bool DryRun = false;
     THolder<NActors::IEventHandle> Response;
     bool Error = false;
+    TString ErrorReason;
     bool Modify = false;
     TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> UnknownFieldsCollector = nullptr;
     ui32 Version;
diff --git a/ydb/core/cms/console/console_audit.cpp b/ydb/core/cms/console/console_audit.cpp
@@ -0,0 +1,34 @@
+#include "console_audit.h"
+
+#include <ydb/core/audit/audit_log.h>
+#include <ydb/core/util/address_classifier.h>
+
+namespace NKikimr::NConsole {
+
+void AuditLogReplaceConfigTransaction(
+    const TString& peer,
+    const TString& userSID,
+    const TString& oldConfig,
+    const TString& newConfig,
+    const TString& reason,
+    bool success)
+{
+    static const TString COMPONENT_NAME = "console";
+
+    static const TString EMPTY_VALUE = "{none}";
+
+    auto peerName = NKikimr::NAddressClassifier::ExtractAddress(peer);
+
+    AUDIT_LOG(
+        AUDIT_PART("component", COMPONENT_NAME)
+        AUDIT_PART("remote_address", (!peerName.empty() ? peerName : EMPTY_VALUE))
+        AUDIT_PART("subject", (!userSID.empty() ? userSID : EMPTY_VALUE))
+        AUDIT_PART("status", TString(success ? "SUCCESS" : "ERROR"))
+        AUDIT_PART("reason", reason, !reason.empty())
+        AUDIT_PART("operation", TString("REPLACE DYNCONFIG"))
+        AUDIT_PART("old_config", oldConfig)
+        AUDIT_PART("new_config", newConfig)
+    );
+}
+
+} // namespace NKikimr::NConsole
diff --git a/ydb/core/cms/console/console_audit.h b/ydb/core/cms/console/console_audit.h
@@ -0,0 +1,15 @@
+#pragma once
+
+#include <util/generic/string.h>
+
+namespace NKikimr::NConsole {
+
+void AuditLogReplaceConfigTransaction(
+    const TString& peer,
+    const TString& userSID,
+    const TString& oldConfig,
+    const TString& newConfig,
+    const TString& reason,
+    bool success);
+
+} // namespace NKikimr::NConsole
diff --git a/ydb/core/cms/console/console_configs_manager.cpp b/ydb/core/cms/console/console_configs_manager.cpp
@@ -1,6 +1,7 @@
 #include "console_configs_manager.h"
 
 #include "configs_dispatcher.h"
+#include "console_audit.h"
 #include "console_configs_provider.h"
 #include "console_impl.h"
 #include "http.h"
@@ -974,4 +975,24 @@ void TConfigsManager::ScheduleLogCleanup(const TActorContext &ctx)
                     LogCleanupTimerCookieHolder.Get());
 }
 
+void TConfigsManager::HandleUnauthorized(TEvConsole::TEvReplaceYamlConfigRequest::TPtr &ev, const TActorContext &) {
+    AuditLogReplaceConfigTransaction(
+        /* peer = */ ev->Get()->Record.GetPeerName(),
+        /* userSID = */ ev->Get()->Record.GetUserToken(),
+        /* oldConfig = */ YamlConfig,
+        /* newConfig = */ ev->Get()->Record.GetRequest().config(),
+        /* reason = */ "Unauthorized.",
+        /* success = */ false);
+}
+
+void TConfigsManager::HandleUnauthorized(TEvConsole::TEvSetYamlConfigRequest::TPtr &ev, const TActorContext &) {
+    AuditLogReplaceConfigTransaction(
+        /* peer = */ ev->Get()->Record.GetPeerName(),
+        /* userSID = */ ev->Get()->Record.GetUserToken(),
+        /* oldConfig = */ YamlConfig,
+        /* newConfig = */ ev->Get()->Record.GetRequest().config(),
+        /* reason = */ "Unauthorized.",
+        /* success = */ false);
+}
+
 } // namespace NKikimr::NConsole
diff --git a/ydb/core/cms/console/console_configs_manager.h b/ydb/core/cms/console/console_configs_manager.h
@@ -152,6 +152,8 @@ class TConfigsManager : public TActorBootstrapped<TConfigsManager> {
     void Handle(TEvInterconnect::TEvNodesInfo::TPtr &ev, const TActorContext &ctx);
     void Handle(TEvConsole::TEvReplaceYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
     void Handle(TEvConsole::TEvSetYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
+    void HandleUnauthorized(TEvConsole::TEvReplaceYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
+    void HandleUnauthorized(TEvConsole::TEvSetYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
     void Handle(TEvConsole::TEvDropConfigRequest::TPtr & ev, const TActorContext & ctx);
     void Handle(TEvPrivate::TEvStateLoaded::TPtr &ev, const TActorContext &ctx);
     void Handle(TEvPrivate::TEvCleanupSubscriptions::TPtr &ev, const TActorContext &ctx);
@@ -160,9 +162,16 @@ class TConfigsManager : public TActorBootstrapped<TConfigsManager> {
 
     template <class T>
     void HandleWithRights(T &ev, const TActorContext &ctx) {
+        constexpr bool HasHandleUnauthorized = requires(T &ev) {
+            HandleUnauthorized(ev, ctx);
+        };
+
         if (CheckRights(ev->Get()->Record.GetUserToken())) {
             Handle(ev, ctx);
         } else {
+            if constexpr (HasHandleUnauthorized) {
+                HandleUnauthorized(ev, ctx);
+            }
             auto req = MakeHolder<TEvConsole::TEvUnauthorized>();
             ctx.Send(ev->Sender, req.Release());
         }
diff --git a/ydb/core/cms/console/ya.make b/ydb/core/cms/console/ya.make
@@ -11,6 +11,8 @@ SRCS(
     configs_dispatcher.h
     console.cpp
     console.h
+    console_audit.cpp
+    console_audit.h
     console_configs_manager.cpp
     console_configs_manager.h
     console_configs_provider.cpp
diff --git a/ydb/core/protos/console_config.proto b/ydb/core/protos/console_config.proto
@@ -258,6 +258,7 @@ message TConfigureResponse {
 message TSetYamlConfigRequest {
     optional Ydb.DynamicConfig.SetConfigRequest Request = 1;
     optional bytes UserToken = 2;
+    optional string PeerName = 3;
 }
 
 message TSetYamlConfigResponse {
@@ -267,6 +268,7 @@ message TSetYamlConfigResponse {
 message TReplaceYamlConfigRequest {
     optional Ydb.DynamicConfig.ReplaceConfigRequest Request = 1;
     optional bytes UserToken = 2;
+    optional string PeerName = 3;
 }
 
 message TReplaceYamlConfigResponse {
diff --git a/ydb/tests/functional/audit/test_auditlog.py b/ydb/tests/functional/audit/test_auditlog.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+import json
 import logging
 import os
 import subprocess
@@ -10,6 +11,7 @@
 from ydb.tests.oss.ydb_sdk_import import ydb
 
 from ydb import Driver, DriverConfig, SessionPool
+from ydb.draft import DynamicConfigClient
 from ydb.tests.library.harness.util import LogLevels
 from ydb.tests.library.harness.ydb_fixtures import ydb_database_ctx
 
@@ -365,3 +367,76 @@ def test_cloud_ids_are_logged(ydb_cluster, _database, prepared_test_env, _client
     for k, v in attrs.items():
         name = k if k != 'database_id' else 'resource_id'
         assert fr'''"{name}":"{v}"''' in capture_audit.captured
+
+
+def apply_config(pool, config):
+    client = DynamicConfigClient(pool._driver)
+    client.set_config(config, dry_run=False, allow_unknown_fields=False)
+
+
+@pytest.fixture(scope='module')
+def _good_dynconfig():
+    return '''
+---
+metadata:
+  kind: MainConfig
+  cluster: ""
+  version: 0
+config:
+  yaml_config_enabled: true
+allowed_labels:
+  node_id:
+    type: string
+  host:
+    type: string
+  tenant:
+    type: string
+selector_config: []
+    '''
+
+
+@pytest.fixture(scope='module')
+def _bad_dynconfig():
+    return '''
+---
+123metadata:
+  kind: MainConfig
+  cluster: ""
+  version: %s
+config:
+  yaml_config_enabled: true
+allowed_labels:
+  node_id:
+    type: string
+  host:
+    type: string
+  tenant:
+    type: string
+selector_config: []
+    '''
+
+
+def test_dynconfig(ydb_cluster, prepared_test_env, _client_session_pool_with_auth_root, _good_dynconfig):
+    config = _good_dynconfig
+    _table_path, capture_audit = prepared_test_env
+    with capture_audit:
+        _client_session_pool_with_auth_root.retry_operation_sync(apply_config, config=config)
+
+    print(capture_audit.captured, file=sys.stderr)
+    assert json.dumps(config) in capture_audit.captured
+
+
+@pytest.mark.parametrize('config_fixture', ["_bad_dynconfig", "_good_dynconfig"])
+@pytest.mark.parametrize('pool_fixture', ["_client_session_pool_with_auth_root", "_client_session_pool_no_auth", "_client_session_pool_bad_auth", "_client_session_pool_with_auth_other"])
+def test_broken_dynconfig(ydb_cluster, prepared_test_env, pool_fixture, config_fixture, request):
+    pool = request.getfixturevalue(pool_fixture)
+    config = request.getfixturevalue(config_fixture)
+    _table_path, capture_audit = prepared_test_env
+    with capture_audit:
+        try:
+            pool.retry_operation_sync(apply_config, config=config)
+        except ydb.issues.BadRequest:
+            pass
+
+    print(capture_audit.captured, file=sys.stderr)
+    assert json.dumps(config) in capture_audit.captured

Original file line number	Diff line number	Diff line change
`@@ -258,6 +258,7 @@ message TConfigureResponse {`
`258`	`258`	`message TSetYamlConfigRequest {`
`259`	`259`	`optional Ydb.DynamicConfig.SetConfigRequest Request = 1;`
`260`	`260`	`optional bytes UserToken = 2;`
	`261`	`+ optional string PeerName = 3;`
`261`	`262`	`}`
`262`	`263`
`263`	`264`	`message TSetYamlConfigResponse {`
`@@ -267,6 +268,7 @@ message TSetYamlConfigResponse {`
`267`	`268`	`message TReplaceYamlConfigRequest {`
`268`	`269`	`optional Ydb.DynamicConfig.ReplaceConfigRequest Request = 1;`
`269`	`270`	`optional bytes UserToken = 2;`
	`271`	`+ optional string PeerName = 3;`
`270`	`272`	`}`
`271`	`273`
`272`	`274`	`message TReplaceYamlConfigResponse {`