prohibit creating secrets with duplicating names (ydb-platform#11680)

Conflicts: ydb/services/metadata/manager/abstract.h
zverevgeny · Jan 1, 2025 · f7434a7 · f7434a7
1 parent 91e3dd6
commit f7434a7
Show file tree

Hide file tree

Showing 18 changed files with 253 additions and 63 deletions.
diff --git a/ydb/core/testlib/common_helper.cpp b/ydb/core/testlib/common_helper.cpp
@@ -53,7 +53,7 @@ void THelper::WaitForSchemeOperation(TActorId sender, ui64 txId) {
 
 void THelper::StartScanRequest(const TString& request, const bool expectSuccess, TVector<THashMap<TString, NYdb::TValue>>* result) const {
     NYdb::NTable::TTableClient tClient(Server.GetDriver(),
-        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken("root@builtin"));
+        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken(AuthToken));
     auto expectation = expectSuccess;
     bool resultReady = false;
     TVector<THashMap<TString, NYdb::TValue>> rows;
@@ -109,7 +109,7 @@ void THelper::StartScanRequest(const TString& request, const bool expectSuccess,
 
 void THelper::StartDataRequest(const TString& request, const bool expectSuccess, TString* result) const {
     NYdb::NTable::TTableClient tClient(Server.GetDriver(),
-        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken("root@builtin"));
+        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken(AuthToken));
     auto expectation = expectSuccess;
     bool resultReady = false;
     bool* rrPtr = &resultReady;
@@ -144,7 +144,7 @@ void THelper::StartDataRequest(const TString& request, const bool expectSuccess,
 
 void THelper::StartSchemaRequestTableServiceImpl(const TString& request, const bool expectation, const bool waiting) const {
     NYdb::NTable::TTableClient tClient(Server.GetDriver(),
-        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken("root@builtin"));
+        NYdb::NTable::TClientSettings().UseQueryCache(false).AuthToken(AuthToken));
 
     std::shared_ptr<bool> rrPtr = std::make_shared<bool>(false);
     tClient.CreateSession().Subscribe([rrPtr, request, expectation](NThreading::TFuture<NYdb::NTable::TCreateSessionResult> f) {
@@ -171,7 +171,7 @@ void THelper::StartSchemaRequestTableServiceImpl(const TString& request, const b
 
 void THelper::StartSchemaRequestQueryServiceImpl(const TString& request, const bool expectation, const bool waiting) const {
     NYdb::NQuery::TQueryClient qClient(Server.GetDriver(),
-        NYdb::NQuery::TClientSettings().AuthToken("root@builtin"));
+        NYdb::NQuery::TClientSettings().AuthToken(AuthToken));
 
     std::shared_ptr<bool> rrPtr = std::make_shared<bool>(false);
     auto future = qClient.ExecuteQuery(request, NYdb::NQuery::TTxControl::NoTx());

diff --git a/ydb/core/testlib/common_helper.h b/ydb/core/testlib/common_helper.h
@@ -54,6 +54,10 @@ class TLoggerInit {
 };
 
 class THelper {
+private:
+    inline static const TString DefaultAuthToken = "root@builtin";
+    YDB_ACCESSOR(TString, AuthToken, DefaultAuthToken);
+
 protected:
     void WaitForSchemeOperation(TActorId sender, ui64 txId);
     void PrintResultSet(const NYdb::TResultSet& resultSet, NYson::TYsonWriter& writer) const;
@@ -73,6 +77,10 @@ class THelper {
         UseQueryService = use;
     }
 
+    void ResetAuthToken() {
+        AuthToken = DefaultAuthToken;
+    }
+
     void DropTable(const TString& tablePath);
 
     void StartScanRequest(const TString& request, const bool expectSuccess, TVector<THashMap<TString, NYdb::TValue>>* result) const;

diff --git a/ydb/mvp/meta/meta_cloud.h b/ydb/mvp/meta/meta_cloud.h
@@ -14,7 +14,6 @@
 #include <ydb/public/api/protos/ydb_discovery.pb.h>
 #include <ydb/public/api/protos/ydb_table.pb.h>
 #include <ydb/public/sdk/cpp/client/ydb_result/result.h>
-#include <ydb/core/kqp/provider/yql_kikimr_results.h>
 #include <ydb/core/ydb_convert/ydb_convert.h>
 #include <ydb/public/api/client/yc_private/resourcemanager/cloud_service.grpc.pb.h>
 #include <ydb/mvp/core/core_ydb.h>

diff --git a/ydb/mvp/meta/meta_cluster.h b/ydb/mvp/meta/meta_cluster.h
@@ -13,7 +13,6 @@
 #include <ydb/public/api/grpc/ydb_scripting_v1.grpc.pb.h>
 #include <ydb/public/api/protos/ydb_discovery.pb.h>
 #include <ydb/public/sdk/cpp/client/ydb_result/result.h>
-#include <ydb/core/kqp/provider/yql_kikimr_results.h>
 #include <ydb/core/ydb_convert/ydb_convert.h>
 #include <ydb/mvp/core/core_ydb.h>
 #include <ydb/mvp/core/core_ydb_impl.h>

diff --git a/ydb/mvp/meta/meta_clusters.h b/ydb/mvp/meta/meta_clusters.h
@@ -21,7 +21,6 @@
 #include <ydb/public/sdk/cpp/client/ydb_proto/accessor.h>
 #include <ydb/public/sdk/cpp/client/draft/ydb_scripting.h>
 #include <ydb/public/sdk/cpp/client/ydb_result/result.h>
-#include <ydb/core/kqp/provider/yql_kikimr_results.h>
 #include <ydb/core/ydb_convert/ydb_convert.h>
 #include <ydb/mvp/core/core_ydb.h>
 #include <ydb/mvp/core/core_ydb_impl.h>

diff --git a/ydb/mvp/meta/meta_cp_databases.h b/ydb/mvp/meta/meta_cp_databases.h
@@ -14,7 +14,6 @@
 #include <ydb/public/api/grpc/ydb_scripting_v1.grpc.pb.h>
 #include <ydb/public/api/protos/ydb_discovery.pb.h>
 #include <ydb/public/sdk/cpp/client/ydb_result/result.h>
-#include <ydb/core/kqp/provider/yql_kikimr_results.h>
 #include <ydb/core/ydb_convert/ydb_convert.h>
 #include <ydb/mvp/core/core_ydb.h>
 #include <ydb/mvp/core/core_ydb_impl.h>

diff --git a/ydb/mvp/meta/meta_db_clusters.h b/ydb/mvp/meta/meta_db_clusters.h
@@ -13,7 +13,6 @@
 #include <ydb/public/api/grpc/ydb_scripting_v1.grpc.pb.h>
 #include <ydb/public/api/protos/ydb_discovery.pb.h>
 #include <ydb/public/sdk/cpp/client/ydb_result/result.h>
-#include <ydb/core/kqp/provider/yql_kikimr_results.h>
 #include <ydb/core/ydb_convert/ydb_convert.h>
 #include <ydb/mvp/core/core_ydb.h>
 #include <ydb/mvp/core/core_ydb_impl.h>

diff --git a/ydb/services/metadata/manager/abstract.h b/ydb/services/metadata/manager/abstract.h
@@ -4,13 +4,15 @@
 
 #include <ydb/core/protos/kqp_physical.pb.h>
 #include <ydb/core/tx/locks/sys_tables.h>
+
 #include <ydb/library/accessor/accessor.h>
 #include <ydb/library/aclib/aclib.h>
-#include <ydb/library/conclusion/status.h>
+#include <ydb/library/actors/core/actorsystem.h>
 #include <ydb/library/conclusion/result.h>
-
+#include <ydb/library/conclusion/status.h>
 #include <ydb/services/metadata/abstract/kqp_common.h>
 #include <ydb/services/metadata/abstract/parsing.h>
+#include <ydb/services/metadata/manager/modification.h>
 
 #include <library/cpp/threading/future/core/future.h>
 #include <ydb/library/actors/core/actorsystem.h>
@@ -168,6 +170,11 @@ class IObjectOperationsManager: public IOperationsManager {
         const TInternalModificationContext& context, const TAlterOperationContext& alterContext) const {
         return DoPrepareObjectsBeforeModification(std::move(patchedObjects), controller, context, alterContext);
     }
+
+    virtual std::vector<TModificationStage::TPtr> GetPreconditions(
+        const std::vector<TObject>& /*objects*/, const IOperationsManager::TInternalModificationContext& /*context*/) const {
+        return {};
+    }
 };
 
 class IObjectModificationCommand {

diff --git a/ydb/services/metadata/manager/alter.h b/ydb/services/metadata/manager/alter.h
@@ -15,8 +15,8 @@ class TUpdateObjectActor: public TModificationActor<TObject> {
     using TBase = TModificationActor<TObject>;
 protected:
     virtual bool ProcessPreparedObjects(NInternal::TTableRecords&& records) const override {
-        TBase::Register(new TUpdateObjectsActor<TObject>(std::move(records), TBase::UserToken,
-            TBase::InternalController, TBase::SessionId, TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken()));
+        TBase::Register(new TUpdateObjectsActor<TObject>(std::move(records), TBase::UserToken, TBase::InternalController, TBase::SessionId,
+            TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken(), TBase::Preconditions));
         return true;
     }
 
@@ -33,9 +33,8 @@ class TUpsertObjectActor: public TModificationActor<TObject> {
     using TBase = TModificationActor<TObject>;
 protected:
     virtual bool ProcessPreparedObjects(NInternal::TTableRecords&& records) const override {
-        TBase::Register(new TUpsertObjectsActor<TObject>(std::move(records), TBase::UserToken,
-            TBase::InternalController, TBase::SessionId, TBase::TransactionId,
-            TBase::Context.GetExternalData().GetUserToken()));
+        TBase::Register(new TUpsertObjectsActor<TObject>(std::move(records), TBase::UserToken, TBase::InternalController, TBase::SessionId,
+            TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken(), TBase::Preconditions));
         return true;
     }
 
@@ -53,9 +52,8 @@ class TCreateObjectActor: public TModificationActor<TObject> {
     bool ExistingOk = false;
 protected:
     virtual bool ProcessPreparedObjects(NInternal::TTableRecords&& records) const override {
-        TBase::Register(new TInsertObjectsActor<TObject>(std::move(records), TBase::UserToken,
-            TBase::InternalController, TBase::SessionId, TBase::TransactionId,
-            TBase::Context.GetExternalData().GetUserToken(), ExistingOk));
+        TBase::Register(new TInsertObjectsActor<TObject>(std::move(records), TBase::UserToken, TBase::InternalController, TBase::SessionId,
+            TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken(), TBase::Preconditions, ExistingOk));
         return true;
     }
 
@@ -103,8 +101,8 @@ class TDeleteObjectActor: public TModificationActor<TObject> {
     using TBase::TBase;
 
     virtual bool ProcessPreparedObjects(NInternal::TTableRecords&& records) const override {
-        TBase::Register(new TDeleteObjectsActor<TObject>(std::move(records), TBase::UserToken,
-            TBase::InternalController, TBase::SessionId, TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken()));
+        TBase::Register(new TDeleteObjectsActor<TObject>(std::move(records), TBase::UserToken, TBase::InternalController, TBase::SessionId,
+            TBase::TransactionId, TBase::Context.GetExternalData().GetUserToken(), TBase::Preconditions));
         return true;
     }
 

diff --git a/ydb/services/metadata/manager/alter_impl.h b/ydb/services/metadata/manager/alter_impl.h
@@ -58,6 +58,7 @@ class TModificationActorImpl: public NActors::TActorBootstrapped<TModificationAc
     typename IObjectOperationsManager<TObject>::TPtr Manager;
     const IOperationsManager::TInternalModificationContext Context;
     std::vector<NInternal::TTableRecord> Patches;
+    std::vector<TModificationStage::TPtr> Preconditions;
     NInternal::TTableRecords RestoreObjectIds;
     const NACLib::TUserToken UserToken = NACLib::TSystemUsers::Metadata();
     virtual bool PrepareRestoredObjects(std::vector<TObject>& objects) const = 0;
@@ -179,6 +180,7 @@ class TModificationActorImpl: public NActors::TActorBootstrapped<TModificationAc
     }
 
     void Handle(typename TEvAlterPreparationFinished<TObject>::TPtr& ev) {
+        Preconditions = Manager->GetPreconditions(ev->Get()->GetObjects(), Context);
         NInternal::TTableRecords records;
         records.InitColumns(Manager->GetSchema().GetYDBColumns());
         records.ReserveRows(ev->Get()->GetObjects().size());

diff --git a/ydb/services/metadata/manager/modification.h b/ydb/services/metadata/manager/modification.h
@@ -10,6 +10,32 @@
 
 namespace NKikimr::NMetadata::NModifications {
 
+class TModificationStage {
+private:
+    YDB_ACCESSOR_DEF(Ydb::Table::ExecuteDataQueryRequest, Request);
+
+public:
+    using TPtr = std::shared_ptr<TModificationStage>;
+
+    virtual TConclusionStatus HandleResult(const Ydb::Table::ExecuteQueryResult& /*result*/) const {
+        return TConclusionStatus::Success();
+    }
+
+    virtual TConclusionStatus HandleError(const NRequest::TEvRequestFailed& ev) const {
+        return TConclusionStatus::Fail(ev.GetErrorMessage());
+    }
+
+    void SetCommit() {
+        Request.mutable_tx_control()->set_commit_tx(true);
+    }
+
+    TModificationStage(Ydb::Table::ExecuteDataQueryRequest request)
+        : Request(std::move(request)) {
+    }
+
+    virtual ~TModificationStage() = default;
+};
+
 template <class TObject>
 class TModifyObjectsActor: public NActors::TActorBootstrapped<TModifyObjectsActor<TObject>> {
 private:
@@ -19,17 +45,44 @@ class TModifyObjectsActor: public NActors::TActorBootstrapped<TModifyObjectsActo
     const TString TransactionId;
     const NACLib::TUserToken SystemUserToken;
     const std::optional<NACLib::TUserToken> UserToken;
+
+    void FillRequestSettings(Ydb::Table::ExecuteDataQueryRequest& request) {
+        request.set_session_id(SessionId);
+        request.mutable_tx_control()->set_tx_id(TransactionId);
+    }
+
+    void AdvanceStage() {
+        AFL_VERIFY(!Stages.empty());
+        Stages.pop_front();
+        if (Stages.size()) {
+            TBase::Register(
+                new NRequest::TYDBCallbackRequest<NRequest::TDialogYQLRequest>(Stages.front()->GetRequest(), SystemUserToken, TBase::SelfId()));
+        } else {
+            Controller->OnModificationFinished();
+            TBase::PassAway();
+        }
+    }
+
 protected:
-    std::deque<NRequest::TDialogYQLRequest::TRequest> Requests;
+    std::deque<TModificationStage::TPtr> Stages;
     NInternal::TTableRecords Objects;
     virtual Ydb::Table::ExecuteDataQueryRequest BuildModifyQuery() const = 0;
     virtual TString GetModifyType() const = 0;
+    virtual TModificationStage::TPtr DoBuildRequestDirect(Ydb::Table::ExecuteDataQueryRequest query) const {
+        return std::make_shared<TModificationStage>(std::move(query));
+    }
+
+    void BuildPreconditionStages(const std::vector<TModificationStage::TPtr>& stages) {
+        for (auto&& stage : stages) {
+            FillRequestSettings(stage->MutableRequest());
+            Stages.emplace_back(std::move(stage));
+        }
+    }
 
     void BuildRequestDirect() {
         Ydb::Table::ExecuteDataQueryRequest request = BuildModifyQuery();
-        request.set_session_id(SessionId);
-        request.mutable_tx_control()->set_tx_id(TransactionId);
-        Requests.emplace_back(std::move(request));
+        FillRequestSettings(request);
+        Stages.emplace_back(DoBuildRequestDirect(request));
     }
 
     void BuildRequestHistory() {
@@ -42,31 +95,39 @@ class TModifyObjectsActor: public NActors::TActorBootstrapped<TModifyObjectsActo
         Objects.AddColumn(NInternal::TYDBColumn::UInt64("historyInstant"), NInternal::TYDBValue::UInt64(TActivationContext::Now().MicroSeconds()));
         Objects.AddColumn(NInternal::TYDBColumn::Utf8("historyAction"), NInternal::TYDBValue::Utf8(GetModifyType()));
         Ydb::Table::ExecuteDataQueryRequest request = Objects.BuildInsertQuery(TObject::GetBehaviour()->GetStorageHistoryTablePath());
-        request.set_session_id(SessionId);
-        request.mutable_tx_control()->set_tx_id(TransactionId);
-        Requests.emplace_back(std::move(request));
+        FillRequestSettings(request);
+        Stages.emplace_back(std::make_shared<TModificationStage>(std::move(request)));
     }
 
-    void Handle(NRequest::TEvRequestResult<NRequest::TDialogYQLRequest>::TPtr& /*ev*/) {
-        if (Requests.size()) {
-            TBase::Register(new NRequest::TYDBCallbackRequest<NRequest::TDialogYQLRequest>(
-                Requests.front(), SystemUserToken, TBase::SelfId()));
-            Requests.pop_front();
-        } else {
-            Controller->OnModificationFinished();
+    void Handle(NRequest::TEvRequestResult<NRequest::TDialogYQLRequest>::TPtr& ev) {
+        const auto& operation = ev->Get()->GetResult().operation();
+        AFL_VERIFY(operation.ready());
+
+        Ydb::Table::ExecuteQueryResult result;
+        operation.result().UnpackTo(&result);
+
+        if (auto status = Stages.front()->HandleResult(result); status.IsFail()) {
+            Controller->OnModificationProblem(status.GetErrorMessage());
             TBase::PassAway();
+            return;
         }
+
+        AdvanceStage();
     }
 
-    virtual void Handle(NRequest::TEvRequestFailed::TPtr& ev) {
+    void Handle(NRequest::TEvRequestFailed::TPtr& ev) {
         auto g = TBase::PassAwayGuard();
-        Controller->OnModificationProblem("cannot execute yql request for " + GetModifyType() +
-            " objects: " + ev->Get()->GetErrorMessage());
+        if (auto status = Stages.front()->HandleError(*ev->Get()); status.IsFail()) {
+            Controller->OnModificationProblem(status.GetErrorMessage());
+        } else {
+            Controller->OnModificationFinished();
+        }
     }
 
 public:
-    TModifyObjectsActor(NInternal::TTableRecords&& objects, const NACLib::TUserToken& systemUserToken, IModificationObjectsController::TPtr controller, const TString& sessionId,
-        const TString& transactionId, const std::optional<NACLib::TUserToken>& userToken)
+    TModifyObjectsActor(NInternal::TTableRecords&& objects, const NACLib::TUserToken& systemUserToken,
+        IModificationObjectsController::TPtr controller, const TString& sessionId, const TString& transactionId,
+        const std::optional<NACLib::TUserToken>& userToken, const std::vector<TModificationStage::TPtr>& preconditions)
         : Controller(controller)
         , SessionId(sessionId)
         , TransactionId(transactionId)
@@ -75,6 +136,7 @@ class TModifyObjectsActor: public NActors::TActorBootstrapped<TModifyObjectsActo
         , Objects(std::move(objects))
 
     {
+        BuildPreconditionStages(preconditions);
         Y_ABORT_UNLESS(SessionId);
     }
 
@@ -91,12 +153,10 @@ class TModifyObjectsActor: public NActors::TActorBootstrapped<TModifyObjectsActo
         TBase::Become(&TModifyObjectsActor::StateMain);
         BuildRequestDirect();
         BuildRequestHistory();
-        Y_ABORT_UNLESS(Requests.size());
-        Requests.back().mutable_tx_control()->set_commit_tx(true);
+        Y_ABORT_UNLESS(Stages.size());
+        Stages.back()->SetCommit();
 
-        TBase::Register(new NRequest::TYDBCallbackRequest<NRequest::TDialogYQLRequest>(
-            Requests.front(), SystemUserToken, TBase::SelfId()));
-        Requests.pop_front();
+        TBase::Register(new NRequest::TYDBCallbackRequest<NRequest::TDialogYQLRequest>(Stages.front()->GetRequest(), SystemUserToken, TBase::SelfId()));
     }
 };
 
@@ -148,6 +208,23 @@ class TDeleteObjectsActor: public TModifyObjectsActor<TObject> {
     using TBase::TBase;
 };
 
+class TStageInsertObjects: public NModifications::TModificationStage {
+private:
+    const bool ExistingOk;
+
+public:
+    TConclusionStatus HandleError(const NRequest::TEvRequestFailed& ev) const override {
+        if (ExistingOk && ev.GetStatus() == Ydb::StatusIds::PRECONDITION_FAILED) {
+            return TConclusionStatus::Success();
+        }
+        return TConclusionStatus::Fail(ev.GetErrorMessage());
+    }
+
+    TStageInsertObjects(Ydb::Table::ExecuteDataQueryRequest request, const bool existingOk)
+        : TModificationStage(std::move(request)), ExistingOk(existingOk) {
+    }
+};
+
 template <class TObject>
 class TInsertObjectsActor: public TModifyObjectsActor<TObject> {
 private:
@@ -161,19 +238,13 @@ class TInsertObjectsActor: public TModifyObjectsActor<TObject> {
         return "insert";
     }
 
-    void Handle(NRequest::TEvRequestFailed::TPtr& ev) override {
-        if (ev->Get()->GetStatus() == Ydb::StatusIds::PRECONDITION_FAILED && ExistingOk) {
-            NRequest::TDialogYQLRequest::TResponse resp;
-            this->Send(this->SelfId(), new NRequest::TEvRequestResult<NRequest::TDialogYQLRequest>(std::move(resp)));
-            this->Requests.clear(); // Remove history request
-            return;
-        }
-        TBase::Handle(ev);
+    TModificationStage::TPtr DoBuildRequestDirect(Ydb::Table::ExecuteDataQueryRequest query) const override {
+        return std::make_shared<TStageInsertObjects>(std::move(query), ExistingOk);
     }
 public:
     TInsertObjectsActor(NInternal::TTableRecords&& objects, const NACLib::TUserToken& systemUserToken, IModificationObjectsController::TPtr controller, const TString& sessionId,
-        const TString& transactionId, const std::optional<NACLib::TUserToken>& userToken, bool existingOk)
-        : TBase(std::move(objects), systemUserToken, std::move(controller), sessionId, transactionId, userToken)
+        const TString& transactionId, const std::optional<NACLib::TUserToken>& userToken, const std::vector<TModificationStage::TPtr>& preconditions, bool existingOk)
+        : TBase(std::move(objects), systemUserToken, std::move(controller), sessionId, transactionId, userToken, preconditions)
         , ExistingOk(existingOk)
     {
     }