implement secret identification by name (ydb-platform#12641)

Conflicts: ydb/core/kqp/federated_query/kqp_federated_query_actors.cpp ydb/services/metadata/secret/snapshot.cpp ydb/services/metadata/secret/snapshot.h
zverevgeny · Jan 5, 2025 · 1fc93a5 · 1fc93a5
1 parent 4bef6ca
commit 1fc93a5
Show file tree

Hide file tree

Showing 16 changed files with 231 additions and 111 deletions.
diff --git a/ydb/core/kqp/federated_query/kqp_federated_query_actors.cpp b/ydb/core/kqp/federated_query/kqp_federated_query_actors.cpp
@@ -19,18 +19,19 @@ class TDescribeSecretsActor: public NActors::TActorBootstrapped<TDescribeSecrets
         std::vector<TString> secretValues;
         secretValues.reserve(SecretIds.size());
         for (const auto& secretId: SecretIds) {
-            TString secretValue;
-            const bool isFound = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretId), secretValue);
-            if (!isFound) {
-                if (!AskSent) {
-                    AskSent = true;
-                    Send(NMetadata::NProvider::MakeServiceId(SelfId().NodeId()), new NMetadata::NProvider::TEvAskSnapshot(GetSecretsSnapshotParser()));
-                } else {
-                    CompleteAndPassAway(TEvDescribeSecretsResponse::TDescription(Ydb::StatusIds::BAD_REQUEST, { NYql::TIssue("secret with name '" + secretId.GetSecretId() + "' not found") }));
-                }
-                return;
+            auto secretValue = snapshot->GetSecretValue(NMetadata::NSecret::TSecretIdOrValue::BuildAsId(secretId));
+            if (secretValue.IsSuccess()) {
+                secretValues.push_back(secretValue.DetachResult());
+                continue;
             }
-            secretValues.push_back(secretValue);
+
+            if (!AskSent) {
+                AskSent = true;
+                Send(NMetadata::NProvider::MakeServiceId(SelfId().NodeId()), new NMetadata::NProvider::TEvAskSnapshot(GetSecretsSnapshotParser()));
+            } else {
+                CompleteAndPassAway(TEvDescribeSecretsResponse::TDescription(Ydb::StatusIds::BAD_REQUEST, { NYql::TIssue("secret with name '" + secretId.GetSecretId() + "' not found") }));
+            }
+            return;
         }
 
         CompleteAndPassAway(TEvDescribeSecretsResponse::TDescription(secretValues));

diff --git a/ydb/core/tx/columnshard/engines/changes/general_compaction.cpp b/ydb/core/tx/columnshard/engines/changes/general_compaction.cpp
@@ -13,6 +13,7 @@ namespace NKikimr::NOlap::NCompaction {
 std::shared_ptr<NArrow::TColumnFilter> TGeneralCompactColumnEngineChanges::BuildPortionFilter(
     const std::optional<NKikimr::NOlap::TGranuleShardingInfo>& shardingActual, const std::shared_ptr<NArrow::TGeneralContainer>& batch,
     const TPortionInfo& pInfo, const THashSet<ui64>& portionsInUsage, const ISnapshotSchema::TPtr& resultSchema) const {
+    Y_UNUSED(resultSchema);
     std::shared_ptr<NArrow::TColumnFilter> filter;
     if (shardingActual && pInfo.NeedShardingFilter(*shardingActual)) {
         std::set<std::string> fieldNames;

diff --git a/ydb/core/tx/columnshard/engines/reader/simple_reader/constructor/read_metadata.cpp b/ydb/core/tx/columnshard/engines/reader/simple_reader/constructor/read_metadata.cpp
@@ -13,6 +13,9 @@ std::unique_ptr<TScanIteratorBase> TReadMetadata::StartScan(const std::shared_pt
 
 TConclusionStatus TReadMetadata::DoInitCustom(
     const NColumnShard::TColumnShard* owner, const TReadDescription& readDescription, const TDataStorageAccessor& dataAccessor) {
+    Y_UNUSED(owner);
+    Y_UNUSED(readDescription);
+    Y_UNUSED(dataAccessor);
     return TConclusionStatus::Success();
 }
 

diff --git a/ydb/core/tx/columnshard/engines/storage/granule/portions_index.h b/ydb/core/tx/columnshard/engines/storage/granule/portions_index.h
@@ -19,6 +19,7 @@ class TPortionsIndex {
         : Owner(owner)
     {
         Y_UNUSED(Owner);
+        Y_UNUSED(counters);
     }
 
     void AddPortion(const std::shared_ptr<TPortionInfo>& p) {

diff --git a/ydb/core/tx/columnshard/hooks/testing/controller.h b/ydb/core/tx/columnshard/hooks/testing/controller.h
@@ -193,6 +193,7 @@ class TController: public TReadOnlyController {
         return OverrideRejectMemoryIntervalLimit.value_or(def);
     }
     virtual ui64 DoGetMetadataRequestSoftMemoryLimit(const ui64 def) const override {
+        Y_UNUSED(def);
         return 0;
     }
     virtual EOptimizerCompactionWeightControl GetCompactionControl() const override {

diff --git a/ydb/core/tx/replication/controller/secret_resolver.cpp b/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>

diff --git a/ydb/core/tx/tiering/tier/checker.cpp b/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));

diff --git a/ydb/core/tx/tiering/tier/object.cpp b/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;

diff --git a/ydb/core/tx/tiering/tier/object.h b/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>
 

diff --git a/ydb/services/ext_index/ut/ut_ext_index.cpp b/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;

diff --git a/ydb/services/metadata/initializer/ut/ut_init.cpp b/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 {

diff --git a/ydb/services/metadata/secret/secret.cpp b/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);
 }
 
 }