diff --git a/ydb/core/fq/libs/actors/database_resolver.cpp b/ydb/core/fq/libs/actors/database_resolver.cpp index e24c0a43e7b7..b0858f65d46e 100644 --- a/ydb/core/fq/libs/actors/database_resolver.cpp +++ b/ydb/core/fq/libs/actors/database_resolver.cpp @@ -138,6 +138,31 @@ class TResponseProcessor : public TActorBootstrapped LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): got MDB API response: code=" << ev->Get()->Response->Status); + try { + HandleResponse(ev, requestIter, errorMessage, result); + } catch (...) { + const TString msg = TStringBuilder() << "error while response processing, params " + << ((requestIter != Requests.end()) ? requestIter->second.ToDebugString() : TString{"unknown"}) + << ", details: " << CurrentExceptionMessage(); + LOG_E("ResponseProccessor::Handle(TEvHttpIncomingResponse): " << msg); + } + + LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): progress: " + << DatabaseId2Description.size() << " of " << Requests.size() << " requests are done"); + + if (HandledIds == Requests.size()) { + SendResolvedEndpointsAndDie(errorMessage); + } + } + +private: + + void HandleResponse( + NHttp::TEvHttpProxy::TEvHttpIncomingResponse::TPtr& ev, + const TRequestMap::const_iterator& requestIter, + TString& errorMessage, + TMaybe& result) + { if (ev->Get()->Error.empty() && (ev->Get()->Response && ev->Get()->Response->Status == "200")) { errorMessage = HandleSuccessfulResponse(ev, requestIter, result); } else { @@ -152,26 +177,18 @@ class TResponseProcessor : public TActorBootstrapped auto key = std::make_tuple(params.Id, params.DatabaseType, params.DatabaseAuth); if (errorMessage) { LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): put value in cache" - << "; params: " << params.ToDebugString() - << ", error: " << errorMessage); + << "; params: " << params.ToDebugString() + << ", error: " << errorMessage); Cache.Put(key, errorMessage); } else { LOG_T("ResponseProcessor::Handle(HttpIncomingResponse): put value in cache" - << "; params: " << params.ToDebugString() - << ", result: " << result->ToDebugString()); + << "; params: " << params.ToDebugString() + << ", result: " << result->ToDebugString()); Cache.Put(key, result); } } - - LOG_D("ResponseProcessor::Handle(HttpIncomingResponse): progress: " - << DatabaseId2Description.size() << " of " << Requests.size() << " requests are done"); - - if (HandledIds == Requests.size()) { - SendResolvedEndpointsAndDie(errorMessage); - } } -private: TString HandleSuccessfulResponse( NHttp::TEvHttpProxy::TEvHttpIncomingResponse::TPtr& ev, const TRequestMap::const_iterator& requestIter, @@ -179,7 +196,7 @@ class TResponseProcessor : public TActorBootstrapped ) { if (requestIter == Requests.end()) { return "unknown request"; - } + } NJson::TJsonReaderConfig jsonConfig; NJson::TJsonValue databaseInfo; @@ -224,12 +241,7 @@ class TResponseProcessor : public TActorBootstrapped const auto& status = ev->Get()->Response->Status; if (status == "403") { - const auto second = requestIter->second; - auto mdbTypeStr = NYql::DatabaseTypeLowercase(second.DatabaseType); - - return TStringBuilder() << "You have no permission to resolve database id into database endpoint. " << - "Please check that your service account has role " << - "`managed-" << mdbTypeStr << ".viewer`."; + return TStringBuilder() << "You have no permission to resolve database id into database endpoint. " + DetailedPermissionsError(requestIter->second); } auto errorMessage = ev->Get()->Error; @@ -245,6 +257,17 @@ class TResponseProcessor : public TActorBootstrapped return errorMessage; } + + TString DetailedPermissionsError(const TResolveParams& params) const { + + if (params.DatabaseType == EDatabaseType::ClickHouse || params.DatabaseType == EDatabaseType::PostgreSQL) { + auto mdbTypeStr = NYql::DatabaseTypeLowercase(params.DatabaseType); + return TStringBuilder() << "Please check that your service account has role " << + "`managed-" << mdbTypeStr << ".viewer`."; + } + return {}; + } + const TActorId Sender; TCache& Cache; const TRequestMap Requests; diff --git a/ydb/core/fq/libs/actors/ut/database_resolver_ut.cpp b/ydb/core/fq/libs/actors/ut/database_resolver_ut.cpp index 9a0baccb8a59..4055825c0c9b 100644 --- a/ydb/core/fq/libs/actors/ut/database_resolver_ut.cpp +++ b/ydb/core/fq/libs/actors/ut/database_resolver_ut.cpp @@ -14,6 +14,8 @@ namespace { using namespace NKikimr; using namespace NFq; +TString NoPermissionStr = "You have no permission to resolve database id into database endpoint. "; + struct TTestBootstrap : public TTestActorRuntime { NConfig::TCheckpointCoordinatorConfig Settings; NActors::TActorId DatabaseResolver; @@ -295,7 +297,7 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) { Y_UNIT_TEST(ClickHouse_PermissionDenied) { NYql::TIssues issues{ NYql::TIssue( - "You have no permission to resolve database id into database endpoint. Please check that your service account has role `managed-clickhouse.viewer`." + TStringBuilder{} << NoPermissionStr << "Please check that your service account has role `managed-clickhouse.viewer`." ) }; @@ -363,7 +365,7 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) { Y_UNIT_TEST(PostgreSQL_PermissionDenied) { NYql::TIssues issues{ NYql::TIssue( - "You have no permission to resolve database id into database endpoint. Please check that your service account has role `managed-postgresql.viewer`." + TStringBuilder{} << NoPermissionStr << "Please check that your service account has role `managed-postgresql.viewer`." ) }; @@ -389,6 +391,27 @@ Y_UNIT_TEST_SUITE(TDatabaseResolverTests) { issues ); } + + Y_UNIT_TEST(DataStreams_PermissionDenied) { + NYql::TIssues issues{ + NYql::TIssue( + NoPermissionStr + ) + }; + Test( + NYql::EDatabaseType::DataStreams, + NYql::NConnector::NApi::EProtocol::PROTOCOL_UNSPECIFIED, + "https://ydbc.ydb.cloud.yandex.net:8789/ydbc/cloud-prod/database?databaseId=etn021us5r9rhld1vgbh", + "403", + R"( + { + "message": "Permission denied" + })", + NYql::TDatabaseResolverResponse::TDatabaseDescription{ + }, + issues + ); + } } } // namespace NFq diff --git a/ydb/library/yql/providers/common/db_id_async_resolver/db_async_resolver.h b/ydb/library/yql/providers/common/db_id_async_resolver/db_async_resolver.h index 37673ce1a576..a03263fd8deb 100644 --- a/ydb/library/yql/providers/common/db_id_async_resolver/db_async_resolver.h +++ b/ydb/library/yql/providers/common/db_id_async_resolver/db_async_resolver.h @@ -41,14 +41,7 @@ inline NConnector::NApi::EDataSourceKind DatabaseTypeToDataSourceKind(EDatabaseT inline TString DatabaseTypeLowercase(EDatabaseType databaseType) { auto dump = ToString(databaseType); dump.to_lower(); - - switch (databaseType) { - case EDatabaseType::ClickHouse: - case EDatabaseType::PostgreSQL: - return dump; - default: - ythrow yexception() << "Unsupported database type: " << ToString(databaseType); - } + return dump; } // TODO: remove this function after /kikimr/yq/tests/control_plane_storage is moved to /ydb. @@ -73,7 +66,7 @@ struct TDatabaseAuth { NConnector::NApi::EProtocol Protocol = NConnector::NApi::EProtocol::PROTOCOL_UNSPECIFIED; bool operator==(const TDatabaseAuth& other) const { - return std::tie(StructuredToken, AddBearerToToken, UseTls) == std::tie(other.StructuredToken, other.AddBearerToToken, other.UseTls); + return std::tie(StructuredToken, AddBearerToToken, UseTls, Protocol) == std::tie(other.StructuredToken, other.AddBearerToToken, other.UseTls, Protocol); } bool operator!=(const TDatabaseAuth& other) const {