Skip to content

Commit 8194541

Browse files
n00bcrackern00bcracker
committed
1. Filled db names in remained SchemeCache requests
2. Deleted filling db name in GRPC requests with default values 3. Deleted unnecessary CanonizePath calls for db names
1 parent 218bc07 commit 8194541

File tree

24 files changed

+106
-117
lines changed

24 files changed

+106
-117
lines changed

‎ydb/core/grpc_services/operation_helpers.cpp‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,6 @@ namespace NGRpcService {
2929
#define LOG_W(stream) LOG_WARN_S(*TlsActivationContext, NKikimrServices::TX_PROXY, LogPrefix << stream)
3030
#define LOG_E(stream) LOG_ERROR_S(*TlsActivationContext, NKikimrServices::TX_PROXY, LogPrefix << stream)
3131

32-
IEventBase* CreateNavigateForPath(const TString& path) {
33-
auto request = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
34-
35-
request->DatabaseName = path;
36-
37-
auto& entry = request->ResultSet.emplace_back();
38-
entry.Operation = NSchemeCache::TSchemeCacheNavigate::OpPath;
39-
entry.Path = ::NKikimr::SplitPath(path);
40-
entry.RedirectRequired = false;
41-
42-
return new TEvTxProxySchemeCache::TEvNavigateKeySet(request.Release());
43-
}
44-
4532
TActorId CreatePipeClient(ui64 id, const TActorContext& ctx) {
4633
NTabletPipe::TClientConfig clientConfig;
4734
clientConfig.RetryPolicy = {.RetryLimitCount = 3};

‎ydb/core/grpc_services/rpc_alter_table.cpp‎

Lines changed: 55 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -31,29 +31,6 @@ using namespace NActors;
3131
using namespace NConsole;
3232
using namespace Ydb;
3333

34-
static bool CheckAddIndexAccess(const NACLib::TUserToken& userToken, const NSchemeCache::TSchemeCacheNavigate* navigate) {
35-
bool isDatabaseEntry = true;
36-
37-
using TEntry = NSchemeCache::TSchemeCacheNavigate::TEntry;
38-
39-
for (const TEntry& entry : navigate->ResultSet) {
40-
if (!entry.SecurityObject) {
41-
continue;
42-
}
43-
if (isDatabaseEntry) { // first entry is always database
44-
isDatabaseEntry = false;
45-
continue;
46-
}
47-
48-
const ui32 access = NACLib::AlterSchema | NACLib::DescribeSchema;
49-
if (!entry.SecurityObject->CheckAccess(access, userToken)) {
50-
return false;
51-
}
52-
}
53-
54-
return true;
55-
}
56-
5734
using TEvAlterTableRequest = TGrpcRequestOperationCall<Ydb::Table::AlterTableRequest,
5835
Ydb::Table::AlterTableResponse>;
5936

@@ -64,6 +41,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
6441
public:
6542
TAlterTableRPC(IRequestOpCtx* msg)
6643
: TBase(msg)
44+
, DatabaseName(Request_->GetDatabaseName().GetOrElse(""))
6745
{}
6846

6947
void Bootstrap(const TActorContext &ctx) {
@@ -113,7 +91,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
11391
case EOp::Attribute:
11492
case EOp::AddChangefeed:
11593
case EOp::DropChangefeed:
116-
GetProxyServices();
94+
Navigate(GetProtoRequest()->path());;
11795
break;
11896

11997
case EOp::DropIndex:
@@ -128,8 +106,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
128106
private:
129107
void AlterStateWork(TAutoPtr<IEventHandle>& ev) {
130108
switch (ev->GetTypeRewrite()) {
131-
HFunc(TEvTxUserProxy::TEvAllocateTxIdResult, Handle);
132-
HFunc(TEvTxUserProxy::TEvGetProxyServicesResponse, Handle);
109+
hFunc(TEvTxUserProxy::TEvAllocateTxIdResult, Handle);
133110
HFunc(TEvTxProxySchemeCache::TEvNavigateKeySetResult, Handle);
134111
HFunc(NSchemeShard::TEvIndexBuilder::TEvCreateResponse, Handle);
135112
HFunc(NSchemeShard::TEvIndexBuilder::TEvGetResponse, Handle);
@@ -189,61 +166,46 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
189166
Send(MakeTxProxyID(), new TEvTxUserProxy::TEvAllocateTxId);
190167
}
191168

192-
void Handle(TEvTxUserProxy::TEvAllocateTxIdResult::TPtr& ev, const TActorContext& ctx) {
169+
void Handle(TEvTxUserProxy::TEvAllocateTxIdResult::TPtr& ev) {
193170
TXLOG_D("Handle TEvTxUserProxy::TEvAllocateTxIdResult");
194171

195172
const auto* msg = ev->Get();
196173
TxId = msg->TxId;
197174
LogPrefix = TStringBuilder() << "[AlterTableAddIndex " << SelfId() << " TxId# " << TxId << "] ";
198175

199-
Navigate(msg->Services.SchemeCache, ctx);
200-
}
201-
202-
void GetProxyServices() {
203-
using namespace NTxProxy;
204-
Send(MakeTxProxyID(), new TEvTxUserProxy::TEvGetProxyServicesRequest);
205-
}
206-
207-
void Handle(TEvTxUserProxy::TEvGetProxyServicesResponse::TPtr& ev, const TActorContext& ctx) {
208-
Navigate(ev->Get()->Services.SchemeCache, ctx);
176+
Navigate(GetProtoRequest()->path());
209177
}
210178

211-
void Navigate(const TActorId& schemeCache, const TActorContext& ctx) {
212-
DatabaseName = Request_->GetDatabaseName()
213-
.GetOrElse(DatabaseFromDomain(AppData()));
214-
215-
const auto& path = GetProtoRequest()->path();
216-
217-
const auto paths = NKikimr::SplitPath(path);
179+
void Navigate(const TString& path) {
180+
auto paths = NKikimr::SplitPath(path);
218181
if (paths.empty()) {
182+
auto& ctx = TlsActivationContext->AsActorContext();
219183
TString error = TStringBuilder() << "Failed to split table path " << path;
220184
Request_->RaiseIssue(NYql::TIssue(error));
221185
return Reply(Ydb::StatusIds::BAD_REQUEST, ctx);
222186
}
223187

224-
auto ev = CreateNavigateForPath(DatabaseName);
225-
{
226-
auto& entry = static_cast<TEvTxProxySchemeCache::TEvNavigateKeySet*>(ev)->Request->ResultSet.emplace_back();
227-
entry.Path = paths;
228-
}
188+
auto navigate = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
189+
navigate->DatabaseName = DatabaseName;
190+
191+
auto& entry = navigate->ResultSet.emplace_back();
192+
entry.Operation = NSchemeCache::TSchemeCacheNavigate::OpPath;
193+
entry.Path = std::move(paths);
229194

230-
Send(schemeCache, ev);
195+
Send(MakeSchemeCacheID(), new TEvTxProxySchemeCache::TEvNavigateKeySet(navigate));
231196
}
232197

233-
void Navigate(const TTableId& pathId, const TActorContext& /*ctx*/) {
234-
DatabaseName = Request_->GetDatabaseName()
235-
.GetOrElse(DatabaseFromDomain(AppData()));
236-
237-
auto ev = CreateNavigateForPath(DatabaseName);
238-
{
239-
auto& entry = static_cast<TEvTxProxySchemeCache::TEvNavigateKeySet*>(ev)->Request->ResultSet.emplace_back();
240-
entry.RequestType = NSchemeCache::TSchemeCacheNavigate::TEntry::ERequestType::ByTableId;
241-
entry.TableId = pathId;
242-
entry.ShowPrivatePath = true;
243-
entry.Operation = NSchemeCache::TSchemeCacheNavigate::OpList;
244-
}
198+
void Navigate(const TTableId& pathId) {
199+
auto navigate = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
200+
navigate->DatabaseName = DatabaseName;
201+
202+
auto& entry = navigate->ResultSet.emplace_back();
203+
entry.Operation = NSchemeCache::TSchemeCacheNavigate::OpList;
204+
entry.RequestType = NSchemeCache::TSchemeCacheNavigate::TEntry::ERequestType::ByTableId;
205+
entry.TableId = pathId;
206+
entry.ShowPrivatePath = true;
245207

246-
Send(MakeSchemeCacheID(), ev);
208+
Send(MakeSchemeCacheID(), new TEvTxProxySchemeCache::TEvNavigateKeySet(navigate));
247209
}
248210

249211
static bool IsChangefeedOperation(EOp type) {
@@ -278,8 +240,8 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
278240
return Reply(Ydb::StatusIds::SCHEME_ERROR, ctx);
279241
}
280242

281-
Y_ABORT_UNLESS(!resp->ResultSet.empty());
282-
const auto& entry = resp->ResultSet.back();
243+
Y_ABORT_UNLESS(resp->ResultSet.size() == 1);
244+
const auto& entry = resp->ResultSet.front();
283245

284246
switch (entry.Kind) {
285247
case NSchemeCache::TSchemeCacheNavigate::KindTable:
@@ -301,7 +263,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
301263

302264
switch (OpType) {
303265
case EOp::AddIndex:
304-
return AlterTableAddIndexOp(resp, ctx);
266+
return AlterTableAddIndexOp(entry, ctx);
305267
case EOp::Attribute:
306268
ResolvedPathId = resp->ResultSet.back().TableId.PathId;
307269
return AlterTable(ctx);
@@ -317,7 +279,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
317279
const auto& child = list->Children.at(0);
318280
AlterTable(ctx, CanonizePath(ChildPath(NKikimr::SplitPath(GetProtoRequest()->path()), child.Name)));
319281
} else {
320-
Navigate(entry.TableId, ctx);
282+
Navigate(entry.TableId);
321283
}
322284
break;
323285
default:
@@ -326,13 +288,32 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
326288
}
327289
}
328290

329-
void AlterTableAddIndexOp(const NSchemeCache::TSchemeCacheNavigate* resp, const TActorContext& ctx) {
330-
if (UserToken && !CheckAddIndexAccess(*UserToken, resp)) {
331-
TXLOG_W("Access check failed");
332-
return Reply(Ydb::StatusIds::UNAUTHORIZED, ctx);
291+
bool CheckAddIndexAccess(const NSchemeCache::TSchemeCacheNavigate::TEntry& entry, const TActorContext& ctx) {
292+
if (!UserToken || !entry.SecurityObject) {
293+
return true;
294+
}
295+
296+
const ui32 access = NACLib::AlterSchema | NACLib::DescribeSchema;
297+
if (entry.SecurityObject->CheckAccess(access, *UserToken)) {
298+
return true;
299+
}
300+
301+
TXLOG_W("Access check failed");
302+
Reply(Ydb::StatusIds::UNAUTHORIZED,
303+
TStringBuilder() << "Access denied"
304+
<< " for# " << UserToken->GetUserSID()
305+
<< ", path# " << CanonizePath(entry.Path)
306+
<< ", access# " << NACLib::AccessRightsToString(access),
307+
NKikimrIssues::TIssuesIds::ACCESS_DENIED, ctx);
308+
return false;
309+
}
310+
311+
void AlterTableAddIndexOp(const NSchemeCache::TSchemeCacheNavigate::TEntry& entry, const TActorContext& ctx) {
312+
if (!CheckAddIndexAccess(entry, ctx)) {
313+
return;
333314
}
334315

335-
auto domainInfo = resp->ResultSet.front().DomainInfo;
316+
const auto& domainInfo = entry.DomainInfo;
336317
if (!domainInfo) {
337318
TXLOG_E("Got empty domain info");
338319
return Reply(Ydb::StatusIds::INTERNAL_ERROR, ctx);
@@ -416,7 +397,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
416397
Die(ctx);
417398
}
418399

419-
void AlterTable(const TActorContext &ctx, const TMaybe<TString>& overridePath = {}) {
400+
void AlterTable(const TActorContext &ctx, const TMaybe<TString>& overridePath = {}) {
420401
const auto req = GetProtoRequest();
421402
std::unique_ptr<TEvTxUserProxy::TEvProposeTransaction> proposeRequest = CreateProposeTransaction();
422403
auto modifyScheme = proposeRequest->Record.MutableTransaction()->MutableModifyScheme();
@@ -433,7 +414,7 @@ class TAlterTableRPC : public TRpcSchemeRequestActor<TAlterTableRPC, TEvAlterTab
433414
}
434415

435416
ui64 TxId = 0;
436-
TString DatabaseName;
417+
const TString DatabaseName;
437418
TString LogPrefix;
438419
TIntrusiveConstPtr<NACLib::TUserToken> UserToken;
439420
TPathId ResolvedPathId;

‎ydb/core/grpc_services/rpc_describe_path.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class TBaseDescribe : public TRpcSchemeRequestActor<TDerived, TRequest> {
3737
private:
3838
void ResolvePath(const TActorContext& ctx) {
3939
auto request = MakeHolder<TSchemeCacheNavigate>();
40-
request->DatabaseName = NKikimr::CanonizePath(this->Request_->GetDatabaseName().GetOrElse(""));
40+
request->DatabaseName = this->Request_->GetDatabaseName().GetOrElse("");
4141

4242
auto& entry = request->ResultSet.emplace_back();
4343
entry.Operation = TSchemeCacheNavigate::OpList; // we need ListNodeEntry

‎ydb/core/grpc_services/rpc_describe_table.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class TDescribeTableRPC : public TRpcSchemeRequestActor<TDescribeTableRPC, TEvDe
5959
}
6060

6161
auto navigate = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
62-
navigate->DatabaseName = CanonizePath(Request_->GetDatabaseName().GetOrElse(""));
62+
navigate->DatabaseName = Request_->GetDatabaseName().GetOrElse("");
6363
auto& entry = navigate->ResultSet.emplace_back();
6464
entry.Path = paths;
6565
entry.Operation = NSchemeCache::TSchemeCacheNavigate::OpList;

‎ydb/core/grpc_services/rpc_import_data.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class TImportDataRPC: public TRpcRequestActor<TImportDataRPC, TEvImportDataReque
120120

121121
void ResolvePath() {
122122
auto request = MakeHolder<TNavigate>();
123-
request->DatabaseName = NKikimr::CanonizePath(GetDatabaseName());
123+
request->DatabaseName = GetDatabaseName();
124124

125125
auto& entry = request->ResultSet.emplace_back();
126126
entry.Operation = TNavigate::OpTable;

‎ydb/core/grpc_services/rpc_rate_limiter_api.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ class TRateLimiterControlRequest : public TRateLimiterRequest<TRateLimiterContro
169169
}
170170

171171
auto req = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
172-
req->DatabaseName = this->Request_->GetDatabaseName().GetOrElse(DatabaseFromDomain(AppData()));
172+
req->DatabaseName = this->Request_->GetDatabaseName().GetOrElse("");
173173
req->ResultSet.emplace_back();
174174
req->ResultSet.back().Path.swap(path);
175175
req->ResultSet.back().Operation = NSchemeCache::TSchemeCacheNavigate::OpPath;

‎ydb/core/grpc_services/rpc_read_rows.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class TReadRowsRPC : public TActorBootstrapped<TReadRowsRPC> {
266266
}
267267

268268
TString GetDatabase() {
269-
return Request->GetDatabaseName().GetOrElse(DatabaseFromDomain(AppData()));
269+
return Request->GetDatabaseName().GetOrElse("");
270270
}
271271

272272
const TString& GetTable() {

‎ydb/core/grpc_services/rpc_request_base.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class TRpcRequestActor: public TActorBootstrapped<TDerived> {
176176
if (name) {
177177
DatabaseName_.emplace(std::move(*name));
178178
} else {
179-
DatabaseName_.emplace(DatabaseFromDomain(AppData()));
179+
DatabaseName_.emplace("");
180180
}
181181
}
182182
return *DatabaseName_;

‎ydb/core/kafka_proxy/actors/kafka_produce_actor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ void TKafkaProduceActor::ProcessInitializationRequests(const TActorContext& ctx)
622622
request->ResultSet.emplace_back(entry);
623623
}
624624

625-
request->DatabaseName = CanonizePath(Context->DatabasePath);
625+
request->DatabaseName = Context->DatabasePath;
626626

627627
ctx.Send(MakeSchemeCacheID(), MakeHolder<TEvTxProxySchemeCache::TEvNavigateKeySet>(request.release()));
628628
}

‎ydb/core/kqp/gateway/behaviour/streaming_query/queries.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ class TDescribeStreamingQuerySchemeActor final : public TSchemeActorBase<TDescri
564564
LOG_D("Describe streaming query in database: " << Database);
565565

566566
auto request = std::make_unique<NSchemeCache::TSchemeCacheNavigate>();
567-
request->DatabaseName = CanonizePath(Database);
567+
request->DatabaseName = Database;
568568
request->UserToken = GetUserToken();
569569

570570
auto& entry = request->ResultSet.emplace_back();

0 commit comments

Comments
 (0)