Skip to content

Commit f542580

Browse files
EnjectionEnjection
authored
24-2: Add basic dynconfig audit (#8123)
1 parent 82dba10 commit f542580

8 files changed

+182
-0
lines changed

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
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>
6+
#include <ydb/library/aclib/aclib.h>
57

68
namespace NKikimr::NConsole {
79

@@ -14,7 +16,9 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
1416
bool force)
1517
: TBase(self)
1618
, Config(ev->Get()->Record.GetRequest().config())
19+
, Peer(ev->Get()->Record.GetPeerName())
1720
, Sender(ev->Sender)
21+
, UserSID(NACLib::TUserToken(ev->Get()->Record.GetUserToken()).GetUserSID())
1822
, Force(force)
1923
, AllowUnknownFields(ev->Get()->Record.GetRequest().allow_unknown_fields())
2024
, DryRun(ev->Get()->Record.GetRequest().dry_run())
@@ -121,6 +125,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
121125
auto *issue = ev->Record.AddIssues();
122126
issue->set_severity(NYql::TSeverityIds::S_ERROR);
123127
issue->set_message(ex.what());
128+
ErrorReason = ex.what();
124129
Response = MakeHolder<NActors::IEventHandle>(Sender, ctx.SelfID, ev.Release());
125130
}
126131

@@ -134,6 +139,14 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
134139
ctx.Send(Response.Release());
135140

136141
if (!Error && Modify && !DryRun) {
142+
AuditLogReplaceConfigTransaction(
143+
/* peer = */ Peer,
144+
/* userSID = */ UserSID,
145+
/* oldConfig = */ Self->YamlConfig,
146+
/* newConfig = */ Config,
147+
/* reason = */ {},
148+
/* success = */ true);
149+
137150
Self->YamlVersion = Version + 1;
138151
Self->YamlConfig = UpdatedConfig;
139152
Self->YamlDropped = false;
@@ -142,19 +155,30 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
142155

143156
auto resp = MakeHolder<TConfigsProvider::TEvPrivate::TEvUpdateYamlConfig>(Self->YamlConfig);
144157
ctx.Send(Self->ConfigsProvider, resp.Release());
158+
} else if (Error && !DryRun) {
159+
AuditLogReplaceConfigTransaction(
160+
/* peer = */ Peer,
161+
/* userSID = */ UserSID,
162+
/* oldConfig = */ Self->YamlConfig,
163+
/* newConfig = */ Config,
164+
/* reason = */ ErrorReason,
165+
/* success = */ false);
145166
}
146167

147168
Self->TxProcessor->TxCompleted(this, ctx);
148169
}
149170

150171
private:
151172
const TString Config;
173+
const TString Peer;
152174
const TActorId Sender;
175+
const TString UserSID;
153176
const bool Force = false;
154177
const bool AllowUnknownFields = false;
155178
const bool DryRun = false;
156179
THolder<NActors::IEventHandle> Response;
157180
bool Error = false;
181+
TString ErrorReason;
158182
bool Modify = false;
159183
TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> UnknownFieldsCollector = nullptr;
160184
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
@@ -253,6 +253,7 @@ message TConfigureResponse {
253253
message TSetYamlConfigRequest {
254254
optional Ydb.DynamicConfig.SetConfigRequest Request = 1;
255255
optional bytes UserToken = 2;
256+
optional string PeerName = 3;
256257
}
257258

258259
message TSetYamlConfigResponse {
@@ -262,6 +263,7 @@ message TSetYamlConfigResponse {
262263
message TReplaceYamlConfigRequest {
263264
optional Ydb.DynamicConfig.ReplaceConfigRequest Request = 1;
264265
optional bytes UserToken = 2;
266+
optional string PeerName = 3;
265267
}
266268

267269
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)