Skip to content

Commit

Permalink
implement secret identification by name (#12641)
Browse files Browse the repository at this point in the history
  • Loading branch information
swalrus1 authored Dec 16, 2024
1 parent 0a53ed1 commit 01126ac
Show file tree
Hide file tree
Showing 12 changed files with 208 additions and 109 deletions.
17 changes: 9 additions & 8 deletions ydb/core/kqp/federated_query/kqp_federated_query_actors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ class TDescribeSecretsActor: public NActors::TActorBootstrapped<TDescribeSecrets
std::vector<TString> secretValues;
secretValues.reserve(SecretIds.size());
for (const auto& secretId: SecretIds) {
TString secretValue;
bool isFound = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretId), secretValue);
if (isFound) {
secretValues.push_back(secretValue);
auto secretValue = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretId));
if (secretValue.IsSuccess()) {
secretValues.push_back(secretValue.DetachResult());
continue;
}

Expand All @@ -32,10 +31,12 @@ class TDescribeSecretsActor: public NActors::TActorBootstrapped<TDescribeSecrets
return;
}

isFound = !secretIds.empty() && snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretIds[0]), secretValue);
if (isFound) {
secretValues.push_back(secretValue);
continue;
if (!secretIds.empty()) {
secretValue = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretIds[0]));
if (secretValue.IsSuccess()) {
secretValues.push_back(secretValue.DetachResult());
continue;
}
}

if (!AskSent) {
Expand Down
8 changes: 4 additions & 4 deletions ydb/core/tx/replication/controller/secret_resolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ class TSecretResolver: public TActorBootstrapped<TSecretResolver> {
void Handle(NMetadata::NProvider::TEvRefreshSubscriberData::TPtr& ev) {
const auto* snapshot = ev->Get()->GetSnapshotAs<NMetadata::NSecret::TSnapshot>();

TString secretValue;
if (!snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(SecretId), secretValue)) {
return Reply(false, TStringBuilder() << "Secret '" << SecretName << "' not found");
auto secretValue = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(SecretId));
if (secretValue.IsFail()) {
return Reply(false, secretValue.GetErrorMessage());
}

Reply(secretValue);
Reply(secretValue.DetachResult());
}

template <typename... Args>
Expand Down
16 changes: 9 additions & 7 deletions ydb/core/tx/tiering/tier/checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ void TTierPreparationActor::StartChecker() {
return;
}
auto g = PassAwayGuard();
for (auto&& tier : Objects) {
if (!Secrets->CheckSecretAccess(tier.GetAccessKey(), Context.GetExternalData().GetUserToken())) {
Controller->OnPreparationProblem("no access for secret: " + tier.GetAccessKey().DebugString());
return;
} else if (!Secrets->CheckSecretAccess(tier.GetSecretKey(), Context.GetExternalData().GetUserToken())) {
Controller->OnPreparationProblem("no access for secret: " + tier.GetSecretKey().DebugString());
return;
if (const auto& userToken = Context.GetExternalData().GetUserToken()) {
for (auto&& tier : Objects) {
if (!Secrets->CheckSecretAccess(tier.GetAccessKey(), *userToken)) {
Controller->OnPreparationProblem("no access for secret: " + tier.GetAccessKey().DebugString());
return;
} else if (!Secrets->CheckSecretAccess(tier.GetSecretKey(), *userToken)) {
Controller->OnPreparationProblem("no access for secret: " + tier.GetSecretKey().DebugString());
return;
}
}
}
Controller->OnPreparationFinished(std::move(Objects));
Expand Down
20 changes: 13 additions & 7 deletions ydb/core/tx/tiering/tier/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,22 @@ NMetadata::NInternal::TTableRecord TTierConfig::SerializeToRecord() const {
return result;
}

NKikimrSchemeOp::TS3Settings TTierConfig::GetPatchedConfig(
std::shared_ptr<NMetadata::NSecret::TSnapshot> secrets) const
{
NKikimrSchemeOp::TS3Settings TTierConfig::GetPatchedConfig(std::shared_ptr<NMetadata::NSecret::TSnapshot> secrets) const {
auto config = ProtoConfig.GetObjectStorage();
if (secrets) {
if (!secrets->GetSecretValue(GetAccessKey(), *config.MutableAccessKey())) {
ALS_ERROR(NKikimrServices::TX_TIERING) << "cannot read access key secret for " << GetAccessKey().DebugString();
{
auto value = secrets->GetSecretValue(GetAccessKey());
if (value.IsFail()) {
AFL_ERROR(NKikimrServices::TX_TIERING)("error", "invalid_secret")("object", "access_key")("reason", value.GetErrorMessage());
}
config.SetAccessKey(value.DetachResult());
}
if (!secrets->GetSecretValue(GetSecretKey(), *config.MutableSecretKey())) {
ALS_ERROR(NKikimrServices::TX_TIERING) << "cannot read secret key secret for " << GetSecretKey().DebugString();
{
auto value = secrets->GetSecretValue(GetSecretKey());
if (value.IsFail()) {
AFL_ERROR(NKikimrServices::TX_TIERING)("error", "invalid_secret")("object", "secret_key")("reason", value.GetErrorMessage());
}
config.SetSecretKey(value.DetachResult());
}
}
return config;
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/tx/tiering/tier/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
#include <ydb/services/metadata/manager/preparation_controller.h>
#include <ydb/services/metadata/manager/table_record.h>
#include <ydb/services/metadata/manager/object.h>
#include <ydb/services/metadata/secret/snapshot.h>
#include <ydb/services/metadata/service.h>
#include <ydb/services/metadata/secret/secret.h>

#include <library/cpp/json/writer/json_value.h>

Expand Down
3 changes: 0 additions & 3 deletions ydb/services/ext_index/ut/ut_ext_index.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include <ydb/core/cms/console/configs_dispatcher.h>
#include <ydb/core/testlib/cs_helper.h>
#include <ydb/core/tx/tiering/external_data.h>
#include <ydb/core/tx/schemeshard/schemeshard.h>
#include <ydb/core/tx/tx_proxy/proxy.h>
#include <ydb/core/formats/arrow/size_calcer.h>
Expand All @@ -25,8 +24,6 @@

namespace NKikimr {

using namespace NColumnShard;

class TLocalHelper: public Tests::NCS::THelper {
private:
using TBase = Tests::NCS::THelper;
Expand Down
3 changes: 0 additions & 3 deletions ydb/services/metadata/initializer/ut/ut_init.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include <ydb/core/cms/console/configs_dispatcher.h>
#include <ydb/core/testlib/cs_helper.h>
#include <ydb/core/tx/tiering/external_data.h>
#include <ydb/core/tx/schemeshard/schemeshard.h>
#include <ydb/core/tx/tx_proxy/proxy.h>
#include <ydb/core/wrappers/ut_helpers/s3_mock.h>
Expand Down Expand Up @@ -28,8 +27,6 @@

namespace NKikimr {

using namespace NColumnShard;

Y_UNIT_TEST_SUITE(Initializer) {

class TTestInitializer: public NMetadata::NInitializer::IInitializationBehaviour {
Expand Down
22 changes: 15 additions & 7 deletions ydb/services/metadata/secret/secret.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,22 @@ TString TSecretId::SerializeToString() const {
return sb;
}


TString TSecretIdOrValue::DebugString() const {
if (SecretId) {
return SecretId->SerializeToString();
} else if (Value) {
return MD5::Calc(*Value);
}
return "";
return std::visit(TOverloaded(
[](std::monostate) -> TString{
return "__NONE__";
},
[](const TSecretId& id) -> TString{
return id.SerializeToString();
},
[](const TSecretName& name) -> TString{
return name.SerializeToString();
},
[](const TString& value) -> TString{
return MD5::Calc(value);
}
),
State);
}

}
125 changes: 90 additions & 35 deletions ydb/services/metadata/secret/secret.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ class TSecretId {
private:
YDB_READONLY_PROTECT_DEF(TString, OwnerUserId);
YDB_READONLY_PROTECT_DEF(TString, SecretId);

public:
inline static const TString PrefixWithUser = "USId:";

TSecretId() = default;
TSecretId(const TString& ownerUserId, const TString& secretId)
: OwnerUserId(ownerUserId)
Expand All @@ -31,7 +34,7 @@ class TSecretId {
if (proto.HasValue()) {
return proto.GetValue();
} else {
return TStringBuilder() << "USId:" << (proto.GetSecretOwnerId() ? proto.GetSecretOwnerId() : defaultOwnerId) << ":" << SecretId;
return TStringBuilder() << PrefixWithUser << (proto.GetSecretOwnerId() ? proto.GetSecretOwnerId() : defaultOwnerId) << ":" << SecretId;
}
}

Expand All @@ -43,18 +46,41 @@ class TSecretId {
}
};

class TSecretName {
private:
YDB_READONLY_DEF(TString, SecretId);

public:
inline static const TString PrefixNoUser = "SId:";

TSecretName() = default;
TSecretName(const TString& secretId) : SecretId(secretId) {}

TString SerializeToString() const {
return TStringBuilder() << "SId:" << SecretId;
}

bool DeserializeFromString(const TString& secretString) {
if (secretString.StartsWith(PrefixNoUser)) {
SecretId = secretString.substr(PrefixNoUser.size());
return true;
}
return false;
}
};

class TSecretIdOrValue {
private:
YDB_READONLY_DEF(std::optional<TSecretId>, SecretId);
YDB_READONLY_DEF(std::optional<TString>, Value);
using TState = std::variant<std::monostate, TSecretId, TSecretName, TString>;
YDB_READONLY_DEF(TState, State);

private:
TSecretIdOrValue() = default;

bool DeserializeFromStringImpl(const TString& info, const TString& defaultUserId) {
static const TString prefixWithUser = "USId:";
static const TString prefixNoUser = "SId:";
if (info.StartsWith(prefixWithUser)) {
bool DeserializeFromStringImpl(const TString& info, const TString& defaultUserId = "") {
if (info.StartsWith(TSecretId::PrefixWithUser)) {
TStringBuf sb(info.data(), info.size());
sb.Skip(prefixWithUser.size());
sb.Skip(TSecretId::PrefixWithUser.size());
TStringBuf uId;
TStringBuf sId;
if (!sb.TrySplit(':', uId, sId)) {
Expand All @@ -63,32 +89,37 @@ class TSecretIdOrValue {
if (!uId || !sId) {
return false;
}
SecretId = TSecretId(uId, sId);
} else if (info.StartsWith(prefixNoUser)) {
State = TSecretId(uId, sId);
} else if (info.StartsWith(TSecretName::PrefixNoUser)) {
TStringBuf sb(info.data(), info.size());
sb.Skip(prefixNoUser.size());
SecretId = TSecretId(defaultUserId, TString(sb));
if (!sb || !defaultUserId) {
sb.Skip(TSecretName::PrefixNoUser.size());
if (!sb) {
return false;
}
if (defaultUserId) {
State = TSecretId(defaultUserId, TString(sb));
} else {
State = TSecretName(TString(sb));
}
} else {
Value = info;
State = info;
}
return true;
}
explicit TSecretIdOrValue(const TSecretId& id)
: SecretId(id) {

explicit TSecretIdOrValue(const TSecretId& id)
: State(id) {
}
explicit TSecretIdOrValue(const TSecretName& id)
: State(id) {
}

explicit TSecretIdOrValue(const TString& value)
: Value(value) {

: State(value) {
}

public:
bool operator!() const {
return !Value && !SecretId;
return std::holds_alternative<std::monostate>(State);
}

static TSecretIdOrValue BuildAsValue(const TString& value) {
Expand All @@ -103,12 +134,18 @@ class TSecretIdOrValue {
return TSecretIdOrValue(id);
}

static std::optional<TSecretIdOrValue> DeserializeFromOptional(const NKikimrSchemeOp::TSecretableVariable& proto, const TString& secretInfo, const TString& defaultOwnerId = Default<TString>()) {
static TSecretIdOrValue BuildAsId(const TSecretName& id) {
return TSecretIdOrValue(id);
}

static std::optional<TSecretIdOrValue> DeserializeFromOptional(
const NKikimrSchemeOp::TSecretableVariable& proto, const TString& secretInfo, const TString& defaultOwnerId = Default<TString>()) {
if (proto.HasSecretId()) {
return DeserializeFromProto(proto, defaultOwnerId);
} else if (proto.HasValue()) {
return DeserializeFromString(proto.GetValue().GetData());
} if (secretInfo) {
}
if (secretInfo) {
return DeserializeFromString(secretInfo, defaultOwnerId);
} else {
return {};
Expand All @@ -117,16 +154,25 @@ class TSecretIdOrValue {

NKikimrSchemeOp::TSecretableVariable SerializeToProto() const {
NKikimrSchemeOp::TSecretableVariable result;
if (SecretId) {
result.MutableSecretId()->SetId(SecretId->GetSecretId());
result.MutableSecretId()->SetOwnerId(SecretId->GetOwnerUserId());
} else if (Value) {
result.MutableValue()->SetData(*Value);
}
std::visit(TOverloaded(
[](std::monostate){ },
[&result](const TSecretId& id){
result.MutableSecretId()->SetId(id.GetSecretId());
result.MutableSecretId()->SetOwnerId(id.GetOwnerUserId());
},
[&result](const TSecretName& name){
result.MutableSecretId()->SetId(name.GetSecretId());
},
[&result](const TString& value){
result.MutableValue()->SetData(value);
}
),
State);
return result;
}

static std::optional<TSecretIdOrValue> DeserializeFromProto(const NKikimrSchemeOp::TSecretableVariable& proto, const TString& defaultOwnerId = Default<TString>()) {
static std::optional<TSecretIdOrValue> DeserializeFromProto(
const NKikimrSchemeOp::TSecretableVariable& proto, const TString& defaultOwnerId = Default<TString>()) {
if (proto.HasSecretId()) {
TString ownerId;
TString secretId;
Expand Down Expand Up @@ -157,12 +203,21 @@ class TSecretIdOrValue {
}

TString SerializeToString() const {
if (SecretId) {
return SecretId->SerializeToString();
} else if (Value) {
return *Value;
}
return "";
return std::visit(TOverloaded(
[](std::monostate) -> TString{
return "";
},
[](const TSecretId& id) -> TString{
return TStringBuilder() << TSecretId::PrefixWithUser << id.GetOwnerUserId() << ":" << id.GetSecretId();
},
[](const TSecretName& name) -> TString{
return TStringBuilder() << TSecretName::PrefixNoUser << name.GetSecretId();
},
[](const TString& value) -> TString{
return value;
}
),
State);
}

TString DebugString() const;
Expand Down
Loading

0 comments on commit 01126ac

Please sign in to comment.