implement secret identification by name

ydb/core/kqp/federated_query/kqp_federated_query_actors.cpp

@@ -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;
             }
 
@@ -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) {

ydb/core/tx/replication/controller/secret_resolver.cpp

@@ -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>

ydb/core/tx/tiering/tier/checker.cpp

@@ -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));

ydb/core/tx/tiering/tier/object.cpp

@@ -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;

ydb/core/tx/tiering/tier/object.h

@@ -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>
 

ydb/services/ext_index/ut/ut_ext_index.cpp

@@ -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>
@@ -25,8 +24,6 @@
 
 namespace NKikimr {
 
-using namespace NColumnShard;
-
 class TLocalHelper: public Tests::NCS::THelper {
 private:
     using TBase = Tests::NCS::THelper;

ydb/services/metadata/initializer/ut/ut_init.cpp

@@ -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>
@@ -28,8 +27,6 @@
 
 namespace NKikimr {
 
-using namespace NColumnShard;
-
 Y_UNIT_TEST_SUITE(Initializer) {
 
     class TTestInitializer: public NMetadata::NInitializer::IInitializationBehaviour {

ydb/services/metadata/secret/secret.cpp

@@ -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);
 }
 
 }

ydb/services/metadata/secret/secret.h

@@ -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)
@@ -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;
         }
     }
 
@@ -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)) {
@@ -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) {
@@ -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 {};
@@ -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;
@@ -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;