Skip to content

Commit 72fd04f

Browse files
n00bcrackern00bcracker
authored
Filled db names in SchemeCache requests and removed unnecessary CanonizePath (#27877)
1 parent 60f14e7 commit 72fd04f

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
@@ -164,7 +164,7 @@ class TRateLimiterControlRequest : public TRateLimiterRequest<TRateLimiterContro
164164
}
165165

166166
auto req = MakeHolder<NSchemeCache::TSchemeCacheNavigate>();
167-
req->DatabaseName = this->Request_->GetDatabaseName().GetOrElse(DatabaseFromDomain(AppData()));
167+
req->DatabaseName = this->Request_->GetDatabaseName().GetOrElse("");
168168
req->ResultSet.emplace_back();
169169
req->ResultSet.back().Path.swap(path);
170170
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)