From 844c854572c1245eb674d155a474d61e6888f24e Mon Sep 17 00:00:00 2001 From: Vitaly Isaev Date: Fri, 1 Mar 2024 12:45:21 +0000 Subject: [PATCH 1/5] Add yql_generic_token_provider.cpp --- .../common/proto/gateways_config.proto | 11 ++-- .../yql/providers/generic/actors/ya.make | 1 + .../generic/actors/yql_generic_read_actor.cpp | 53 +++------------ .../generic/actors/yql_generic_read_actor.h | 2 +- .../actors/yql_generic_source_factory.cpp | 4 +- .../actors/yql_generic_token_provider.cpp | 66 +++++++++++++++++++ .../actors/yql_generic_token_provider.h | 32 +++++++++ .../yql/providers/generic/proto/source.proto | 11 ++-- .../provider/ut/pushdown/pushdown_ut.cpp | 8 +-- .../provider/yql_generic_dq_integration.cpp | 10 ++- .../provider/yql_generic_load_meta.cpp | 27 ++++++-- .../generic/provider/yql_generic_settings.cpp | 1 + 12 files changed, 157 insertions(+), 69 deletions(-) create mode 100644 ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp create mode 100644 ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h diff --git a/ydb/library/yql/providers/common/proto/gateways_config.proto b/ydb/library/yql/providers/common/proto/gateways_config.proto index 6659e421284b..865d42bc7bf8 100644 --- a/ydb/library/yql/providers/common/proto/gateways_config.proto +++ b/ydb/library/yql/providers/common/proto/gateways_config.proto @@ -563,12 +563,11 @@ message TGenericClusterConfig { // Credentials used to access data source instance optional NYql.NConnector.NApi.TCredentials Credentials = 10; - // Credentials used to access MDB API. - // When working with data source instances deployed in a cloud, - // you should either set (ServiceAccountId, ServiceAccountIdSignature) pair, - // or set IAM Token. - // The names of these fields must satisfy this template function: - // https://github.com/ydb-platform/ydb/arcadia/contrib/ydb/core/fq/libs/actors/clusters_from_connections.cpp?rev=r11823087#L19 + // Credentials used to access managed databases APIs. + // When working with external data source instances deployed in clouds, + // one should either set (ServiceAccountId, ServiceAccountIdSignature) pair + // that will be resolved into IAM Token via Token Accessor, + // or provide IAM Token directly. optional string ServiceAccountId = 6; optional string ServiceAccountIdSignature = 7; optional string Token = 11; diff --git a/ydb/library/yql/providers/generic/actors/ya.make b/ydb/library/yql/providers/generic/actors/ya.make index 40471d122e07..53f40afdca7c 100644 --- a/ydb/library/yql/providers/generic/actors/ya.make +++ b/ydb/library/yql/providers/generic/actors/ya.make @@ -3,6 +3,7 @@ LIBRARY() SRCS( yql_generic_read_actor.cpp yql_generic_source_factory.cpp + yql_generic_token_provider.cpp ) PEERDIR( diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp index 2efba5888531..8bcf2896b843 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp @@ -1,4 +1,5 @@ #include "yql_generic_read_actor.h" +#include "yql_generic_token_provider.h" #include #include @@ -9,11 +10,9 @@ #include #include #include -#include #include #include #include -#include #include #include #include @@ -104,14 +103,14 @@ namespace NYql::NDq { ui64 inputIndex, TCollectStatsLevel statsLevel, NConnector::IClient::TPtr client, - NYdb::TCredentialsProviderPtr credentialsProvider, - NConnector::TSource&& source, + TGenericTokenProvider::TPtr tokenProvider, + NGeneric::TSource&& source, const NActors::TActorId& computeActorId, const NKikimr::NMiniKQL::THolderFactory& holderFactory) : InputIndex_(inputIndex) , ComputeActorId_(computeActorId) , Client_(std::move(client)) - , CredentialsProvider_(std::move(credentialsProvider)) + , TokenProvider_(std::move(tokenProvider)) , HolderFactory_(holderFactory) , Source_(source) { @@ -146,7 +145,7 @@ namespace NYql::NDq { // Prepare request NConnector::NApi::TListSplitsRequest request; NConnector::NApi::TSelect select = Source_.select(); // copy TSelect from source - MaybeRefreshToken(select.mutable_data_source_instance()); + TokenProvider_->MaybeFillToken(select.mutable_data_source_instance()); *request.mutable_selects()->Add() = std::move(select); // Initialize stream @@ -242,7 +241,7 @@ namespace NYql::NDq { Splits_.cbegin(), Splits_.cend(), [&](const NConnector::NApi::TSplit& split) { NConnector::NApi::TSplit splitCopy = split; - MaybeRefreshToken(splitCopy.mutable_select()->mutable_data_source_instance()); + TokenProvider_->MaybeFillToken(splitCopy.mutable_select()->mutable_data_source_instance()); *request.mutable_splits()->Add() = std::move(split); }); @@ -459,20 +458,6 @@ namespace NYql::NDq { return total; } - void MaybeRefreshToken(NConnector::NApi::TDataSourceInstance* dsi) const { - if (!dsi->credentials().has_token()) { - return; - } - - // Token may have expired. Refresh it. - Y_ENSURE(CredentialsProvider_, "CredentialsProvider is not initialized"); - auto iamToken = CredentialsProvider_->GetAuthInfo(); - Y_ENSURE(iamToken, "empty IAM token"); - - *dsi->mutable_credentials()->mutable_token()->mutable_value() = iamToken; - *dsi->mutable_credentials()->mutable_token()->mutable_type() = "IAM"; - } - // IActor & IDqComputeActorAsyncInput void PassAway() override { // Is called from Compute Actor YQL_CLOG(INFO, ProviderGeneric) << "PassAway :: final ingress stats" @@ -505,7 +490,7 @@ namespace NYql::NDq { const NActors::TActorId ComputeActorId_; NConnector::IClient::TPtr Client_; - NYdb::TCredentialsProviderPtr CredentialsProvider_; + TGenericTokenProvider::TPtr TokenProvider_; NConnector::IListSplitsStreamIterator::TPtr ListSplitsIterator_; TVector Splits_; // accumulated list of table splits NConnector::IReadSplitsStreamIterator::TPtr ReadSplitsIterator_; @@ -514,12 +499,12 @@ namespace NYql::NDq { NKikimr::NMiniKQL::TPlainContainerCache ArrowRowContainerCache_; const NKikimr::NMiniKQL::THolderFactory& HolderFactory_; - NConnector::TSource Source_; + NGeneric::TSource Source_; }; std::pair CreateGenericReadActor(NConnector::IClient::TPtr genericClient, - NConnector::TSource&& source, + NGeneric::TSource&& source, ui64 inputIndex, TCollectStatsLevel statsLevel, const THashMap& /*secureParams*/, @@ -548,24 +533,6 @@ namespace NYql::NDq { */ // Obtain token to access remote data source if necessary - NYdb::TCredentialsProviderPtr credentialProvider; - if (source.GetServiceAccountId() && source.GetServiceAccountIdSignature()) { - Y_ENSURE(credentialsFactory, "CredentialsFactory is not initialized"); - - auto structuredTokenJSON = TStructuredTokenBuilder().SetServiceAccountIdAuth( - source.GetServiceAccountId(), source.GetServiceAccountIdSignature()) - .ToJson(); - - // If service account is provided, obtain IAM-token - Y_ENSURE(structuredTokenJSON, "empty structured token"); - - auto credentialsProviderFactory = CreateCredentialsProviderFactoryForStructuredToken( - credentialsFactory, - structuredTokenJSON, - false); - credentialProvider = credentialsProviderFactory->CreateProvider(); - } - // TODO: partitioning is not implemented now, but this code will be useful for the further research: /* TStringBuilder part; @@ -583,7 +550,7 @@ namespace NYql::NDq { inputIndex, statsLevel, genericClient, - std::move(credentialProvider), + CreateGenericTokenProvider(source, credentialsFactory), std::move(source), computeActorId, holderFactory); diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h index 6f8b81bb3063..3ea18d5e0b3f 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h @@ -9,7 +9,7 @@ namespace NYql::NDq { std::pair - CreateGenericReadActor(NConnector::IClient::TPtr genericClient, NConnector::TSource&& params, ui64 inputIndex, + CreateGenericReadActor(NConnector::IClient::TPtr genericClient, NGeneric::TSource&& params, ui64 inputIndex, TCollectStatsLevel statsLevel, const THashMap& secureParams, const THashMap& taskParams, const NActors::TActorId& computeActorId, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp index 3b035bcb5378..cecbbe34e7ac 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp @@ -10,14 +10,14 @@ namespace NYql::NDq { ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, NYql::NConnector::IClient::TPtr genericClient) { auto genericFactory = [credentialsFactory, genericClient]( - NConnector::TSource&& settings, + NGeneric::TSource&& settings, IDqAsyncIoFactory::TSourceArguments&& args) { return CreateGenericReadActor(genericClient, std::move(settings), args.InputIndex, args.StatsLevel, args.SecureParams, args.TaskParams, args.ComputeActorId, credentialsFactory, args.HolderFactory); }; for (auto& sourceName : {"ClickHouseGeneric", "PostgreSqlGeneric", "YdbGeneric"}) { - factory.RegisterSource(sourceName, genericFactory); + factory.RegisterSource(sourceName, genericFactory); } } diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp new file mode 100644 index 000000000000..ad290984b2ac --- /dev/null +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp @@ -0,0 +1,66 @@ +#include "yql_generic_token_provider.h" + +#include + +namespace NYql::NDq { + TGenericTokenProvider::TGenericTokenProvider( + const NYql::NGeneric::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) + : Source_(source) + , StaticIAMToken_(source.GetToken()) + , CredentialsProvider_(nullptr) { + // 1. User has provided IAM-token itself. + // This token will be used during the whole lifetime of a read actor. + if (!StaticIAMToken_.empty()) { + return; + } + + // 2. User has provided service account creds. + // We create token accessor client that will renew token accessor by demand. + if (source.GetServiceAccountId() && source.GetServiceAccountIdSignature()) { + Y_ENSURE(credentialsFactory, "CredentialsFactory is not initialized"); + + auto structuredTokenJSON = + TStructuredTokenBuilder() + .SetServiceAccountIdAuth(source.GetServiceAccountId(), source.GetServiceAccountIdSignature()) + .ToJson(); + + // If service account is provided, obtain IAM-token + Y_ENSURE(structuredTokenJSON, "empty structured token"); + + auto credentialsProviderFactory = + CreateCredentialsProviderFactoryForStructuredToken(credentialsFactory, structuredTokenJSON, false); + CredentialsProvider_ = credentialsProviderFactory->CreateProvider(); + } + + // 3. If we reached this point, it means that user doesn't need token auth. + } + + void TGenericTokenProvider::MaybeFillToken(NConnector::NApi::TDataSourceInstance* dsi) const { + // 1. Don't need tokens if basic auth is set + if (dsi->credentials().has_basic()) { + return; + } + + *dsi->mutable_credentials()->mutable_token()->mutable_type() = "IAM"; + + // 2. If static IAM-token has been provided, use it + if (!StaticIAMToken_.empty()) { + *dsi->mutable_credentials()->mutable_token()->mutable_value() = StaticIAMToken_; + return; + } + + // 3. Otherwise use credentials provider to get token + Y_ENSURE(CredentialsProvider_, "CredentialsProvider is not initialized"); + + auto iamToken = CredentialsProvider_->GetAuthInfo(); + Y_ENSURE(iamToken, "CredentialsProvider returned empty IAM token"); + + *dsi->mutable_credentials()->mutable_token()->mutable_value() = std::move(iamToken); + } + + TGenericTokenProvider::TPtr + CreateGenericTokenProvider(const NYql::NGeneric::TSource& source, + const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) { + return std::make_unique(source, credentialsFactory); + } +} //namespace NYql::NDq diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h new file mode 100644 index 000000000000..cbd5d7909c1b --- /dev/null +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h @@ -0,0 +1,32 @@ +#pragma once + +#include +#include +#include + +namespace NYql::NDq { + // When accessing external data sources using authentication via tokens, + // there are two options: + // 1. Use static IAM-token provided by user (especially usefull during debugging); + // 2. Use service account credentials in order to get (and renew) token by demand. + class TGenericTokenProvider { + public: + using TPtr = std::unique_ptr; + + TGenericTokenProvider() = delete; + + TGenericTokenProvider(const NYql::NGeneric::TSource& source, + const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory); + + void MaybeFillToken(NConnector::NApi::TDataSourceInstance* dsi) const; + + private: + NYql::NGeneric::TSource Source_; + std::string StaticIAMToken_; + NYdb::TCredentialsProviderPtr CredentialsProvider_; + }; + + TGenericTokenProvider::TPtr + CreateGenericTokenProvider(const NYql::NGeneric::TSource& source, + const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory); +} //namespace NYql::NDq diff --git a/ydb/library/yql/providers/generic/proto/source.proto b/ydb/library/yql/providers/generic/proto/source.proto index 2ea009080872..d271731c42fe 100644 --- a/ydb/library/yql/providers/generic/proto/source.proto +++ b/ydb/library/yql/providers/generic/proto/source.proto @@ -2,7 +2,7 @@ syntax = "proto3"; option cc_enable_arenas = true; -package NYql.NConnector; +package NYql.NGeneric; import "ydb/library/yql/providers/generic/connector/api/service/protos/connector.proto"; import "ydb/library/yql/providers/generic/connector/api/common/data_source.proto"; @@ -11,11 +11,14 @@ message TSource { // Prepared Select expression NYql.NConnector.NApi.TSelect select = 2; - // ServiceAccountId and ServiceAccountIdSignature are used to obtain tokens - // to access external data source supporting this kind of authentication - // during the runtime phase. + // Credentials used to access managed databases APIs. + // When working with external data source instances deployed in clouds, + // one should either set (ServiceAccountId, ServiceAccountIdSignature) pair + // that will be resolved into IAM Token via Token Accessor, + // or provide IAM Token directly. string ServiceAccountId = 4; string ServiceAccountIdSignature = 5; + string Token = 6; reserved 1, 3; } diff --git a/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp b/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp index f632e1f2627c..ac398cd30212 100644 --- a/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp +++ b/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp @@ -144,7 +144,7 @@ struct TFakeGenericClient: public NConnector::IClient { class TBuildDqSourceSettingsTransformer: public TOptimizeTransformerBase { public: - explicit TBuildDqSourceSettingsTransformer(TTypeAnnotationContext* types, NConnector::TSource* dqSourceSettings, bool* dqSourceSettingsWereBuilt) + explicit TBuildDqSourceSettingsTransformer(TTypeAnnotationContext* types, NGeneric::TSource* dqSourceSettings, bool* dqSourceSettingsWereBuilt) : TOptimizeTransformerBase(types, NLog::EComponent::ProviderGeneric, {}) , DqSourceSettings_(dqSourceSettings) , DqSourceSettingsWereBuilt_(dqSourceSettingsWereBuilt) @@ -182,13 +182,13 @@ class TBuildDqSourceSettingsTransformer: public TOptimizeTransformerBase { TString sourceType; dqIntegration->FillSourceSettings(*dqSourceNode, settings, sourceType, 1); UNIT_ASSERT_STRINGS_EQUAL(sourceType, "PostgreSqlGeneric"); - UNIT_ASSERT(settings.Is()); + UNIT_ASSERT(settings.Is()); settings.UnpackTo(DqSourceSettings_); *DqSourceSettingsWereBuilt_ = true; } private: - NConnector::TSource* DqSourceSettings_; + NGeneric::TSource* DqSourceSettings_; bool* DqSourceSettingsWereBuilt_; }; @@ -207,7 +207,7 @@ struct TPushdownFixture: public NUnitTest::TBaseFixture { TAutoPtr Transformer; TAutoPtr BuildDqSourceSettingsTransformer; - NConnector::TSource DqSourceSettings; + NGeneric::TSource DqSourceSettings; bool DqSourceSettingsWereBuilt = false; TExprNode::TPtr InitialExprRoot; diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp index 8bee8f7eb75f..228b52541672 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp @@ -102,7 +102,7 @@ namespace NYql { const auto& clusterConfig = State_->Configuration->ClusterNamesToClusterConfigs[clusterName]; const auto& endpoint = clusterConfig.endpoint(); - NConnector::TSource source; + NGeneric::TSource source; // for backward compability full path can be used (cluster_name.`db_name.table`) // TODO: simplify during https://st.yandex-team.ru/YQ-2494 @@ -149,10 +149,16 @@ namespace NYql { } // Managed YDB supports access via IAM token. - // Copy service account ids to obtain tokens during request execution phase. + // If exist, copy service account creds to obtain tokens during request execution phase. + // If exists, copy previously created token. if (clusterConfig.kind() == NConnector::NApi::EDataSourceKind::YDB) { source.SetServiceAccountId(clusterConfig.GetServiceAccountId()); source.SetServiceAccountIdSignature(clusterConfig.GetServiceAccountIdSignature()); + source.SetToken(State_->Types->Credentials->FindCredentialContent( + "default_" + clusterConfig.name(), + "default_generic", + clusterConfig.GetToken()) + ); } // preserve source description for read actor diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index 329bd634de3b..2c11ebd69f95 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -262,23 +262,36 @@ namespace NYql { void FillCredentials(NConnector::NApi::TDescribeTableRequest& request, const TGenericClusterConfig& clusterConfig) { auto dsi = request.mutable_data_source_instance(); - // If login/password is provided, just copy them into request + // If login/password is provided, just copy them into request: + // connector will use Basic Auth to access external data sources. if (clusterConfig.GetCredentials().Hasbasic()) { *dsi->mutable_credentials() = clusterConfig.GetCredentials(); return; } - Y_ENSURE(State_->CredentialsFactory, "CredentialsFactory is not initialized"); + // If there are no Basic Auth, two options can be considered: - // If service account is provided, prepare to obtain IAM-token + // 1. Client provided own IAM-token to access external data source + auto iamToken = State_->Types->Credentials->FindCredentialContent( + "default_" + clusterConfig.name(), + "default_generic", + clusterConfig.GetToken()); + if (iamToken) { + *dsi->mutable_credentials()->mutable_token()->mutable_value() = iamToken; + *dsi->mutable_credentials()->mutable_token()->mutable_type() = "IAM"; + } + + // 2. Client provided service account creds that must be converted into IAM-token + Y_ENSURE(State_->CredentialsFactory, "CredentialsFactory is not initialized"); auto structuredTokenJSON = TStructuredTokenBuilder().SetServiceAccountIdAuth( clusterConfig.GetServiceAccountId(), clusterConfig.GetServiceAccountIdSignature()) .ToJson(); + Y_ENSURE(structuredTokenJSON, "empty structured token"); - // Create provider or get existing one. + // Create provider (== Token Accessor client) or get existing one. // It's crucial to reuse providers because their construction implies synchronous IO. auto providersIt = State_->CredentialProviders.find(clusterConfig.name()); if (providersIt == State_->CredentialProviders.end()) { @@ -288,11 +301,11 @@ namespace NYql { false); providersIt = State_->CredentialProviders.emplace( - std::make_pair(clusterConfig.name(), credentialsProviderFactory->CreateProvider())) - .first; + std::make_pair(clusterConfig.name(), credentialsProviderFactory->CreateProvider())) + .first; } - auto iamToken = providersIt->second->GetAuthInfo(); + iamToken = providersIt->second->GetAuthInfo(); Y_ENSURE(iamToken, "empty IAM token"); *dsi->mutable_credentials()->mutable_token()->mutable_value() = iamToken; diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp index 1c2521573ddb..14a5f839486b 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp @@ -84,6 +84,7 @@ namespace NYql { "default_" + cluster.name(), "default_generic", cluster.GetToken()); + Y_ENSURE(iamToken); if (iamToken) { return b.SetIAMToken(iamToken).ToJson(); } From 51ca01457e23db81290192fb2b11bc1dca7c6dc3 Mon Sep 17 00:00:00 2001 From: Vitaly Isaev Date: Fri, 1 Mar 2024 12:56:06 +0000 Subject: [PATCH 2/5] Minor fixes --- .../providers/generic/actors/yql_generic_token_provider.cpp | 5 +++-- .../generic/provider/yql_generic_dq_integration.cpp | 3 +-- .../yql/providers/generic/provider/yql_generic_load_meta.cpp | 5 +++-- .../yql/providers/generic/provider/yql_generic_settings.cpp | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp index ad290984b2ac..5c366161556c 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp @@ -7,7 +7,8 @@ namespace NYql::NDq { const NYql::NGeneric::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) : Source_(source) , StaticIAMToken_(source.GetToken()) - , CredentialsProvider_(nullptr) { + , CredentialsProvider_(nullptr) + { // 1. User has provided IAM-token itself. // This token will be used during the whole lifetime of a read actor. if (!StaticIAMToken_.empty()) { @@ -49,7 +50,7 @@ namespace NYql::NDq { return; } - // 3. Otherwise use credentials provider to get token + // 3. Otherwise use credentials provider to get token Y_ENSURE(CredentialsProvider_, "CredentialsProvider is not initialized"); auto iamToken = CredentialsProvider_->GetAuthInfo(); diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp index 228b52541672..95bb51184a7b 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp @@ -157,8 +157,7 @@ namespace NYql { source.SetToken(State_->Types->Credentials->FindCredentialContent( "default_" + clusterConfig.name(), "default_generic", - clusterConfig.GetToken()) - ); + clusterConfig.GetToken())); } // preserve source description for read actor diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index 2c11ebd69f95..b931c8c09679 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -279,6 +279,7 @@ namespace NYql { if (iamToken) { *dsi->mutable_credentials()->mutable_token()->mutable_value() = iamToken; *dsi->mutable_credentials()->mutable_token()->mutable_type() = "IAM"; + return; } // 2. Client provided service account creds that must be converted into IAM-token @@ -301,8 +302,8 @@ namespace NYql { false); providersIt = State_->CredentialProviders.emplace( - std::make_pair(clusterConfig.name(), credentialsProviderFactory->CreateProvider())) - .first; + std::make_pair(clusterConfig.name(), credentialsProviderFactory->CreateProvider())) + .first; } iamToken = providersIt->second->GetAuthInfo(); diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp index 14a5f839486b..1c2521573ddb 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp @@ -84,7 +84,6 @@ namespace NYql { "default_" + cluster.name(), "default_generic", cluster.GetToken()); - Y_ENSURE(iamToken); if (iamToken) { return b.SetIAMToken(iamToken).ToJson(); } From cd825684a017d2341d35eb5a91be7ec1fa83fa40 Mon Sep 17 00:00:00 2001 From: Vitaly Isaev Date: Fri, 1 Mar 2024 14:16:49 +0000 Subject: [PATCH 3/5] Order of computations --- .../yql/providers/generic/actors/yql_generic_read_actor.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp index 8bcf2896b843..9a0920e176c9 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp @@ -546,11 +546,13 @@ namespace NYql::NDq { part << ';'; */ + auto tokenProvider = CreateGenericTokenProvider(source, credentialsFactory); + const auto actor = new TGenericReadActor( inputIndex, statsLevel, genericClient, - CreateGenericTokenProvider(source, credentialsFactory), + std::move(tokenProvider), std::move(source), computeActorId, holderFactory); From 3fe6dbac1cfd7ee0a81c84567c5981aeb90fed0e Mon Sep 17 00:00:00 2001 From: Vitaly Isaev Date: Fri, 1 Mar 2024 15:38:08 +0000 Subject: [PATCH 4/5] Review fix pt. 1 --- .../generic/actors/yql_generic_read_actor.cpp | 4 ++-- .../generic/actors/yql_generic_token_provider.cpp | 10 +++++----- .../generic/actors/yql_generic_token_provider.h | 10 ++++------ .../generic/provider/yql_generic_load_meta.cpp | 4 ++-- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp index 9a0920e176c9..42fc93dedeb8 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp @@ -145,7 +145,7 @@ namespace NYql::NDq { // Prepare request NConnector::NApi::TListSplitsRequest request; NConnector::NApi::TSelect select = Source_.select(); // copy TSelect from source - TokenProvider_->MaybeFillToken(select.mutable_data_source_instance()); + TokenProvider_->MaybeFillToken(*select.mutable_data_source_instance()); *request.mutable_selects()->Add() = std::move(select); // Initialize stream @@ -241,7 +241,7 @@ namespace NYql::NDq { Splits_.cbegin(), Splits_.cend(), [&](const NConnector::NApi::TSplit& split) { NConnector::NApi::TSplit splitCopy = split; - TokenProvider_->MaybeFillToken(splitCopy.mutable_select()->mutable_data_source_instance()); + TokenProvider_->MaybeFillToken(*splitCopy.mutable_select()->mutable_data_source_instance()); *request.mutable_splits()->Add() = std::move(split); }); diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp index 5c366161556c..dab7b8388488 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp @@ -36,17 +36,17 @@ namespace NYql::NDq { // 3. If we reached this point, it means that user doesn't need token auth. } - void TGenericTokenProvider::MaybeFillToken(NConnector::NApi::TDataSourceInstance* dsi) const { + void TGenericTokenProvider::MaybeFillToken(NConnector::NApi::TDataSourceInstance& dsi) const { // 1. Don't need tokens if basic auth is set - if (dsi->credentials().has_basic()) { + if (dsi.credentials().has_basic()) { return; } - *dsi->mutable_credentials()->mutable_token()->mutable_type() = "IAM"; + *dsi.mutable_credentials()->mutable_token()->mutable_type() = "IAM"; // 2. If static IAM-token has been provided, use it if (!StaticIAMToken_.empty()) { - *dsi->mutable_credentials()->mutable_token()->mutable_value() = StaticIAMToken_; + *dsi.mutable_credentials()->mutable_token()->mutable_value() = StaticIAMToken_; return; } @@ -56,7 +56,7 @@ namespace NYql::NDq { auto iamToken = CredentialsProvider_->GetAuthInfo(); Y_ENSURE(iamToken, "CredentialsProvider returned empty IAM token"); - *dsi->mutable_credentials()->mutable_token()->mutable_value() = std::move(iamToken); + *dsi.mutable_credentials()->mutable_token()->mutable_value() = std::move(iamToken); } TGenericTokenProvider::TPtr diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h index cbd5d7909c1b..e61c641fa72a 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h @@ -7,22 +7,20 @@ namespace NYql::NDq { // When accessing external data sources using authentication via tokens, // there are two options: - // 1. Use static IAM-token provided by user (especially usefull during debugging); - // 2. Use service account credentials in order to get (and renew) token by demand. + // 1. Use static IAM-token provided by user (especially useful during debugging); + // 2. Use service account credentials in order to get (and refresh) IAM-token by demand. class TGenericTokenProvider { public: using TPtr = std::unique_ptr; - TGenericTokenProvider() = delete; - TGenericTokenProvider(const NYql::NGeneric::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory); - void MaybeFillToken(NConnector::NApi::TDataSourceInstance* dsi) const; + void MaybeFillToken(NConnector::NApi::TDataSourceInstance& dsi) const; private: NYql::NGeneric::TSource Source_; - std::string StaticIAMToken_; + TString StaticIAMToken_; NYdb::TCredentialsProviderPtr CredentialsProvider_; }; diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp index b931c8c09679..383e342c1523 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp @@ -269,7 +269,7 @@ namespace NYql { return; } - // If there are no Basic Auth, two options can be considered: + // If there are no Basic Auth parameters, two options can be considered: // 1. Client provided own IAM-token to access external data source auto iamToken = State_->Types->Credentials->FindCredentialContent( @@ -292,7 +292,7 @@ namespace NYql { Y_ENSURE(structuredTokenJSON, "empty structured token"); - // Create provider (== Token Accessor client) or get existing one. + // Create provider or get existing one. // It's crucial to reuse providers because their construction implies synchronous IO. auto providersIt = State_->CredentialProviders.find(clusterConfig.name()); if (providersIt == State_->CredentialProviders.end()) { From b5b46b2eb5d3db591b9f11eea6a1ebcd6e5b8a45 Mon Sep 17 00:00:00 2001 From: Vitaly Isaev Date: Mon, 4 Mar 2024 08:35:46 +0000 Subject: [PATCH 5/5] NGeneric::TSource -> Generic::TSource --- .../providers/generic/actors/yql_generic_read_actor.cpp | 6 +++--- .../yql/providers/generic/actors/yql_generic_read_actor.h | 2 +- .../generic/actors/yql_generic_source_factory.cpp | 4 ++-- .../generic/actors/yql_generic_token_provider.cpp | 4 ++-- .../providers/generic/actors/yql_generic_token_provider.h | 6 +++--- ydb/library/yql/providers/generic/proto/source.proto | 2 +- .../generic/provider/ut/pushdown/pushdown_ut.cpp | 8 ++++---- .../generic/provider/yql_generic_dq_integration.cpp | 2 +- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp index 42fc93dedeb8..51c02bb40456 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp @@ -104,7 +104,7 @@ namespace NYql::NDq { TCollectStatsLevel statsLevel, NConnector::IClient::TPtr client, TGenericTokenProvider::TPtr tokenProvider, - NGeneric::TSource&& source, + Generic::TSource&& source, const NActors::TActorId& computeActorId, const NKikimr::NMiniKQL::THolderFactory& holderFactory) : InputIndex_(inputIndex) @@ -499,12 +499,12 @@ namespace NYql::NDq { NKikimr::NMiniKQL::TPlainContainerCache ArrowRowContainerCache_; const NKikimr::NMiniKQL::THolderFactory& HolderFactory_; - NGeneric::TSource Source_; + Generic::TSource Source_; }; std::pair CreateGenericReadActor(NConnector::IClient::TPtr genericClient, - NGeneric::TSource&& source, + Generic::TSource&& source, ui64 inputIndex, TCollectStatsLevel statsLevel, const THashMap& /*secureParams*/, diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h index 3ea18d5e0b3f..1bdb050dcd72 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h +++ b/ydb/library/yql/providers/generic/actors/yql_generic_read_actor.h @@ -9,7 +9,7 @@ namespace NYql::NDq { std::pair - CreateGenericReadActor(NConnector::IClient::TPtr genericClient, NGeneric::TSource&& params, ui64 inputIndex, + CreateGenericReadActor(NConnector::IClient::TPtr genericClient, Generic::TSource&& params, ui64 inputIndex, TCollectStatsLevel statsLevel, const THashMap& secureParams, const THashMap& taskParams, const NActors::TActorId& computeActorId, ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp index cecbbe34e7ac..3f0cce4a6dc9 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_source_factory.cpp @@ -10,14 +10,14 @@ namespace NYql::NDq { ISecuredServiceAccountCredentialsFactory::TPtr credentialsFactory, NYql::NConnector::IClient::TPtr genericClient) { auto genericFactory = [credentialsFactory, genericClient]( - NGeneric::TSource&& settings, + Generic::TSource&& settings, IDqAsyncIoFactory::TSourceArguments&& args) { return CreateGenericReadActor(genericClient, std::move(settings), args.InputIndex, args.StatsLevel, args.SecureParams, args.TaskParams, args.ComputeActorId, credentialsFactory, args.HolderFactory); }; for (auto& sourceName : {"ClickHouseGeneric", "PostgreSqlGeneric", "YdbGeneric"}) { - factory.RegisterSource(sourceName, genericFactory); + factory.RegisterSource(sourceName, genericFactory); } } diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp index dab7b8388488..e8430b87e9ec 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.cpp @@ -4,7 +4,7 @@ namespace NYql::NDq { TGenericTokenProvider::TGenericTokenProvider( - const NYql::NGeneric::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) + const NYql::Generic::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) : Source_(source) , StaticIAMToken_(source.GetToken()) , CredentialsProvider_(nullptr) @@ -60,7 +60,7 @@ namespace NYql::NDq { } TGenericTokenProvider::TPtr - CreateGenericTokenProvider(const NYql::NGeneric::TSource& source, + CreateGenericTokenProvider(const NYql::Generic::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory) { return std::make_unique(source, credentialsFactory); } diff --git a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h index e61c641fa72a..495a44c15e57 100644 --- a/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h +++ b/ydb/library/yql/providers/generic/actors/yql_generic_token_provider.h @@ -13,18 +13,18 @@ namespace NYql::NDq { public: using TPtr = std::unique_ptr; - TGenericTokenProvider(const NYql::NGeneric::TSource& source, + TGenericTokenProvider(const NYql::Generic::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory); void MaybeFillToken(NConnector::NApi::TDataSourceInstance& dsi) const; private: - NYql::NGeneric::TSource Source_; + NYql::Generic::TSource Source_; TString StaticIAMToken_; NYdb::TCredentialsProviderPtr CredentialsProvider_; }; TGenericTokenProvider::TPtr - CreateGenericTokenProvider(const NYql::NGeneric::TSource& source, + CreateGenericTokenProvider(const NYql::Generic::TSource& source, const ISecuredServiceAccountCredentialsFactory::TPtr& credentialsFactory); } //namespace NYql::NDq diff --git a/ydb/library/yql/providers/generic/proto/source.proto b/ydb/library/yql/providers/generic/proto/source.proto index d271731c42fe..060593a99c15 100644 --- a/ydb/library/yql/providers/generic/proto/source.proto +++ b/ydb/library/yql/providers/generic/proto/source.proto @@ -2,7 +2,7 @@ syntax = "proto3"; option cc_enable_arenas = true; -package NYql.NGeneric; +package NYql.Generic; import "ydb/library/yql/providers/generic/connector/api/service/protos/connector.proto"; import "ydb/library/yql/providers/generic/connector/api/common/data_source.proto"; diff --git a/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp b/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp index ac398cd30212..48bb17d52670 100644 --- a/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp +++ b/ydb/library/yql/providers/generic/provider/ut/pushdown/pushdown_ut.cpp @@ -144,7 +144,7 @@ struct TFakeGenericClient: public NConnector::IClient { class TBuildDqSourceSettingsTransformer: public TOptimizeTransformerBase { public: - explicit TBuildDqSourceSettingsTransformer(TTypeAnnotationContext* types, NGeneric::TSource* dqSourceSettings, bool* dqSourceSettingsWereBuilt) + explicit TBuildDqSourceSettingsTransformer(TTypeAnnotationContext* types, Generic::TSource* dqSourceSettings, bool* dqSourceSettingsWereBuilt) : TOptimizeTransformerBase(types, NLog::EComponent::ProviderGeneric, {}) , DqSourceSettings_(dqSourceSettings) , DqSourceSettingsWereBuilt_(dqSourceSettingsWereBuilt) @@ -182,13 +182,13 @@ class TBuildDqSourceSettingsTransformer: public TOptimizeTransformerBase { TString sourceType; dqIntegration->FillSourceSettings(*dqSourceNode, settings, sourceType, 1); UNIT_ASSERT_STRINGS_EQUAL(sourceType, "PostgreSqlGeneric"); - UNIT_ASSERT(settings.Is()); + UNIT_ASSERT(settings.Is()); settings.UnpackTo(DqSourceSettings_); *DqSourceSettingsWereBuilt_ = true; } private: - NGeneric::TSource* DqSourceSettings_; + Generic::TSource* DqSourceSettings_; bool* DqSourceSettingsWereBuilt_; }; @@ -207,7 +207,7 @@ struct TPushdownFixture: public NUnitTest::TBaseFixture { TAutoPtr Transformer; TAutoPtr BuildDqSourceSettingsTransformer; - NGeneric::TSource DqSourceSettings; + Generic::TSource DqSourceSettings; bool DqSourceSettingsWereBuilt = false; TExprNode::TPtr InitialExprRoot; diff --git a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp index 95bb51184a7b..74a6bd819177 100644 --- a/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp +++ b/ydb/library/yql/providers/generic/provider/yql_generic_dq_integration.cpp @@ -102,7 +102,7 @@ namespace NYql { const auto& clusterConfig = State_->Configuration->ClusterNamesToClusterConfigs[clusterName]; const auto& endpoint = clusterConfig.endpoint(); - NGeneric::TSource source; + Generic::TSource source; // for backward compability full path can be used (cluster_name.`db_name.table`) // TODO: simplify during https://st.yandex-team.ru/YQ-2494