Skip to content

Commit 1269ae5

Browse files
EnjectionEnjection
authored
Add basic dynconfig audit (#7574)
1 parent 13d5b4c commit 1269ae5

8 files changed

+182
-2
lines changed

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

Lines changed: 24 additions & 2 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)
@@ -37,7 +39,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
3739
{
3840
}
3941

40-
void DoAudit(TTransactionContext &txc, const TActorContext &ctx)
42+
void DoInternalAudit(TTransactionContext &txc, const TActorContext &ctx)
4143
{
4244
auto logData = NKikimrConsole::TLogRecordData{};
4345

@@ -118,7 +120,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
118120
hasForbiddenUnknown = !unknownFields.empty() && !AllowUnknownFields;
119121

120122
if (!DryRun && !hasForbiddenUnknown) {
121-
DoAudit(txc, ctx);
123+
DoInternalAudit(txc, ctx);
122124

123125
db.Table<Schema::YamlConfig>().Key(Version + 1)
124126
.Update<Schema::YamlConfig::Config>(UpdatedConfig)
@@ -154,6 +156,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
154156
Error = true;
155157
auto ev = MakeHolder<TEvConsole::TEvGenericError>();
156158
ev->Record.SetYdbStatus(Ydb::StatusIds::BAD_REQUEST);
159+
ErrorReason = "Unknown keys in config.";
157160
fillResponse(ev, NYql::TSeverityIds::S_ERROR);
158161
} else if (!Force) {
159162
auto ev = MakeHolder<TEvConsole::TEvReplaceYamlConfigResponse>();
@@ -170,6 +173,7 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
170173
auto *issue = ev->Record.AddIssues();
171174
issue->set_severity(NYql::TSeverityIds::S_ERROR);
172175
issue->set_message(ex.what());
176+
ErrorReason = ex.what();
173177
Response = MakeHolder<NActors::IEventHandle>(Sender, ctx.SelfID, ev.Release());
174178
}
175179

@@ -183,6 +187,14 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
183187
ctx.Send(Response.Release());
184188

185189
if (!Error && Modify && !DryRun) {
190+
AuditLogReplaceConfigTransaction(
191+
/* peer = */ Peer,
192+
/* userSID = */ UserSID,
193+
/* oldConfig = */ Self->YamlConfig,
194+
/* newConfig = */ Config,
195+
/* reason = */ {},
196+
/* success = */ true);
197+
186198
Self->YamlVersion = Version + 1;
187199
Self->YamlConfig = UpdatedConfig;
188200
Self->YamlDropped = false;
@@ -191,20 +203,30 @@ class TConfigsManager::TTxReplaceYamlConfig : public TTransactionBase<TConfigsMa
191203

192204
auto resp = MakeHolder<TConfigsProvider::TEvPrivate::TEvUpdateYamlConfig>(Self->YamlConfig);
193205
ctx.Send(Self->ConfigsProvider, resp.Release());
206+
} else if (Error && !DryRun) {
207+
AuditLogReplaceConfigTransaction(
208+
/* peer = */ Peer,
209+
/* userSID = */ UserSID,
210+
/* oldConfig = */ Self->YamlConfig,
211+
/* newConfig = */ Config,
212+
/* reason = */ ErrorReason,
213+
/* success = */ false);
194214
}
195215

196216
Self->TxProcessor->TxCompleted(this, ctx);
197217
}
198218

199219
private:
200220
const TString Config;
221+
const TString Peer;
201222
const TActorId Sender;
202223
const TString UserSID;
203224
const bool Force = false;
204225
const bool AllowUnknownFields = false;
205226
const bool DryRun = false;
206227
THolder<NActors::IEventHandle> Response;
207228
bool Error = false;
229+
TString ErrorReason;
208230
bool Modify = false;
209231
TSimpleSharedPtr<NYamlConfig::TBasicUnknownFieldsCollector> UnknownFieldsCollector = nullptr;
210232
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
@@ -259,6 +259,7 @@ message TConfigureResponse {
259259
message TSetYamlConfigRequest {
260260
optional Ydb.DynamicConfig.SetConfigRequest Request = 1;
261261
optional bytes UserToken = 2;
262+
optional string PeerName = 3;
262263
}
263264

264265
message TSetYamlConfigResponse {
@@ -268,6 +269,7 @@ message TSetYamlConfigResponse {
268269
message TReplaceYamlConfigRequest {
269270
optional Ydb.DynamicConfig.ReplaceConfigRequest Request = 1;
270271
optional bytes UserToken = 2;
272+
optional string PeerName = 3;
271273
}
272274

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