diff --git a/envoy/secret/secret_manager.h b/envoy/secret/secret_manager.h index 80713398a3f8..6bac464b5ce3 100644 --- a/envoy/secret/secret_manager.h +++ b/envoy/secret/secret_manager.h @@ -25,9 +25,9 @@ class SecretManager { /** * @param add a static secret from envoy::extensions::transport_sockets::tls::v3::Secret. - * @throw an EnvoyException if the secret is invalid or not supported, or there is duplicate. + * @return a status indicating if the function completed successfully. */ - virtual void + virtual absl::Status addStaticSecret(const envoy::extensions::transport_sockets::tls::v3::Secret& secret) PURE; /** diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 717f5a2555b2..caa5619bc720 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -26,7 +26,7 @@ SecretManagerImpl::SecretManagerImpl(OptRef config_tracke } } -void SecretManagerImpl::addStaticSecret( +absl::Status SecretManagerImpl::addStaticSecret( const envoy::extensions::transport_sockets::tls::v3::Secret& secret) { switch (secret.type_case()) { case envoy::extensions::transport_sockets::tls::v3::Secret::TypeCase::kTlsCertificate: { @@ -34,7 +34,7 @@ void SecretManagerImpl::addStaticSecret( std::make_shared(secret.tls_certificate()); if (!static_tls_certificate_providers_.insert(std::make_pair(secret.name(), secret_provider)) .second) { - throwEnvoyExceptionOrPanic( + return absl::InvalidArgumentError( absl::StrCat("Duplicate static TlsCertificate secret name ", secret.name())); } break; @@ -45,7 +45,7 @@ void SecretManagerImpl::addStaticSecret( if (!static_certificate_validation_context_providers_ .insert(std::make_pair(secret.name(), secret_provider)) .second) { - throwEnvoyExceptionOrPanic(absl::StrCat( + return absl::InvalidArgumentError(absl::StrCat( "Duplicate static CertificateValidationContext secret name ", secret.name())); } break; @@ -56,7 +56,7 @@ void SecretManagerImpl::addStaticSecret( if (!static_session_ticket_keys_providers_ .insert(std::make_pair(secret.name(), secret_provider)) .second) { - throwEnvoyExceptionOrPanic( + return absl::InvalidArgumentError( absl::StrCat("Duplicate static TlsSessionTicketKeys secret name ", secret.name())); } break; @@ -66,14 +66,15 @@ void SecretManagerImpl::addStaticSecret( std::make_shared(secret.generic_secret()); if (!static_generic_secret_providers_.insert(std::make_pair(secret.name(), secret_provider)) .second) { - throwEnvoyExceptionOrPanic( + return absl::InvalidArgumentError( absl::StrCat("Duplicate static GenericSecret secret name ", secret.name())); } break; } default: - throwEnvoyExceptionOrPanic("Secret type not implemented"); + return absl::InvalidArgumentError("Secret type not implemented"); } + return absl::OkStatus(); } TlsCertificateConfigProviderSharedPtr diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index 952206f106b7..f4f780ac8d1b 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -19,7 +19,7 @@ namespace Secret { class SecretManagerImpl : public SecretManager { public: SecretManagerImpl(OptRef config_tracker); - void + absl::Status addStaticSecret(const envoy::extensions::transport_sockets::tls::v3::Secret& secret) override; TlsCertificateConfigProviderSharedPtr diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 585d92ba903d..80818215be7b 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -119,7 +119,7 @@ void MainImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bootstr ENVOY_LOG(info, "loading {} static secret(s)", secrets.size()); for (ssize_t i = 0; i < secrets.size(); i++) { ENVOY_LOG(debug, "static secret #{}: {}", i, secrets[i].name()); - server.secretManager().addStaticSecret(secrets[i]); + THROW_IF_NOT_OK(server.secretManager().addStaticSecret(secrets[i])); } ENVOY_LOG(info, "loading {} cluster(s)", bootstrap.static_resources().clusters().size()); diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index b6335026065e..cbd8e21277aa 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -77,7 +77,7 @@ name: "abc.com" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); SecretManagerPtr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_EQ(secret_manager->findStaticTlsCertificateProvider("undefined"), nullptr); ASSERT_NE(secret_manager->findStaticTlsCertificateProvider("abc.com"), nullptr); @@ -111,11 +111,11 @@ TEST_F(SecretManagerImplTest, DuplicateStaticTlsCertificateSecret) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); SecretManagerPtr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_NE(secret_manager->findStaticTlsCertificateProvider("abc.com"), nullptr); - EXPECT_THROW_WITH_MESSAGE(secret_manager->addStaticSecret(secret_config), EnvoyException, - "Duplicate static TlsCertificate secret name abc.com"); + EXPECT_EQ(secret_manager->addStaticSecret(secret_config).message(), + "Duplicate static TlsCertificate secret name abc.com"); } // Validate that secret manager adds static certificate validation context secret successfully. @@ -130,7 +130,7 @@ TEST_F(SecretManagerImplTest, CertificateValidationContextSecretLoadSuccess) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_EQ(secret_manager->findStaticCertificateValidationContextProvider("undefined"), nullptr); ASSERT_NE(secret_manager->findStaticCertificateValidationContextProvider("abc.com"), nullptr); @@ -158,11 +158,11 @@ TEST_F(SecretManagerImplTest, DuplicateStaticCertificateValidationContextSecret) )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); SecretManagerPtr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_NE(secret_manager->findStaticCertificateValidationContextProvider("abc.com"), nullptr); - EXPECT_THROW_WITH_MESSAGE(secret_manager->addStaticSecret(secret_config), EnvoyException, - "Duplicate static CertificateValidationContext secret name abc.com"); + EXPECT_EQ(secret_manager->addStaticSecret(secret_config).message(), + "Duplicate static CertificateValidationContext secret name abc.com"); } // Validate that secret manager adds static STKs secret successfully. @@ -181,7 +181,7 @@ name: "abc.com" SecretManagerPtr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_EQ(secret_manager->findStaticTlsSessionTicketKeysContextProvider("undefined"), nullptr); ASSERT_NE(secret_manager->findStaticTlsSessionTicketKeysContextProvider("abc.com"), nullptr); @@ -210,11 +210,11 @@ name: "abc.com" SecretManagerPtr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_NE(secret_manager->findStaticTlsSessionTicketKeysContextProvider("abc.com"), nullptr); - EXPECT_THROW_WITH_MESSAGE(secret_manager->addStaticSecret(secret_config), EnvoyException, - "Duplicate static TlsSessionTicketKeys secret name abc.com"); + EXPECT_EQ(secret_manager->addStaticSecret(secret_config).message(), + "Duplicate static TlsSessionTicketKeys secret name abc.com"); } // Validate that secret manager adds static generic secret successfully. @@ -230,7 +230,7 @@ name: "encryption_key" filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/aes_128_key" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret); - secret_manager->addStaticSecret(secret); + EXPECT_TRUE(secret_manager->addStaticSecret(secret).ok()); ASSERT_EQ(secret_manager->findStaticGenericSecretProvider("undefined"), nullptr); ASSERT_NE(secret_manager->findStaticGenericSecretProvider("encryption_key"), nullptr); @@ -255,11 +255,11 @@ name: "encryption_key" filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/aes_128_key" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret); - secret_manager->addStaticSecret(secret); + EXPECT_TRUE(secret_manager->addStaticSecret(secret).ok()); ASSERT_NE(secret_manager->findStaticGenericSecretProvider("encryption_key"), nullptr); - EXPECT_THROW_WITH_MESSAGE(secret_manager->addStaticSecret(secret), EnvoyException, - "Duplicate static GenericSecret secret name encryption_key"); + EXPECT_EQ(secret_manager->addStaticSecret(secret).message(), + "Duplicate static GenericSecret secret name encryption_key"); } // Validate that secret manager deduplicates dynamic TLS certificate secret provider. @@ -900,7 +900,7 @@ name: "abc.com" )EOF"; envoy::extensions::transport_sockets::tls::v3::Secret tls_cert_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate), tls_cert_secret); - secret_manager->addStaticSecret(tls_cert_secret); + EXPECT_TRUE(secret_manager->addStaticSecret(tls_cert_secret).ok()); TestUtility::loadFromYaml(TestEnvironment::substitute(R"EOF( name: "abc.com.nopassword" tls_certificate: @@ -910,7 +910,7 @@ name: "abc.com.nopassword" inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" )EOF"), tls_cert_secret); - secret_manager->addStaticSecret(tls_cert_secret); + EXPECT_TRUE(secret_manager->addStaticSecret(tls_cert_secret).ok()); const std::string expected_config_dump = R"EOF( static_secrets: - name: "abc.com.nopassword" @@ -972,7 +972,7 @@ name: "abc.com.validation" )EOF"; envoy::extensions::transport_sockets::tls::v3::Secret validation_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(validation_context), validation_secret); - secret_manager->addStaticSecret(validation_secret); + EXPECT_TRUE(secret_manager->addStaticSecret(validation_secret).ok()); const std::string expected_config_dump = R"EOF( static_secrets: - name: "abc.com.validation" @@ -1021,7 +1021,7 @@ name: "abc.com.stek" )EOF"; envoy::extensions::transport_sockets::tls::v3::Secret stek_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(stek_context), stek_secret); - secret_manager->addStaticSecret(stek_secret); + EXPECT_TRUE(secret_manager->addStaticSecret(stek_secret).ok()); const std::string expected_config_dump = R"EOF( static_secrets: - name: "abc.com.stek" @@ -1051,7 +1051,7 @@ name: "signing_key" )EOF"; envoy::extensions::transport_sockets::tls::v3::Secret typed_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), typed_secret); - secret_manager->addStaticSecret(typed_secret); + EXPECT_TRUE(secret_manager->addStaticSecret(typed_secret).ok()); const std::string expected_config_dump = R"EOF( static_secrets: @@ -1147,7 +1147,7 @@ TEST_F(SecretManagerImplTest, DeprecatedSanMatcher) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); - secret_manager->addStaticSecret(secret_config); + EXPECT_TRUE(secret_manager->addStaticSecret(secret_config).ok()); ASSERT_EQ(secret_manager->findStaticCertificateValidationContextProvider("undefined"), nullptr); ASSERT_NE(secret_manager->findStaticCertificateValidationContextProvider("abc.com"), nullptr); diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index 945b8997c9fd..11a441051229 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -989,7 +989,7 @@ name: "abc.com" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; envoy::extensions::transport_sockets::tls::v3::TlsCertificate* server_cert = @@ -1478,7 +1478,7 @@ name: "abc.com" ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); const std::string cert_pem = @@ -1514,7 +1514,7 @@ TEST_F(ClientContextConfigImplTest, PasswordProtectedTlsCertificates) { ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); const std::string cert_pem = @@ -1554,7 +1554,7 @@ TEST_F(ClientContextConfigImplTest, PasswordProtectedPkcs12) { ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); const std::string cert_p12 = @@ -1588,7 +1588,7 @@ TEST_F(ClientContextConfigImplTest, PasswordWrongPkcs12) { ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); Stats::IsolatedStoreImpl store; @@ -1616,7 +1616,7 @@ TEST_F(ClientContextConfigImplTest, PasswordNotSuppliedPkcs12) { ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); Stats::IsolatedStoreImpl store; @@ -1647,7 +1647,7 @@ TEST_F(ClientContextConfigImplTest, PasswordNotSuppliedTlsCertificates) { ->Add() ->set_name("abc.com"); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); ClientContextConfigImpl client_context_config(tls_context, factory_context_); Stats::IsolatedStoreImpl store; @@ -1670,7 +1670,7 @@ TEST_F(ClientContextConfigImplTest, StaticCertificateValidationContext) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), tls_certificate_secret_config); - factory_context_.secretManager().addStaticSecret(tls_certificate_secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(tls_certificate_secret_config).ok()); envoy::extensions::transport_sockets::tls::v3::Secret certificate_validation_context_secret_config; const std::string certificate_validation_context_yaml = R"EOF( @@ -1681,7 +1681,9 @@ TEST_F(ClientContextConfigImplTest, StaticCertificateValidationContext) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(certificate_validation_context_yaml), certificate_validation_context_secret_config); - factory_context_.secretManager().addStaticSecret(certificate_validation_context_secret_config); + EXPECT_TRUE(factory_context_.secretManager() + .addStaticSecret(certificate_validation_context_secret_config) + .ok()); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; tls_context.mutable_common_tls_context() @@ -1715,7 +1717,7 @@ name: "abc.com" TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - factory_context_.secretManager().addStaticSecret(secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(secret_config).ok()); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; tls_context.mutable_common_tls_context() @@ -1742,7 +1744,7 @@ TEST_F(ClientContextConfigImplTest, MissingStaticCertificateValidationContext) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml), tls_certificate_secret_config); - factory_context_.secretManager().addStaticSecret(tls_certificate_secret_config); + EXPECT_TRUE(factory_context_.secretManager().addStaticSecret(tls_certificate_secret_config).ok()); envoy::extensions::transport_sockets::tls::v3::Secret certificate_validation_context_secret_config; const std::string certificate_validation_context_yaml = R"EOF( @@ -1753,7 +1755,9 @@ TEST_F(ClientContextConfigImplTest, MissingStaticCertificateValidationContext) { )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(certificate_validation_context_yaml), certificate_validation_context_secret_config); - factory_context_.secretManager().addStaticSecret(certificate_validation_context_secret_config); + EXPECT_TRUE(factory_context_.secretManager() + .addStaticSecret(certificate_validation_context_secret_config) + .ok()); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; tls_context.mutable_common_tls_context() diff --git a/test/mocks/secret/mocks.h b/test/mocks/secret/mocks.h index 91c431719610..e87cf0a95cee 100644 --- a/test/mocks/secret/mocks.h +++ b/test/mocks/secret/mocks.h @@ -18,7 +18,7 @@ class MockSecretManager : public SecretManager { MockSecretManager(); ~MockSecretManager() override; - MOCK_METHOD(void, addStaticSecret, + MOCK_METHOD(absl::Status, addStaticSecret, (const envoy::extensions::transport_sockets::tls::v3::Secret& secret)); MOCK_METHOD(TlsCertificateConfigProviderSharedPtr, findStaticTlsCertificateProvider, (const std::string& name), (const));