Skip to content

Commit f185660

Browse files
kardymondskardymonds
committed
move checks to factory
1 parent ef18d5b commit f185660

File tree

8 files changed

+79
-63
lines changed

8 files changed

+79
-63
lines changed

‎ydb/core/base/appdata_fwd.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,6 @@ struct TAppData {
287287
// Tracing configurator (look for tracing config in ydb/core/jaeger_tracing/actors_tracing_control)
288288
TIntrusivePtr<NKikimr::NJaegerTracing::TSamplingThrottlingConfigurator> TracingConfigurator;
289289

290-
TSet<TString> AvailableExternalDataSources;
291-
292290
TAppData(
293291
ui32 sysPoolId, ui32 userPoolId, ui32 ioPoolId, ui32 batchPoolId,
294292
TMap<TString, ui32> servicePools,

‎ydb/core/driver_lib/run/run.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,13 +251,6 @@ class TDomainsInitializer : public IAppDataInitializer {
251251
appData->CompactionConfig = Config.GetCompactionConfig();
252252
appData->BackgroundCleaningConfig = Config.GetBackgroundCleaningConfig();
253253
appData->DataErasureConfig = Config.GetDataErasureConfig();
254-
255-
const auto& queryServiceConfig = Config.GetQueryServiceConfig();
256-
if (queryServiceConfig.AvailableExternalDataSourcesSize() > 0) {
257-
const auto& availableSources = queryServiceConfig.GetAvailableExternalDataSources();
258-
TSet<TString> set(availableSources.cbegin(), availableSources.cend());
259-
appData->AvailableExternalDataSources = std::move(set);
260-
}
261254
}
262255
};
263256

‎ydb/core/external_sources/external_source_factory.cpp‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,27 @@ namespace NKikimr::NExternalSource {
1414
namespace {
1515

1616
struct TExternalSourceFactory : public IExternalSourceFactory {
17-
TExternalSourceFactory(const TMap<TString, IExternalSource::TPtr>& sources)
17+
TExternalSourceFactory(
18+
const TMap<TString, IExternalSource::TPtr>& sources,
19+
const std::set<TString>& availableExternalDataSources)
1820
: Sources(sources)
21+
, AvailableExternalDataSources(availableExternalDataSources)
1922
{}
2023

2124
IExternalSource::TPtr GetOrCreate(const TString& type) const override {
2225
auto it = Sources.find(type);
23-
if (it != Sources.end()) {
24-
return it->second;
26+
if (it == Sources.end()) {
27+
throw TExternalSourceException() << "External source with type " << type << " was not found";
2528
}
26-
throw TExternalSourceException() << "External source with type " << type << " was not found";
29+
if (!AvailableExternalDataSources.empty() && !AvailableExternalDataSources.contains(type)) {
30+
throw TExternalSourceException() << "External source with type " << type << " are disabled. Please contact your system administrator to enable it";
31+
}
32+
return it->second;
2733
}
2834

2935
private:
3036
TMap<TString, IExternalSource::TPtr> Sources;
37+
std::set<TString> AvailableExternalDataSources;
3138
};
3239

3340
}
@@ -37,7 +44,8 @@ IExternalSourceFactory::TPtr CreateExternalSourceFactory(const std::vector<TStri
3744
size_t pathsLimit,
3845
std::shared_ptr<NYql::ISecuredServiceAccountCredentialsFactory> credentialsFactory,
3946
bool enableInfer,
40-
bool allowLocalFiles) {
47+
bool allowLocalFiles,
48+
const std::set<TString>& availableExternalDataSources) {
4149
std::vector<TRegExMatch> hostnamePatternsRegEx(hostnamePatterns.begin(), hostnamePatterns.end());
4250
return MakeIntrusive<TExternalSourceFactory>(TMap<TString, IExternalSource::TPtr>{
4351
{
@@ -84,7 +92,8 @@ IExternalSourceFactory::TPtr CreateExternalSourceFactory(const std::vector<TStri
8492
ToString(NYql::EDatabaseType::Solomon),
8593
CreateExternalDataSource(TString{NYql::SolomonProviderName}, {"NONE", "TOKEN"}, {}, hostnamePatternsRegEx)
8694
}
87-
});
95+
},
96+
availableExternalDataSources);
8897
}
8998

9099
}

‎ydb/core/external_sources/external_source_factory.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ IExternalSourceFactory::TPtr CreateExternalSourceFactory(const std::vector<TStri
1616
size_t pathsLimit = 50000,
1717
std::shared_ptr<NYql::ISecuredServiceAccountCredentialsFactory> credentialsFactory = nullptr,
1818
bool enableInfer = false,
19-
bool allowLocalFiles = false);
19+
bool allowLocalFiles = false,
20+
const std::set<TString>& availableExternalDataSources = {});
2021

2122
}

‎ydb/core/kqp/gateway/behaviour/external_data_source/manager.cpp‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -122,18 +122,6 @@ TYqlConclusion<std::pair<TString, TString>> SplitPath(const TString& tableName,
122122
return TYqlConclusionStatus::Success();
123123
}
124124

125-
[[nodiscard]] TYqlConclusionStatus CheckAvailableExternalDataSources(const NKikimrSchemeOp::TExternalDataSourceDescription& externaDataSourceDesc, const TExternalDataSourceManager::TInternalModificationContext& context) {
126-
auto* actorSystem = context.GetExternalData().GetActorSystem();
127-
if (!actorSystem) {
128-
return TYqlConclusionStatus::Fail(NYql::TIssuesIds::KIKIMR_INTERNAL_ERROR, "Internal error. EXTERNAL_DATA_SOURCE creation and drop operations needs an actor system. Please contact internal support");
129-
}
130-
auto sourceType = externaDataSourceDesc.GetSourceType();
131-
if (!AppData(actorSystem)->AvailableExternalDataSources.empty() && !AppData(actorSystem)->AvailableExternalDataSources.contains(sourceType)) {
132-
return TYqlConclusionStatus::Fail(NYql::TIssuesIds::KIKIMR_UNSUPPORTED, TStringBuilder() << "External data source " << sourceType << " are disabled. Please contact your system administrator to enable it");
133-
}
134-
return TYqlConclusionStatus::Success();
135-
}
136-
137125
[[nodiscard]] TYqlConclusionStatus ErrorFromActivityType(TExternalDataSourceManager::EActivityType activityType) {
138126
using EActivityType = TExternalDataSourceManager::EActivityType;
139127

@@ -221,13 +209,7 @@ TYqlConclusionStatus TExternalDataSourceManager::PrepareCreateExternalDataSource
221209
schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpCreateExternalDataSource);
222210
schemeTx.SetFailedOnAlreadyExists(!settings.GetExistingOk());
223211

224-
if (auto status = FillCreateExternalDataSourceDesc(*schemeTx.MutableCreateExternalDataSource(), name, settings); status.IsFail()) {
225-
return status;
226-
}
227-
if (auto status = CheckAvailableExternalDataSources(schemeTx.GetCreateExternalDataSource(), context); status.IsFail()) {
228-
return status;
229-
}
230-
return TYqlConclusionStatus::Success();
212+
return FillCreateExternalDataSourceDesc(*schemeTx.MutableCreateExternalDataSource(), name, settings);
231213
}
232214

233215
TYqlConclusionStatus TExternalDataSourceManager::PrepareDropExternalDataSource(NKqpProto::TKqpSchemeOperation& schemeOperation, const NYql::TDropObjectSettings& settings, TInternalModificationContext& context) const {

‎ydb/core/kqp/host/kqp_host.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,8 @@ class TKqpHost : public IKqpHost {
11691169
FederatedQuerySetup->S3GatewayConfig.GetGeneratorPathsLimit(),
11701170
FederatedQuerySetup ? FederatedQuerySetup->CredentialsFactory : nullptr,
11711171
Config->FeatureFlags.GetEnableExternalSourceSchemaInference(),
1172-
FederatedQuerySetup->S3GatewayConfig.GetAllowLocalFiles());
1172+
FederatedQuerySetup->S3GatewayConfig.GetAllowLocalFiles(),
1173+
{});
11731174
}
11741175
}
11751176

‎ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp‎

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6618,36 +6618,66 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
66186618
}
66196619

66206620
Y_UNIT_TEST(DisableS3ExternalDataSource) {
6621-
TKikimrRunner kikimr;
6621+
NKikimrConfig::TAppConfig appCfg;
6622+
appCfg.MutableQueryServiceConfig()->AddAvailableExternalDataSources("PostgreSQL");
6623+
TKikimrRunner kikimr(appCfg);
66226624
kikimr.GetTestServer().GetRuntime()->GetAppData(0).FeatureFlags.SetEnableExternalDataSources(true);
6623-
kikimr.GetTestServer().GetRuntime()->GetAppData(0).AvailableExternalDataSources.insert("PostgreSQL");
6625+
{
6626+
auto db = kikimr.GetTableClient();
6627+
auto session = db.CreateSession().GetValueSync().GetSession();
6628+
TString externalDataSourceName = "/Root/ExternalDataSource2";
6629+
auto query = TStringBuilder() << R"(
6630+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6631+
SOURCE_TYPE="ObjectStorage",
6632+
LOCATION="my-bucket",
6633+
AUTH_METHOD="NONE"
6634+
);)";
6635+
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
6636+
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::SCHEME_ERROR);
6637+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External source with type ObjectStorage are disabled. Please contact your system administrator to enable it", result.GetIssues().ToString());
66246638

6625-
auto db = kikimr.GetTableClient();
6626-
auto session = db.CreateSession().GetValueSync().GetSession();
6627-
TString externalDataSourceName = "/Root/ExternalDataSource";
6628-
auto query = TStringBuilder() << R"(
6629-
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6630-
SOURCE_TYPE="ObjectStorage",
6631-
LOCATION="my-bucket",
6632-
AUTH_METHOD="NONE"
6633-
);)";
6634-
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
6635-
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::UNSUPPORTED);
6636-
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External data source ObjectStorage are disabled. Please contact your system administrator to enable it", result.GetIssues().ToString());
6639+
auto query2 = TStringBuilder() << R"(
6640+
CREATE OBJECT `baz2` (TYPE SECRET) WITH value=`MySecretData`;
66376641

6638-
auto query2 = TStringBuilder() << R"(
6639-
CREATE OBJECT `baz` (TYPE SECRET) WITH value=`MySecretData`;
6642+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6643+
SOURCE_TYPE="PostgreSQL",
6644+
LOCATION="my-bucket",
6645+
AUTH_METHOD="BASIC",
6646+
LOGIN="admin",
6647+
PASSWORD_SECRET_NAME = "baz2",
6648+
DATABASE_NAME="cheburashka"
6649+
);)";
6650+
result = session.ExecuteSchemeQuery(query2).GetValueSync();
6651+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6652+
}
6653+
{
6654+
auto client = kikimr.GetQueryClient();
6655+
auto session = client.GetSession().GetValueSync().GetSession();
6656+
TString externalDataSourceName = "/Root/ExternalDataSource";
6657+
auto query = TStringBuilder() << R"(
6658+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6659+
SOURCE_TYPE="ObjectStorage",
6660+
LOCATION="my-bucket",
6661+
AUTH_METHOD="NONE"
6662+
);)";
6663+
auto result = session.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
6664+
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::GENERIC_ERROR);
6665+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External source with type ObjectStorage are disabled. Please contact your system administrator to enable it", result.GetIssues().ToString());
66406666

6641-
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6642-
SOURCE_TYPE="PostgreSQL",
6643-
LOCATION="my-bucket",
6644-
AUTH_METHOD="BASIC",
6645-
LOGIN="admin",
6646-
PASSWORD_SECRET_NAME = "baz",
6647-
DATABASE_NAME="cheburashka"
6648-
);)";
6649-
result = session.ExecuteSchemeQuery(query2).GetValueSync();
6650-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6667+
auto query2 = TStringBuilder() << R"(
6668+
CREATE OBJECT `baz` (TYPE SECRET) WITH value=`MySecretData`;
6669+
6670+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6671+
SOURCE_TYPE="PostgreSQL",
6672+
LOCATION="my-bucket",
6673+
AUTH_METHOD="BASIC",
6674+
LOGIN="admin",
6675+
PASSWORD_SECRET_NAME = "baz",
6676+
DATABASE_NAME="cheburashka"
6677+
);)";
6678+
result = session.ExecuteQuery(query2, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
6679+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6680+
}
66516681
}
66526682

66536683
Y_UNIT_TEST(CreateExternalDataSourceValidationAuthMethod) {

‎ydb/core/tx/schemeshard/schemeshard_impl.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7231,13 +7231,15 @@ void TSchemeShard::ApplyConsoleConfigs(const NKikimrConfig::TAppConfig& appConfi
72317231

72327232
if (appConfig.HasQueryServiceConfig()) {
72337233
const auto& hostnamePatterns = appConfig.GetQueryServiceConfig().GetHostnamePatterns();
7234+
const auto& availableExternalDataSources = appConfig.GetQueryServiceConfig().GetAvailableExternalDataSources();
72347235
ExternalSourceFactory = NExternalSource::CreateExternalSourceFactory(
72357236
std::vector<TString>(hostnamePatterns.begin(), hostnamePatterns.end()),
72367237
nullptr,
72377238
appConfig.GetQueryServiceConfig().GetS3().GetGeneratorPathsLimit(),
72387239
nullptr,
72397240
appConfig.GetFeatureFlags().GetEnableExternalSourceSchemaInference(),
7240-
appConfig.GetQueryServiceConfig().GetS3().GetAllowLocalFiles()
7241+
appConfig.GetQueryServiceConfig().GetS3().GetAllowLocalFiles(),
7242+
std::set<TString>(availableExternalDataSources.cbegin(), availableExternalDataSources.cend())
72417243
);
72427244
}
72437245

0 commit comments

Comments
 (0)