From bc5f2c7c3aa752426eb9e50e59da6fa710a0b7ef Mon Sep 17 00:00:00 2001 From: Oleg Shatov Date: Thu, 28 Dec 2023 09:05:41 +0000 Subject: [PATCH 1/5] KIKIMR-20597 Implemented tvm authentication for wilson uploader --- ydb/core/driver_lib/run/factories.h | 3 +++ .../run/kikimr_services_initializers.cpp | 11 ++++++++--- .../driver_lib/run/kikimr_services_initializers.h | 4 +++- ydb/core/driver_lib/run/run.cpp | 2 +- ydb/core/protos/config.proto | 14 ++++++++++++++ ydb/library/actors/wilson/wilson_uploader.cpp | 12 ++++++++---- ydb/library/actors/wilson/wilson_uploader.h | 8 +++++++- 7 files changed, 44 insertions(+), 10 deletions(-) diff --git a/ydb/core/driver_lib/run/factories.h b/ydb/core/driver_lib/run/factories.h index b2ee874d26e8..b40f50755408 100644 --- a/ydb/core/driver_lib/run/factories.h +++ b/ydb/core/driver_lib/run/factories.h @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -55,6 +56,8 @@ struct TModuleFactories { std::shared_ptr DataStreamsAuthFactory; std::vector AdditionalComputationNodeFactories; + std::function()> WilsonGrpcSignerFactory; + ~TModuleFactories(); }; diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp index 738ce4196258..9399995b029f 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp @@ -370,8 +370,9 @@ static bool IsServiceInitialized(NActors::TActorSystemSetup* setup, TActorId ser return false; } -TBasicServicesInitializer::TBasicServicesInitializer(const TKikimrRunConfig& runConfig) +TBasicServicesInitializer::TBasicServicesInitializer(const TKikimrRunConfig& runConfig, std::shared_ptr factories) : IKikimrServicesInitializer(runConfig) + , Factories(std::move(factories)) { } @@ -827,10 +828,14 @@ void TBasicServicesInitializer::InitializeServices(NActors::TActorSystemSetup* s if (Config.HasTracingConfig()) { const auto& tracing = Config.GetTracingConfig(); + std::unique_ptr grpcSigner; + if (Factories && Factories->WilsonGrpcSignerFactory) { + grpcSigner = Factories->WilsonGrpcSignerFactory(); + } + auto wilsonUploader = NWilson::CreateWilsonUploader(tracing.GetHost(), tracing.GetPort(), tracing.GetRootCA(), tracing.GetServiceName(), std::move(grpcSigner)); setup->LocalServices.emplace_back( NWilson::MakeWilsonUploaderId(), - TActorSetupCmd(NWilson::CreateWilsonUploader(tracing.GetHost(), tracing.GetPort(), tracing.GetRootCA(), tracing.GetServiceName()), - TMailboxType::ReadAsFilled, appData->BatchPoolId)); + TActorSetupCmd(wilsonUploader, TMailboxType::ReadAsFilled, appData->BatchPoolId)); } } diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.h b/ydb/core/driver_lib/run/kikimr_services_initializers.h index a291b4383064..2b6cd37f7c6d 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.h +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.h @@ -49,8 +49,10 @@ class TBasicServicesInitializer : public IKikimrServicesInitializer { static ISchedulerThread* CreateScheduler(const NKikimrConfig::TActorSystemConfig::TScheduler &config); + std::shared_ptr Factories; + public: - TBasicServicesInitializer(const TKikimrRunConfig& runConfig); + TBasicServicesInitializer(const TKikimrRunConfig& runConfig, std::shared_ptr factories); void InitializeServices(NActors::TActorSystemSetup *setup, const NKikimr::TAppData *appData) override; }; diff --git a/ydb/core/driver_lib/run/run.cpp b/ydb/core/driver_lib/run/run.cpp index b656afb5608a..160affb9d238 100644 --- a/ydb/core/driver_lib/run/run.cpp +++ b/ydb/core/driver_lib/run/run.cpp @@ -1375,7 +1375,7 @@ TIntrusivePtr TKikimrRunner::CreateServiceInitializers } if (serviceMask.EnableBasicServices) { - sil->AddServiceInitializer(new TBasicServicesInitializer(runConfig)); + sil->AddServiceInitializer(new TBasicServicesInitializer(runConfig, ModuleFactories)); } if (serviceMask.EnableIcbService) { sil->AddServiceInitializer(new TImmediateControlBoardInitializer(runConfig)); diff --git a/ydb/core/protos/config.proto b/ydb/core/protos/config.proto index 8b16de18701c..4f5d960f198f 100644 --- a/ydb/core/protos/config.proto +++ b/ydb/core/protos/config.proto @@ -1517,10 +1517,24 @@ message TCompactionConfig { } message TTracingConfig { + message TTvmCredentials { + required uint32 SelfTvmId = 1; + required uint32 TracingTvmId = 2; + + optional string DiskCacheDir = 3; + + oneof Secret { + string PlainTextSecret = 4; + string SecretFile = 5; + string SecretEnvironmentVariable = 6; + } + } + optional string Host = 1; optional uint32 Port = 2; optional string RootCA = 3; optional string ServiceName = 4; + optional TTvmCredentials TvmCredentials = 5; } message TFailureInjectionConfig { diff --git a/ydb/library/actors/wilson/wilson_uploader.cpp b/ydb/library/actors/wilson/wilson_uploader.cpp index 24063fe625a6..59e0dd076377 100644 --- a/ydb/library/actors/wilson/wilson_uploader.cpp +++ b/ydb/library/actors/wilson/wilson_uploader.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include namespace NWilson { @@ -32,6 +31,7 @@ namespace NWilson { std::unique_ptr Stub; grpc::CompletionQueue CQ; + std::unique_ptr GrpcSigner; std::unique_ptr Context; std::unique_ptr> Reader; NServiceProto::ExportTraceServiceResponse Response; @@ -53,11 +53,12 @@ namespace NWilson { bool WakeupScheduled = false; public: - TWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName) + TWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner) : Host(std::move(host)) , Port(std::move(port)) , RootCA(std::move(rootCA)) , ServiceName(std::move(serviceName)) + , GrpcSigner(std::move(grpcSigner)) {} ~TWilsonUploader() { @@ -142,6 +143,9 @@ namespace NWilson { ScheduleWakeup(NextSendTimestamp); Context = std::make_unique(); + if (GrpcSigner) { + GrpcSigner->SignClientContext(*Context); + } Reader = Stub->AsyncExport(Context.get(), std::move(request), &CQ); Reader->Finish(&Response, &Status, nullptr); } @@ -192,8 +196,8 @@ namespace NWilson { } // anonymous - IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName) { - return new TWilsonUploader(std::move(host), port, std::move(rootCA), std::move(serviceName)); + IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner) { + return new TWilsonUploader(std::move(host), port, std::move(rootCA), std::move(serviceName), std::move(grpcSigner)); } } // NWilson diff --git a/ydb/library/actors/wilson/wilson_uploader.h b/ydb/library/actors/wilson/wilson_uploader.h index 680f30f1dced..f48eb2c5616d 100644 --- a/ydb/library/actors/wilson/wilson_uploader.h +++ b/ydb/library/actors/wilson/wilson_uploader.h @@ -4,8 +4,14 @@ #include #include #include +#include namespace NWilson { + struct IGrpcSigner { + virtual void SignClientContext(grpc::ClientContext& context) = 0; + + virtual ~IGrpcSigner() = default; + }; struct TEvWilson : NActors::TEventLocal { opentelemetry::proto::trace::v1::Span Span; @@ -19,6 +25,6 @@ namespace NWilson { return NActors::TActorId(0, TStringBuf("WilsonUpload", 12)); } - NActors::IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName); + NActors::IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner); } // NWilson From 977cf71d53031ae31f6d76354966489ded102e24 Mon Sep 17 00:00:00 2001 From: Oleg Shatov Date: Thu, 28 Dec 2023 13:42:28 +0000 Subject: [PATCH 2/5] Enchancements --- ydb/core/driver_lib/run/factories.h | 2 +- .../run/kikimr_services_initializers.cpp | 4 +-- ydb/core/protos/config.proto | 27 ++++++++++++------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/ydb/core/driver_lib/run/factories.h b/ydb/core/driver_lib/run/factories.h index b40f50755408..eee1aefdeef8 100644 --- a/ydb/core/driver_lib/run/factories.h +++ b/ydb/core/driver_lib/run/factories.h @@ -56,7 +56,7 @@ struct TModuleFactories { std::shared_ptr DataStreamsAuthFactory; std::vector AdditionalComputationNodeFactories; - std::function()> WilsonGrpcSignerFactory; + std::function(const NKikimrConfig::TTracingConfig::TAuthCredentials&)> WilsonGrpcSignerFactory; ~TModuleFactories(); }; diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp index 9399995b029f..08ab41eafcb3 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp @@ -829,8 +829,8 @@ void TBasicServicesInitializer::InitializeServices(NActors::TActorSystemSetup* s if (Config.HasTracingConfig()) { const auto& tracing = Config.GetTracingConfig(); std::unique_ptr grpcSigner; - if (Factories && Factories->WilsonGrpcSignerFactory) { - grpcSigner = Factories->WilsonGrpcSignerFactory(); + if (tracing.HasAuthCredentials() && Factories && Factories->WilsonGrpcSignerFactory) { + grpcSigner = Factories->WilsonGrpcSignerFactory(tracing.GetAuthCredentials()); } auto wilsonUploader = NWilson::CreateWilsonUploader(tracing.GetHost(), tracing.GetPort(), tracing.GetRootCA(), tracing.GetServiceName(), std::move(grpcSigner)); setup->LocalServices.emplace_back( diff --git a/ydb/core/protos/config.proto b/ydb/core/protos/config.proto index 4f5d960f198f..12cea9ad7f99 100644 --- a/ydb/core/protos/config.proto +++ b/ydb/core/protos/config.proto @@ -1517,16 +1517,25 @@ message TCompactionConfig { } message TTracingConfig { - message TTvmCredentials { - required uint32 SelfTvmId = 1; - required uint32 TracingTvmId = 2; + message TAuthCredentials { + message TTvm { + optional string Host = 1; + optional uint32 Port = 2; - optional string DiskCacheDir = 3; + required uint32 SelfTvmId = 3; + required uint32 TracingTvmId = 4; - oneof Secret { - string PlainTextSecret = 4; - string SecretFile = 5; - string SecretEnvironmentVariable = 6; + optional string DiskCacheDir = 5; + + oneof Secret { + string PlainTextSecret = 6; + string SecretFile = 7; + string SecretEnvironmentVariable = 8; + } + } + + oneof Method { + TTvm Tvm = 1; } } @@ -1534,7 +1543,7 @@ message TTracingConfig { optional uint32 Port = 2; optional string RootCA = 3; optional string ServiceName = 4; - optional TTvmCredentials TvmCredentials = 5; + optional TAuthCredentials AuthCredentials = 5; } message TFailureInjectionConfig { From b901e7ad8a03c16eead36fde67adf5825e36c1d8 Mon Sep 17 00:00:00 2001 From: Oleg Shatov Date: Thu, 28 Dec 2023 13:49:17 +0000 Subject: [PATCH 3/5] Renaming --- ydb/core/driver_lib/run/factories.h | 2 +- ydb/core/driver_lib/run/kikimr_services_initializers.cpp | 4 ++-- ydb/core/protos/config.proto | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ydb/core/driver_lib/run/factories.h b/ydb/core/driver_lib/run/factories.h index eee1aefdeef8..e309fda49a0f 100644 --- a/ydb/core/driver_lib/run/factories.h +++ b/ydb/core/driver_lib/run/factories.h @@ -56,7 +56,7 @@ struct TModuleFactories { std::shared_ptr DataStreamsAuthFactory; std::vector AdditionalComputationNodeFactories; - std::function(const NKikimrConfig::TTracingConfig::TAuthCredentials&)> WilsonGrpcSignerFactory; + std::unique_ptr(*WilsonGrpcSignerFactory)(const NKikimrConfig::TTracingConfig::TAuthConfig&); ~TModuleFactories(); }; diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp index 08ab41eafcb3..f0af04778043 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp @@ -829,8 +829,8 @@ void TBasicServicesInitializer::InitializeServices(NActors::TActorSystemSetup* s if (Config.HasTracingConfig()) { const auto& tracing = Config.GetTracingConfig(); std::unique_ptr grpcSigner; - if (tracing.HasAuthCredentials() && Factories && Factories->WilsonGrpcSignerFactory) { - grpcSigner = Factories->WilsonGrpcSignerFactory(tracing.GetAuthCredentials()); + if (tracing.HasAuthConfig() && Factories && Factories->WilsonGrpcSignerFactory) { + grpcSigner = Factories->WilsonGrpcSignerFactory(tracing.GetAuthConfig()); } auto wilsonUploader = NWilson::CreateWilsonUploader(tracing.GetHost(), tracing.GetPort(), tracing.GetRootCA(), tracing.GetServiceName(), std::move(grpcSigner)); setup->LocalServices.emplace_back( diff --git a/ydb/core/protos/config.proto b/ydb/core/protos/config.proto index 12cea9ad7f99..a1adc8618b27 100644 --- a/ydb/core/protos/config.proto +++ b/ydb/core/protos/config.proto @@ -1517,7 +1517,7 @@ message TCompactionConfig { } message TTracingConfig { - message TAuthCredentials { + message TAuthConfig { message TTvm { optional string Host = 1; optional uint32 Port = 2; @@ -1543,7 +1543,7 @@ message TTracingConfig { optional uint32 Port = 2; optional string RootCA = 3; optional string ServiceName = 4; - optional TAuthCredentials AuthCredentials = 5; + optional TAuthCredentials AuthConfig = 5; } message TFailureInjectionConfig { From 5ce1ed93e6acd1557a5a9d2ef71733ab9fa8223c Mon Sep 17 00:00:00 2001 From: Oleg Shatov Date: Fri, 29 Dec 2023 10:42:59 +0000 Subject: [PATCH 4/5] Config issues fixed --- ydb/core/protos/config.proto | 2 +- ydb/tools/cfg/static.py | 42 +++++++++++++++++++++++++++++------- ydb/tools/cfg/validation.py | 1 + 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/ydb/core/protos/config.proto b/ydb/core/protos/config.proto index a1adc8618b27..91b546f145fe 100644 --- a/ydb/core/protos/config.proto +++ b/ydb/core/protos/config.proto @@ -1543,7 +1543,7 @@ message TTracingConfig { optional uint32 Port = 2; optional string RootCA = 3; optional string ServiceName = 4; - optional TAuthCredentials AuthConfig = 5; + optional TAuthConfig AuthConfig = 5; } message TFailureInjectionConfig { diff --git a/ydb/tools/cfg/static.py b/ydb/tools/cfg/static.py index 09cb9bbba132..84bf540891da 100644 --- a/ydb/tools/cfg/static.py +++ b/ydb/tools/cfg/static.py @@ -114,11 +114,13 @@ def __init__( ) self._enable_cms_config_cache = template.get("enable_cms_config_cache", enable_cms_config_cache) if "tracing" in template: + tracing = template["tracing"] self.__tracing = ( - template["tracing"]["host"], - template["tracing"]["port"], - template["tracing"]["root_ca"], - template["tracing"]["service_name"], + tracing["host"], + tracing["port"], + tracing["root_ca"], + tracing["service_name"], + tracing.get("auth_config") ) else: self.__tracing = None @@ -1121,12 +1123,36 @@ def __generate_sys_txt(self): def __generate_tracing_txt(self): pb = config_pb2.TAppConfig() if self.__tracing: + tracing_pb = pb.TracingConfig ( - pb.TracingConfig.Host, - pb.TracingConfig.Port, - pb.TracingConfig.RootCA, - pb.TracingConfig.ServiceName, + tracing_pb.Host, + tracing_pb.Port, + tracing_pb.RootCA, + tracing_pb.ServiceName, + auth_config ) = self.__tracing + + if auth_config: + auth_pb = tracing_pb.AuthConfig + if "tvm" in auth_config: + tvm = auth_config.get("tvm") + tvm_pb = auth_pb.Tvm + + if "host" in tvm: + tvm_pb.Host = tvm["host"] + if "port" in tvm: + tvm_pb.Port = tvm["port"] + tvm_pb.SelfTvmId = tvm["self_tvm_id"] + tvm_pb.TracingTvmId = tvm["tracing_tvm_id"] + tvm_pb.DiskCacheDir = tvm["disk_cache_dir"] + + if "plain_text_secret" in tvm: + tvm_pb.PlainTextSecret = tvm["plain_text_secret"] + elif "secret_file" in tvm: + tvm_pb.SecretFile = tvm["secret_file"] + elif "secret_environment_variable" in tvm: + tvm_pb.SecretEnvironmentVariable = tvm["secret_environment_variable"] + self.__proto_configs["tracing.txt"] = pb def __generate_sys_txt_advanced(self): diff --git a/ydb/tools/cfg/validation.py b/ydb/tools/cfg/validation.py index 8f385718b803..4e17e4ac78b2 100644 --- a/ydb/tools/cfg/validation.py +++ b/ydb/tools/cfg/validation.py @@ -133,6 +133,7 @@ port=dict(type="integer"), root_ca=dict(type="string"), service_name=dict(type="string"), + auth_config=dict(type="object"), ), required=[ "host", From 4f0c42bd05df5e6ebad8f7d3fb70a13641496ae3 Mon Sep 17 00:00:00 2001 From: Oleg Shatov Date: Sat, 30 Dec 2023 02:56:35 +0000 Subject: [PATCH 5/5] Comments resolved --- .../run/kikimr_services_initializers.cpp | 8 +++++++- ydb/library/actors/wilson/wilson_uploader.cpp | 20 +++++++++++-------- ydb/library/actors/wilson/wilson_uploader.h | 12 ++++++++++- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp index f0af04778043..0e2c31332004 100644 --- a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp +++ b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp @@ -832,7 +832,13 @@ void TBasicServicesInitializer::InitializeServices(NActors::TActorSystemSetup* s if (tracing.HasAuthConfig() && Factories && Factories->WilsonGrpcSignerFactory) { grpcSigner = Factories->WilsonGrpcSignerFactory(tracing.GetAuthConfig()); } - auto wilsonUploader = NWilson::CreateWilsonUploader(tracing.GetHost(), tracing.GetPort(), tracing.GetRootCA(), tracing.GetServiceName(), std::move(grpcSigner)); + auto wilsonUploader = NWilson::WilsonUploaderParams { + .Host = tracing.GetHost(), + .Port = static_cast(tracing.GetPort()), + .RootCA = tracing.GetRootCA(), + .ServiceName = tracing.GetServiceName(), + .GrpcSigner = std::move(grpcSigner), + }.CreateUploader(); setup->LocalServices.emplace_back( NWilson::MakeWilsonUploaderId(), TActorSetupCmd(wilsonUploader, TMailboxType::ReadAsFilled, appData->BatchPoolId)); diff --git a/ydb/library/actors/wilson/wilson_uploader.cpp b/ydb/library/actors/wilson/wilson_uploader.cpp index 59e0dd076377..2e604e48a1af 100644 --- a/ydb/library/actors/wilson/wilson_uploader.cpp +++ b/ydb/library/actors/wilson/wilson_uploader.cpp @@ -53,12 +53,12 @@ namespace NWilson { bool WakeupScheduled = false; public: - TWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner) - : Host(std::move(host)) - , Port(std::move(port)) - , RootCA(std::move(rootCA)) - , ServiceName(std::move(serviceName)) - , GrpcSigner(std::move(grpcSigner)) + TWilsonUploader(WilsonUploaderParams params) + : Host(std::move(params.Host)) + , Port(std::move(params.Port)) + , RootCA(std::move(params.RootCA)) + , ServiceName(std::move(params.ServiceName)) + , GrpcSigner(std::move(params.GrpcSigner)) {} ~TWilsonUploader() { @@ -196,8 +196,12 @@ namespace NWilson { } // anonymous - IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner) { - return new TWilsonUploader(std::move(host), port, std::move(rootCA), std::move(serviceName), std::move(grpcSigner)); + IActor* CreateWilsonUploader(WilsonUploaderParams params) { + return new TWilsonUploader(std::move(params)); + } + + IActor* WilsonUploaderParams::CreateUploader() && { + return CreateWilsonUploader(std::move(*this)); } } // NWilson diff --git a/ydb/library/actors/wilson/wilson_uploader.h b/ydb/library/actors/wilson/wilson_uploader.h index f48eb2c5616d..be1cbcac4e7a 100644 --- a/ydb/library/actors/wilson/wilson_uploader.h +++ b/ydb/library/actors/wilson/wilson_uploader.h @@ -25,6 +25,16 @@ namespace NWilson { return NActors::TActorId(0, TStringBuf("WilsonUpload", 12)); } - NActors::IActor *CreateWilsonUploader(TString host, ui16 port, TString rootCA, TString serviceName, std::unique_ptr grpcSigner); + struct WilsonUploaderParams { + TString Host; + ui16 Port; + TString RootCA; + TString ServiceName; + std::unique_ptr GrpcSigner; + + NActors::IActor* CreateUploader() &&; + }; + + NActors::IActor* CreateWilsonUploader(WilsonUploaderParams params); } // NWilson