Skip to content

Commit cf0bcd6

Browse files
EnjectionEnjection
authored
24-3: Add basic dynconfig audit (#8155)
1 parent d1693e4 commit cf0bcd6

File tree

8 files changed

+179
-0
lines changed

8 files changed

+179
-0
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "console_configs_manager.h"
22
#include "console_configs_provider.h"
3+
#include "console_audit.h"
34

45
#include <ydb/core/tablet_flat/tablet_flat_executed.h>
56
#include <ydb/library/aclib/aclib.h>
@@ -16,6 +17,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
1617
bool force)
1718
: TBase(self)
1819
, Config(ev->Get()->Record.GetRequest().config())
20+
, Peer(ev->Get()->Record.GetPeerName())
1921
, Sender(ev->Sender)
2022
, UserSID(NACLib::TUserToken(ev->Get()->Record.GetUserToken()).GetUserSID())
2123
, Force(force)
@@ -146,6 +148,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
146148
auto *issue = ev->Record.AddIssues();
147149
issue->set_severity(NYql::TSeverityIds::S_ERROR);
148150
issue->set_message(ex.what());
151+
ErrorReason = ex.what();
149152
Response = MakeHolder<NActors::IEventHandle>(Sender, ctx.SelfID, ev.Release());
150153
}
151154

@@ -159,6 +162,14 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
159162
ctx.Send(Response.Release());
160163

161164
if (!Error && Modify && !DryRun) {
165+
AuditLogReplaceConfigTransaction(
166+
/* peer = */ Peer,
167+
/* userSID = */ UserSID,
168+
/* oldConfig = */ Self->YamlConfig,
169+
/* newConfig = */ Config,
170+
/* reason = */ {},
171+
/* success = */ true);
172+
162173
Self->YamlVersion = Version + 1;
163174
Self->YamlConfig = UpdatedConfig;
164175
Self->YamlDropped = false;
@@ -167,20 +178,30 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
167178

168179
auto resp = MakeHolder<TConfigsProvider::TEvPrivate::TEvUpdateYamlConfig>(Self->YamlConfig);
169180
ctx.Send(Self->ConfigsProvider, resp.Release());
181+
} else if (Error && !DryRun) {
182+
AuditLogReplaceConfigTransaction(
183+
/* peer = */ Peer,
184+
/* userSID = */ UserSID,
185+
/* oldConfig = */ Self->YamlConfig,
186+
/* newConfig = */ Config,
187+
/* reason = */ ErrorReason,
188+
/* success = */ false);
170189
}
171190

172191
Self->TxProcessor->TxCompleted(this, ctx);
173192
}
174193

175194
private:
176195
const TString Config;
196+
const TString Peer;
177197
const TActorId Sender;
178198
const TString UserSID;
179199
const bool Force = false;
180200
const bool AllowUnknownFields = false;
181201
const bool DryRun = false;
182202
THolder<NActors::IEventHandle> Response;
183203
bool Error = false;
204+
TString ErrorReason;
184205
bool Modify = false;
185206
TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> UnknownFieldsCollector = nullptr;
186207
ui32 Version;

‎ydb/core/cms/console/console_audit.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#include "console_audit.h"
2+
3+
#include <ydb/core/audit/audit_log.h>
4+
#include <ydb/core/util/address_classifier.h>
5+
6+
namespace NKikimr::NConsole {
7+
8+
void AuditLogReplaceConfigTransaction(
9+
const TString& peer,
10+
const TString& userSID,
11+
const TString& oldConfig,
12+
const TString& newConfig,
13+
const TString& reason,
14+
bool success)
15+
{
16+
static const TString COMPONENT_NAME = "console";
17+
18+
static const TString EMPTY_VALUE = "{none}";
19+
20+
auto peerName = NKikimr::NAddressClassifier::ExtractAddress(peer);
21+
22+
AUDIT_LOG(
23+
AUDIT_PART("component", COMPONENT_NAME)
24+
AUDIT_PART("remote_address", (!peerName.empty() ? peerName : EMPTY_VALUE))
25+
AUDIT_PART("subject", (!userSID.empty() ? userSID : EMPTY_VALUE))
26+
AUDIT_PART("status", TString(success ? "SUCCESS" : "ERROR"))
27+
AUDIT_PART("reason", reason, !reason.empty())
28+
AUDIT_PART("operation", TString("REPLACE DYNCONFIG"))
29+
AUDIT_PART("old_config", oldConfig)
30+
AUDIT_PART("new_config", newConfig)
31+
);
32+
}
33+
34+
} // namespace NKikimr::NConsole

‎ydb/core/cms/console/console_audit.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#pragma once
2+
3+
#include <util/generic/string.h>
4+
5+
namespace NKikimr::NConsole {
6+
7+
void AuditLogReplaceConfigTransaction(
8+
const TString& peer,
9+
const TString& userSID,
10+
const TString& oldConfig,
11+
const TString& newConfig,
12+
const TString& reason,
13+
bool success);
14+
15+
} // namespace NKikimr::NConsole

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "console_configs_manager.h"
22

33
#include "configs_dispatcher.h"
4+
#include "console_audit.h"
45
#include "console_configs_provider.h"
56
#include "console_impl.h"
67
#include "http.h"
@@ -974,4 +975,24 @@ void TConfigsManager::ScheduleLogCleanup(const TActorContext &ctx)
974975
LogCleanupTimerCookieHolder.Get());
975976
}
976977

978+
void TConfigsManager::HandleUnauthorized(TEvConsole::TEvReplaceYamlConfigRequest::TPtr &ev, const TActorContext &) {
979+
AuditLogReplaceConfigTransaction(
980+
/* peer = */ ev->Get()->Record.GetPeerName(),
981+
/* userSID = */ ev->Get()->Record.GetUserToken(),
982+
/* oldConfig = */ YamlConfig,
983+
/* newConfig = */ ev->Get()->Record.GetRequest().config(),
984+
/* reason = */ "Unauthorized.",
985+
/* success = */ false);
986+
}
987+
988+
void TConfigsManager::HandleUnauthorized(TEvConsole::TEvSetYamlConfigRequest::TPtr &ev, const TActorContext &) {
989+
AuditLogReplaceConfigTransaction(
990+
/* peer = */ ev->Get()->Record.GetPeerName(),
991+
/* userSID = */ ev->Get()->Record.GetUserToken(),
992+
/* oldConfig = */ YamlConfig,
993+
/* newConfig = */ ev->Get()->Record.GetRequest().config(),
994+
/* reason = */ "Unauthorized.",
995+
/* success = */ false);
996+
}
997+
977998
} // namespace NKikimr::NConsole

‎ydb/core/cms/console/console_configs_manager.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ class TConfigsManager : public TActorBootstrapped<TConfigsManager> {
152152
void Handle(TEvInterconnect::TEvNodesInfo::TPtr &ev, const TActorContext &ctx);
153153
void Handle(TEvConsole::TEvReplaceYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
154154
void Handle(TEvConsole::TEvSetYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
155+
void HandleUnauthorized(TEvConsole::TEvReplaceYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
156+
void HandleUnauthorized(TEvConsole::TEvSetYamlConfigRequest::TPtr & ev, const TActorContext & ctx);
155157
void Handle(TEvConsole::TEvDropConfigRequest::TPtr & ev, const TActorContext & ctx);
156158
void Handle(TEvPrivate::TEvStateLoaded::TPtr &ev, const TActorContext &ctx);
157159
void Handle(TEvPrivate::TEvCleanupSubscriptions::TPtr &ev, const TActorContext &ctx);
@@ -160,9 +162,16 @@ class TConfigsManager : public TActorBootstrapped<TConfigsManager> {
160162

161163
template <class T>
162164
void HandleWithRights(T &ev, const TActorContext &ctx) {
165+
constexpr bool HasHandleUnauthorized = requires(T &ev) {
166+
HandleUnauthorized(ev, ctx);
167+
};
168+
163169
if (CheckRights(ev->Get()->Record.GetUserToken())) {
164170
Handle(ev, ctx);
165171
} else {
172+
if constexpr (HasHandleUnauthorized) {
173+
HandleUnauthorized(ev, ctx);
174+
}
166175
auto req = MakeHolder<TEvConsole::TEvUnauthorized>();
167176
ctx.Send(ev->Sender, req.Release());
168177
}

‎ydb/core/cms/console/ya.make

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ SRCS(
1111
configs_dispatcher.h
1212
console.cpp
1313
console.h
14+
console_audit.cpp
15+
console_audit.h
1416
console_configs_manager.cpp
1517
console_configs_manager.h
1618
console_configs_provider.cpp

‎ydb/core/protos/console_config.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ message TConfigureResponse {
258258
message TSetYamlConfigRequest {
259259
optional Ydb.DynamicConfig.SetConfigRequest Request = 1;
260260
optional bytes UserToken = 2;
261+
optional string PeerName = 3;
261262
}
262263

263264
message TSetYamlConfigResponse {
@@ -267,6 +268,7 @@ message TSetYamlConfigResponse {
267268
message TReplaceYamlConfigRequest {
268269
optional Ydb.DynamicConfig.ReplaceConfigRequest Request = 1;
269270
optional bytes UserToken = 2;
271+
optional string PeerName = 3;
270272
}
271273

272274
message TReplaceYamlConfigResponse {

‎ydb/tests/functional/audit/test_auditlog.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# -*- coding: utf-8 -*-
2+
import json
23
import logging
34
import os
45
import subprocess
@@ -10,6 +11,7 @@
1011
from ydb.tests.oss.ydb_sdk_import import ydb
1112

1213
from ydb import Driver, DriverConfig, SessionPool
14+
from ydb.draft import DynamicConfigClient
1315
from ydb.tests.library.harness.util import LogLevels
1416
from ydb.tests.library.harness.ydb_fixtures import ydb_database_ctx
1517

@@ -365,3 +367,76 @@ def test_cloud_ids_are_logged(ydb_cluster, _database, prepared_test_env, _client
365367
for k, v in attrs.items():
366368
name = k if k != 'database_id' else 'resource_id'
367369
assert fr'''"{name}":"{v}"''' in capture_audit.captured
370+
371+
372+
def apply_config(pool, config):
373+
client = DynamicConfigClient(pool._driver)
374+
client.set_config(config, dry_run=False, allow_unknown_fields=False)
375+
376+
377+
@pytest.fixture(scope='module')
378+
def _good_dynconfig():
379+
return '''
380+
---
381+
metadata:
382+
kind: MainConfig
383+
cluster: ""
384+
version: 0
385+
config:
386+
yaml_config_enabled: true
387+
allowed_labels:
388+
node_id:
389+
type: string
390+
host:
391+
type: string
392+
tenant:
393+
type: string
394+
selector_config: []
395+
'''
396+
397+
398+
@pytest.fixture(scope='module')
399+
def _bad_dynconfig():
400+
return '''
401+
---
402+
123metadata:
403+
kind: MainConfig
404+
cluster: ""
405+
version: %s
406+
config:
407+
yaml_config_enabled: true
408+
allowed_labels:
409+
node_id:
410+
type: string
411+
host:
412+
type: string
413+
tenant:
414+
type: string
415+
selector_config: []
416+
'''
417+
418+
419+
def test_dynconfig(ydb_cluster, prepared_test_env, _client_session_pool_with_auth_root, _good_dynconfig):
420+
config = _good_dynconfig
421+
_table_path, capture_audit = prepared_test_env
422+
with capture_audit:
423+
_client_session_pool_with_auth_root.retry_operation_sync(apply_config, config=config)
424+
425+
print(capture_audit.captured, file=sys.stderr)
426+
assert json.dumps(config) in capture_audit.captured
427+
428+
429+
@pytest.mark.parametrize('config_fixture', ["_bad_dynconfig", "_good_dynconfig"])
430+
@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"])
431+
def test_broken_dynconfig(ydb_cluster, prepared_test_env, pool_fixture, config_fixture, request):
432+
pool = request.getfixturevalue(pool_fixture)
433+
config = request.getfixturevalue(config_fixture)
434+
_table_path, capture_audit = prepared_test_env
435+
with capture_audit:
436+
try:
437+
pool.retry_operation_sync(apply_config, config=config)
438+
except ydb.issues.BadRequest:
439+
pass
440+
441+
print(capture_audit.captured, file=sys.stderr)
442+
assert json.dumps(config) in capture_audit.captured

0 commit comments

Comments
 (0)