Skip to content

Commit cd35a2c

Browse files
JimmyCYJJimmyCYJ
committed
Revise per comments.
Signed-off-by: JimmyCYJ <jimmychen.0102@gmail.com>
1 parent 9cb2e20 commit cd35a2c

4 files changed

Lines changed: 56 additions & 49 deletions

File tree

‎source/common/secret/sds_api.cc‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
namespace Envoy {
1414
namespace Secret {
1515

16-
SdsApi::SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher,
17-
Runtime::RandomGenerator& random, Stats::Store& stats,
18-
Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager,
19-
const envoy::api::v2::core::ConfigSource& sds_config, std::string sds_config_name,
20-
std::function<void()> destructor_cb)
16+
template <class SecretType>
17+
SdsApi<SecretType>::SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher,
18+
Runtime::RandomGenerator& random, Stats::Store& stats,
19+
Upstream::ClusterManager& cluster_manager, Init::Manager& init_manager,
20+
const envoy::api::v2::core::ConfigSource& sds_config,
21+
std::string sds_config_name, std::function<void()> destructor_cb)
2122
: secret_hash_(0), local_info_(local_info), dispatcher_(dispatcher), random_(random),
2223
stats_(stats), cluster_manager_(cluster_manager), sds_config_(sds_config),
2324
sds_config_name_(sds_config_name), clean_up_(destructor_cb) {
@@ -28,7 +29,7 @@ SdsApi::SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispat
2829
init_manager.registerTarget(*this);
2930
}
3031

31-
void SdsApi::initialize(std::function<void()> callback) {
32+
template <class SecretType> void SdsApi<SecretType>::initialize(std::function<void()> callback) {
3233
initialize_callback_ = callback;
3334

3435
subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource<
@@ -42,7 +43,8 @@ void SdsApi::initialize(std::function<void()> callback) {
4243
subscription_->start({sds_config_name_}, *this);
4344
}
4445

45-
void SdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&) {
46+
template <class SecretType>
47+
void SdsApi<SecretType>::onConfigUpdate(const ResourceVector& resources, const std::string&) {
4648
if (resources.empty()) {
4749
throw EnvoyException(
4850
fmt::format("Missing SDS resources for {} in onConfigUpdate()", sds_config_name_));
@@ -65,12 +67,12 @@ void SdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&)
6567
runInitializeCallbackIfAny();
6668
}
6769

68-
void SdsApi::onConfigUpdateFailed(const EnvoyException*) {
70+
template <class SecretType> void SdsApi<SecretType>::onConfigUpdateFailed(const EnvoyException*) {
6971
// We need to allow server startup to continue, even if we have a bad config.
7072
runInitializeCallbackIfAny();
7173
}
7274

73-
void SdsApi::runInitializeCallbackIfAny() {
75+
template <class SecretType> void SdsApi<SecretType>::runInitializeCallbackIfAny() {
7476
if (initialize_callback_) {
7577
initialize_callback_();
7678
initialize_callback_ = nullptr;
@@ -82,8 +84,7 @@ void TlsCertificateSdsApi::updateConfigHelper(const envoy::api::v2::auth::Secret
8284
if (new_hash != secret_hash_ &&
8385
secret.type_case() == envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate) {
8486
secret_hash_ = new_hash;
85-
tls_certificate_secrets_ =
86-
std::make_unique<Ssl::TlsCertificateConfigImpl>(secret.tls_certificate());
87+
secrets_ = std::make_unique<Ssl::TlsCertificateConfigImpl>(secret.tls_certificate());
8788

8889
update_callback_manager_.runCallbacks();
8990
}
@@ -95,7 +96,7 @@ void CertificateValidationContextSdsApi::updateConfigHelper(
9596
if (new_hash != secret_hash_ &&
9697
secret.type_case() == envoy::api::v2::auth::Secret::TypeCase::kValidationContext) {
9798
secret_hash_ = new_hash;
98-
certificate_validation_context_secrets_ =
99+
secrets_ =
99100
std::make_unique<Ssl::CertificateValidationContextConfigImpl>(secret.validation_context());
100101

101102
update_callback_manager_.runCallbacks();

‎source/common/secret/sds_api.h‎

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ namespace Secret {
2323
/**
2424
* SDS API implementation that fetches secrets from SDS server via Subscription.
2525
*/
26+
template <class SecretType>
2627
class SdsApi : public Init::Target,
28+
public SecretProvider<SecretType>,
2729
public Config::SubscriptionCallbacks<envoy::api::v2::auth::Secret> {
2830
public:
2931
SdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher,
@@ -42,10 +44,19 @@ class SdsApi : public Init::Target,
4244
return MessageUtil::anyConvert<envoy::api::v2::auth::Secret>(resource).name();
4345
}
4446

47+
// SecretProvider
48+
const SecretType* secret() const override { return secrets_.get(); }
49+
50+
Common::CallbackHandle* addUpdateCallback(std::function<void()> callback) override {
51+
return update_callback_manager_.add(callback);
52+
}
53+
4554
protected:
4655
// Updates local storage of dynamic secrets and invokes callbacks.
47-
virtual void updateConfigHelper(const envoy::api::v2::auth::Secret&) {}
56+
virtual void updateConfigHelper(const envoy::api::v2::auth::Secret&) PURE;
4857
uint64_t secret_hash_;
58+
std::unique_ptr<SecretType> secrets_;
59+
Common::CallbackManager<> update_callback_manager_;
4960

5061
private:
5162
void runInitializeCallbackIfAny();
@@ -67,7 +78,7 @@ class SdsApi : public Init::Target,
6778
/**
6879
* TlsCertificateSdsApi implementation maintains and updates dynamic TLS certificate secrets.
6980
*/
70-
class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider {
81+
class TlsCertificateSdsApi : public SdsApi<Ssl::TlsCertificateConfig> {
7182
public:
7283
TlsCertificateSdsApi(const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher,
7384
Runtime::RandomGenerator& random, Stats::Store& stats,
@@ -77,29 +88,16 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider
7788
: SdsApi(local_info, dispatcher, random, stats, cluster_manager, init_manager, sds_config,
7889
sds_config_name, destructor_cb) {}
7990

80-
// SecretProvider
81-
const Ssl::TlsCertificateConfig* secret() const override {
82-
return tls_certificate_secrets_.get();
83-
}
84-
85-
Common::CallbackHandle* addUpdateCallback(std::function<void()> callback) override {
86-
return update_callback_manager_.add(callback);
87-
}
88-
8991
private:
9092
// SdsApi
9193
void updateConfigHelper(const envoy::api::v2::auth::Secret& secret) override;
92-
93-
Ssl::TlsCertificateConfigPtr tls_certificate_secrets_;
94-
Common::CallbackManager<> update_callback_manager_;
9594
};
9695

9796
/**
9897
* CertificateValidationContextSdsApi implementation maintains and updates dynamic certificate
9998
* validation context secrets.
10099
*/
101-
class CertificateValidationContextSdsApi : public SdsApi,
102-
public CertificateValidationContextConfigProvider {
100+
class CertificateValidationContextSdsApi : public SdsApi<Ssl::CertificateValidationContextConfig> {
103101
public:
104102
CertificateValidationContextSdsApi(const LocalInfo::LocalInfo& local_info,
105103
Event::Dispatcher& dispatcher,
@@ -112,21 +110,9 @@ class CertificateValidationContextSdsApi : public SdsApi,
112110
: SdsApi(local_info, dispatcher, random, stats, cluster_manager, init_manager, sds_config,
113111
sds_config_name, destructor_cb) {}
114112

115-
// SecretProvider
116-
const Ssl::CertificateValidationContextConfig* secret() const override {
117-
return certificate_validation_context_secrets_.get();
118-
}
119-
120-
Common::CallbackHandle* addUpdateCallback(std::function<void()> callback) override {
121-
return update_callback_manager_.add(callback);
122-
}
123-
124113
private:
125114
// SdsApi
126115
void updateConfigHelper(const envoy::api::v2::auth::Secret& secret) override;
127-
128-
Ssl::CertificateValidationContextConfigPtr certificate_validation_context_secrets_;
129-
Common::CallbackManager<> update_callback_manager_;
130116
};
131117

132118
} // namespace Secret

‎source/common/ssl/context_config_impl.cc‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ getCertificateValidationContextConfigProvider(
6565
sds_secret_config.name()));
6666
}
6767
return secret_provider;
68+
} else {
69+
return factory_context.secretManager().findOrCreateCertificateValidationContextProvider(
70+
sds_secret_config.sds_config(), sds_secret_config.name(), factory_context);
6871
}
6972
}
7073
return nullptr;
@@ -98,17 +101,21 @@ ContextConfigImpl::ContextConfigImpl(
98101
ecdh_curves_(StringUtil::nonEmptyStringOrDefault(
99102
RepeatedPtrUtil::join(config.tls_params().ecdh_curves(), ":"), DEFAULT_ECDH_CURVES)),
100103
tls_certficate_provider_(getTlsCertificateConfigProvider(config, factory_context)),
101-
secret_update_callback_handle_(nullptr),
104+
tls_certificate_update_callback_handle_(nullptr),
102105
certficate_validation_context_provider_(
103106
getCertificateValidationContextConfigProvider(config, factory_context)),
107+
certificate_validation_context_update_callback_handle_(nullptr),
104108
min_protocol_version_(
105109
tlsVersionFromProto(config.tls_params().tls_minimum_protocol_version(), TLS1_VERSION)),
106110
max_protocol_version_(tlsVersionFromProto(config.tls_params().tls_maximum_protocol_version(),
107111
TLS1_2_VERSION)) {}
108112

109113
ContextConfigImpl::~ContextConfigImpl() {
110-
if (secret_update_callback_handle_) {
111-
secret_update_callback_handle_->remove();
114+
if (tls_certificate_update_callback_handle_) {
115+
tls_certificate_update_callback_handle_->remove();
116+
}
117+
if (certificate_validation_context_update_callback_handle_) {
118+
certificate_validation_context_update_callback_handle_->remove();
112119
}
113120
}
114121

‎source/common/ssl/context_config_impl.h‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,28 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
3939

4040
bool isReady() const override {
4141
// Either tls_certficate_provider_ is nullptr or
42-
// tls_certficate_provider_->secret() is NOT nullptr.
43-
return !tls_certficate_provider_ || tls_certficate_provider_->secret() != nullptr;
42+
// tls_certficate_provider_->secret() is NOT nullptr and
43+
// either certficate_validation_context_provider_ is nullptr or
44+
// certficate_validation_context_provider_->secret() is NOT nullptr.
45+
return (!tls_certficate_provider_ || tls_certficate_provider_->secret() != nullptr) &&
46+
(!certficate_validation_context_provider_ ||
47+
certficate_validation_context_provider_->secret() != nullptr);
4448
}
4549

4650
void setSecretUpdateCallback(std::function<void()> callback) override {
4751
if (tls_certficate_provider_) {
48-
if (secret_update_callback_handle_) {
49-
secret_update_callback_handle_->remove();
52+
if (tls_certificate_update_callback_handle_) {
53+
tls_certificate_update_callback_handle_->remove();
5054
}
51-
secret_update_callback_handle_ = tls_certficate_provider_->addUpdateCallback(callback);
55+
tls_certificate_update_callback_handle_ =
56+
tls_certficate_provider_->addUpdateCallback(callback);
57+
}
58+
if (certficate_validation_context_provider_) {
59+
if (certificate_validation_context_update_callback_handle_) {
60+
certificate_validation_context_update_callback_handle_->remove();
61+
}
62+
certificate_validation_context_update_callback_handle_ =
63+
certficate_validation_context_provider_->addUpdateCallback(callback);
5264
}
5365
}
5466

@@ -69,9 +81,10 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
6981
const std::string cipher_suites_;
7082
const std::string ecdh_curves_;
7183
Secret::TlsCertificateConfigProviderSharedPtr tls_certficate_provider_;
72-
Common::CallbackHandle* secret_update_callback_handle_;
84+
Common::CallbackHandle* tls_certificate_update_callback_handle_;
7385
Secret::CertificateValidationContextConfigProviderSharedPtr
7486
certficate_validation_context_provider_;
87+
Common::CallbackHandle* certificate_validation_context_update_callback_handle_;
7588
const unsigned min_protocol_version_;
7689
const unsigned max_protocol_version_;
7790
};

0 commit comments

Comments
 (0)