-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sds: allow multiple init managers share sds target #14357
Conversation
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
@snowp this is ready to go. PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix looks good, just some small comments
@@ -92,6 +92,7 @@ class SecretManagerImpl : public SecretManager { | |||
config_name, unregister_secret_provider); | |||
dynamic_secret_providers_[map_key] = secret_provider; | |||
} | |||
secret_provider_context.initManager().add(*secret_provider->initTarget()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment explaining the importance of doing it here?
template <class T> std::string intResourceName(const T& m) { | ||
// gcc doesn't allow inline template function to be specialized, using a constexpr if to | ||
// workaround. | ||
if constexpr (std::is_same<T, envoy::config::endpoint::v3::ClusterLoadAssignment>::value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_same_v
?
Signed-off-by: Lizan Zhou <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* master: (49 commits) sds: allow multiple init managers share sds target (envoyproxy#14357) [http] Remove legacy codecs (envoyproxy#14381) http2: Add integration tests for METADATA and RST_STREAM frame flood mitigation for upstream servers (envoyproxy#14365) test: start dissolving :printers_include rule. (envoyproxy#14429) integration tests: re-enable set_node_on_first_message_only (envoyproxy#14270) formatter: add a formatter that returns a google::protobuf::Struct rather than a string (envoyproxy#14258) ratelimit: support returning custom response bodies for non-OK responses from the external ratelimit service (envoyproxy#14189) deps: update protobuf to 3.14 (envoyproxy#14253) stream_info: add setResponseCode and update local_reply to take a normal StreamInfo (envoyproxy#14402) http: alpn upstream (envoyproxy#13922) Moved starttls integration test to test/extensions/transport_sockets/starttls. (envoyproxy#14425) generic conn pool: directly use thread local cluster (envoyproxy#14423) wasm: add mathetake to CODEOWNERS (envoyproxy#14427) wasm: clear route cache when modifying HTTP request headers. (envoyproxy#14318) tls: disable TLS inspector injection (envoyproxy#14404) aggregate cluster: cleanups (envoyproxy#14411) Mark starttls_integration_test flaky on Windows (envoyproxy#14419) tcp: improved unit testing (envoyproxy#14415) config: making protocol config explicit (envoyproxy#14362) wasm: dead code (envoyproxy#14407) ... Signed-off-by: Michael Puncel <[email protected]>
Fixes #11120, allows more init managers to watch the SDS init target so clusters/listeners won't mark active immediately.
Additional Description:
Risk Level: Medium
Testing: integration test
Docs Changes: N/A
Release Notes: Added.