Skip to content

Commit

Permalink
SystemView Auth TableRange filter (#14023)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kunga
kunga authored Jan 31, 2025
1 parent c058518 commit 59315fe
Show file tree
Hide file tree
Showing 10 changed files with 1,010 additions and 242 deletions.
8 changes: 4 additions & 4 deletions ydb/core/kqp/ut/common/kqp_ut_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,19 +552,19 @@ TString ReformatYson(const TString& yson) {
return output.Str();
}

void CompareYson(const TString& expected, const TString& actual) {
UNIT_ASSERT_VALUES_EQUAL(ReformatYson(expected), ReformatYson(actual));
void CompareYson(const TString& expected, const TString& actual, const TString& message) {
UNIT_ASSERT_VALUES_EQUAL_C(ReformatYson(expected), ReformatYson(actual), message);
}

void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual) {
void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual, const TString& message) {
TStringStream ysonStream;
NYson::TYsonWriter writer(&ysonStream, NYson::EYsonFormat::Text);
NYql::IDataProvider::TFillSettings fillSettings;
bool truncated;
KikimrResultToYson(ysonStream, writer, actual, {}, fillSettings, truncated);
UNIT_ASSERT(!truncated);

CompareYson(expected, ysonStream.Str());
CompareYson(expected, ysonStream.Str(), message);
}

bool HasIssue(const NYql::TIssues& issues, ui32 code,
Expand Down
4 changes: 2 additions & 2 deletions ydb/core/kqp/ut/common/kqp_ut_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,8 @@ inline constexpr TStringBuf IndexWithSqlString(EIndexTypeSql type) {
}

TString ReformatYson(const TString& yson);
void CompareYson(const TString& expected, const TString& actual);
void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual);
void CompareYson(const TString& expected, const TString& actual, const TString& message = {});
void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual, const TString& message = {});

void CreateLargeTable(TKikimrRunner& kikimr, ui32 rowsPerShard, ui32 keyTextSize,
ui32 dataTextSize, ui32 batchSizeRows = 100, ui32 fillShardsCount = 8, ui32 largeTableKeysPerShard = 1000000);
Expand Down
70 changes: 59 additions & 11 deletions ydb/core/sys_view/auth/auth_scan_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,19 @@ class TAuthScanBase : public TScanActorBase<TDerived> {
TAuthScanBase(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId,
const TTableRange& tableRange, const TArrayRef<NMiniKQL::TKqpComputeContextBase::TColumn>& columns,
TIntrusiveConstPtr<NACLib::TUserToken> userToken,
bool requireUserAdministratorAccess)
bool requireUserAdministratorAccess, bool applyPathTableRange)
: TBase(ownerId, scanId, tableId, tableRange, columns)
, UserToken(std::move(userToken))
, RequireUserAdministratorAccess(requireUserAdministratorAccess)
{
if (applyPathTableRange) {
if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) {
PathFrom = cellsFrom[0].AsBuf();
}
if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) {
PathTo = cellsTo[0].AsBuf();
}
}
}

STFUNC(StateScan) {
Expand All @@ -81,16 +89,6 @@ class TAuthScanBase : public TScanActorBase<TDerived> {
return;
}

// TODO: support TableRange filter
if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) {
TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.From filter is not supported");
return;
}
if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) {
TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.To filter is not supported");
return;
}

auto& last = DeepFirstSearchStack.emplace_back();
last.Index = Max<size_t>(); // tenant root

Expand All @@ -108,6 +106,10 @@ class TAuthScanBase : public TScanActorBase<TDerived> {
auto& last = DeepFirstSearchStack.back();

if (last.Index == Max<size_t>()) { // tenant root
if ((PathFrom || PathTo) && ShouldSkipSubTree(TBase::TenantName)) {
DeepFirstSearchStack.pop_back();
continue;
}
NavigatePath(SplitPath(TBase::TenantName));
DeepFirstSearchStack.pop_back();
return;
Expand All @@ -122,6 +124,11 @@ class TAuthScanBase : public TScanActorBase<TDerived> {
}

last.Entry.Path.push_back(child.Name);
if ((PathFrom || PathTo) && ShouldSkipSubTree(CanonizePath(last.Entry.Path))) {
last.Entry.Path.pop_back();
continue;
}

NavigatePath(last.Entry.Path);
last.Entry.Path.pop_back();
return;
Expand Down Expand Up @@ -189,6 +196,46 @@ class TAuthScanBase : public TScanActorBase<TDerived> {

TBase::Send(MakeSchemeCacheID(), new TEvTxProxySchemeCache::TEvNavigateKeySet(request.Release()));
}

// this method only skip foolproof useless paths
// ignores from/to inclusive flags for simplicity
// ignores some boundary cases for simplicity
// precise check will be performed later on batch rows filtering
bool ShouldSkipSubTree(const TString& path) {
Y_DEBUG_ABORT_UNLESS(PathFrom || PathTo);

if (PathFrom) {
// example:
// PathFrom = "Dir2/SubDir2"
// skip:
// - "Dir1"
// - "Dir2/SubDir1"
// do not skip:
// - "Dir2"
// - "Dir3"

if (PathFrom > path && !PathFrom->StartsWith(path)) {
return true;
}
}

if (PathTo) {
// example:
// PathTo = "Dir2/SubDir2"
// skip:
// - "Dir3"
// - "Dir2/SubDir3"
// do not skip:
// - "Dir1"
// - "Dir2"

if (PathTo < path) {
return true;
}
}

return false;
}

virtual void FillBatch(NKqp::TEvKqpCompute::TEvScanData& batch, const TNavigate::TEntry& entry) = 0;

Expand All @@ -197,6 +244,7 @@ class TAuthScanBase : public TScanActorBase<TDerived> {

private:
bool RequireUserAdministratorAccess;
std::optional<TString> PathFrom, PathTo;
TVector<TTraversingChildren> DeepFirstSearchStack;
};

Expand Down
6 changes: 4 additions & 2 deletions ydb/core/sys_view/auth/group_members.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TGroupMembersScan : public TAuthScanBase<TGroupMembersScan> {
TGroupMembersScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId,
const TTableRange& tableRange, const TArrayRef<NMiniKQL::TKqpComputeContextBase::TColumn>& columns,
TIntrusiveConstPtr<NACLib::TUserToken> userToken)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true, false)
{
}

Expand All @@ -34,7 +34,9 @@ class TGroupMembersScan : public TAuthScanBase<TGroupMembersScan> {
TVector<std::pair<const TDomainInfo::TGroup*, const TString*>> memberships;
for (const auto& group : entry.DomainInfo->Groups) {
for (const auto& member : group.Members) {
memberships.emplace_back(&group, &member);
if (StringKeyIsInTableRange({group.Sid, member})) {
memberships.emplace_back(&group, &member);
}
}
}
SortBatch(memberships, [](const auto& left, const auto& right) {
Expand Down
6 changes: 4 additions & 2 deletions ydb/core/sys_view/auth/groups.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TGroupsScan : public TAuthScanBase<TGroupsScan> {
TGroupsScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId,
const TTableRange& tableRange, const TArrayRef<NMiniKQL::TKqpComputeContextBase::TColumn>& columns,
TIntrusiveConstPtr<NACLib::TUserToken> userToken)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true, false)
{
}

Expand All @@ -33,7 +33,9 @@ class TGroupsScan : public TAuthScanBase<TGroupsScan> {

TVector<const TDomainInfo::TGroup*> groups(::Reserve(entry.DomainInfo->Groups.size()));
for (const auto& group : entry.DomainInfo->Groups) {
groups.push_back(&group);
if (StringKeyIsInTableRange({group.Sid})) {
groups.push_back(&group);
}
}
SortBatch(groups, [](const auto* left, const auto* right) {
return left->Sid < right->Sid;
Expand Down
36 changes: 19 additions & 17 deletions ydb/core/sys_view/auth/owners.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TOwnersScan : public TAuthScanBase<TOwnersScan> {
TOwnersScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId,
const TTableRange& tableRange, const TArrayRef<NMiniKQL::TKqpComputeContextBase::TColumn>& columns,
TIntrusiveConstPtr<NACLib::TUserToken> userToken)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false, true)
{
}

Expand All @@ -34,26 +34,28 @@ class TOwnersScan : public TAuthScanBase<TOwnersScan> {

auto entryPath = CanonizePath(entry.Path);

for (auto& column : Columns) {
switch (column.Tag) {
case Schema::AuthOwners::Path::ColumnId:
cells.push_back(TCell(entryPath.data(), entryPath.size()));
break;
case Schema::AuthOwners::Sid::ColumnId:
if (entry.SecurityObject && entry.SecurityObject->HasOwnerSID()) {
cells.push_back(TCell(entry.SecurityObject->GetOwnerSID().data(), entry.SecurityObject->GetOwnerSID().size()));
} else {
if (StringKeyIsInTableRange({entryPath})) {
for (auto& column : Columns) {
switch (column.Tag) {
case Schema::AuthOwners::Path::ColumnId:
cells.push_back(TCell(entryPath.data(), entryPath.size()));
break;
case Schema::AuthOwners::Sid::ColumnId:
if (entry.SecurityObject && entry.SecurityObject->HasOwnerSID()) {
cells.push_back(TCell(entry.SecurityObject->GetOwnerSID().data(), entry.SecurityObject->GetOwnerSID().size()));
} else {
cells.emplace_back();
}
break;
default:
cells.emplace_back();
}
break;
default:
cells.emplace_back();
}
}

TArrayRef<const TCell> ref(cells);
batch.Rows.emplace_back(TOwnedCellVec::Make(ref));
cells.clear();
TArrayRef<const TCell> ref(cells);
batch.Rows.emplace_back(TOwnedCellVec::Make(ref));
cells.clear();
}

batch.Finished = false;
}
Expand Down
13 changes: 9 additions & 4 deletions ydb/core/sys_view/auth/permissions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TPermissionsScan : public TAuthScanBase<TPermissionsScan> {
TPermissionsScan(bool effective, const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId,
const TTableRange& tableRange, const TArrayRef<NMiniKQL::TKqpComputeContextBase::TColumn>& columns,
TIntrusiveConstPtr<NACLib::TUserToken> userToken)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false)
: TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false, true)
, Effective(effective)
{
}
Expand All @@ -36,6 +36,8 @@ class TPermissionsScan : public TAuthScanBase<TPermissionsScan> {
batch.Finished = false;
return;
}

auto entryPath = CanonizePath(entry.Path);

TVector<std::pair<TString, TString>> permissions;
for (const NACLibProto::TACE& ace : entry.SecurityObject->GetACL().GetACE()) {
Expand All @@ -45,10 +47,15 @@ class TPermissionsScan : public TAuthScanBase<TPermissionsScan> {
if (!Effective && ace.GetInherited()) {
continue;
}
if (!ace.HasSID()) {
continue;
}

auto acePermissions = ConvertACLMaskToYdbPermissionNames(ace.GetAccessRight());
for (const auto& permission : acePermissions) {
permissions.emplace_back(ace.HasSID() ? ace.GetSID() : TString{}, std::move(permission));
if (StringKeyIsInTableRange({entryPath, ace.GetSID(), permission})) {
permissions.emplace_back(ace.GetSID(), std::move(permission));
}
}
}
// Note: due to rights inheritance permissions may be duplicated
Expand All @@ -59,8 +66,6 @@ class TPermissionsScan : public TAuthScanBase<TPermissionsScan> {

TVector<TCell> cells(::Reserve(Columns.size()));

auto entryPath = CanonizePath(entry.Path);

for (const auto& [sid, permission] : permissions) {
for (auto& column : Columns) {
switch (column.Tag) {
Expand Down
17 changes: 4 additions & 13 deletions ydb/core/sys_view/auth/users.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,6 @@ class TUsersScan : public TScanActorBase<TUsersScan> {
}

void StartScan() {
// TODO: support TableRange filter
if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) {
TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.From filter is not supported");
return;
}
if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) {
TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.To filter is not supported");
return;
}

auto request = MakeHolder<TEvSchemeShard::TEvListUsers>();

LOG_TRACE_S(TlsActivationContext->AsActorContext(), NKikimrServices::SYSTEM_VIEWS,
Expand Down Expand Up @@ -99,6 +89,9 @@ class TUsersScan : public TScanActorBase<TUsersScan> {
if (!user.HasName() || !CanAccessUser(user.GetName())) {
continue;
}
if (!StringKeyIsInTableRange({user.GetName()})) {
continue;
}
users.push_back(&user);
}
SortBatch(users, [](const auto* left, const auto* right) {
Expand All @@ -111,9 +104,7 @@ class TUsersScan : public TScanActorBase<TUsersScan> {
for (auto& column : Columns) {
switch (column.Tag) {
case Schema::AuthUsers::Sid::ColumnId:
cells.push_back(user->HasName()
? TCell(user->GetName().data(), user->GetName().size())
: TCell());
cells.push_back(TCell(user->GetName().data(), user->GetName().size()));
break;
case Schema::AuthUsers::IsEnabled::ColumnId:
cells.push_back(user->HasIsEnabled()
Expand Down
49 changes: 49 additions & 0 deletions ydb/core/sys_view/common/scan_actor_base_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,55 @@ class TScanActorBase : public TActorBootstrapped<TDerived> {
SendBatch(std::move(batch));
}

bool StringKeyIsInTableRange(const TVector<TString>& key) const {
{
bool equalPrefixes = true;
for (size_t index : xrange(Min(TableRange.From.GetCells().size(), key.size()))) {
if (auto cellFrom = TableRange.From.GetCells()[index]; !cellFrom.IsNull()) {
int cmp = cellFrom.AsBuf().compare(key[index]);
if (cmp < 0) {
equalPrefixes = false;
break;
}
if (cmp > 0) {
return false;
}
// cmp == 0, prefixes are equal, go further
} else {
equalPrefixes = false;
break;
}
}
if (equalPrefixes && !TableRange.FromInclusive) {
return false;
}
}

if (TableRange.To.GetCells().size()) {
bool equalPrefixes = true;
for (size_t index : xrange(Min(TableRange.To.GetCells().size(), key.size()))) {
if (auto cellTo = TableRange.To.GetCells()[index]; !cellTo.IsNull()) {
int cmp = cellTo.AsBuf().compare(key[index]);
if (cmp > 0) {
equalPrefixes = false;
break;
}
if (cmp < 0) {
return false;
}
// cmp == 0, prefixes are equal, go further
} else {
break;
}
}
if (equalPrefixes && !TableRange.ToInclusive) {
return false;
}
}

return true;
}

private:
virtual void ProceedToScan() = 0;

Expand Down
Loading

0 comments on commit 59315fe

Please sign in to comment.