Addressed comments

envoyproxy · Sep 27, 2024 · b360f38 · b360f38
1 parent 78ad7dc
commit b360f38
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 14 deletions.
diff --git a/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto b/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto
@@ -43,7 +43,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // Note that SPIFFE validator inherits and uses the following options from :ref:`CertificateValidationContext <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.CertificateValidationContext>`.
 //
 
-// Example trust_bundle_map json file:
+// Example SPIFFE Trust Bundle Map json file:
 //{
 //  “trust_domains”: {
 //    "example.com": {
@@ -83,8 +83,9 @@ message SPIFFECertValidatorConfig {
   // This field specifies trust domains used for validating incoming X.509-SVID(s).
   repeated TrustDomain trust_domains = 1 [(validate.rules).repeated = {min_items: 1}];
 
-  // This field specifies a trust domain mapping as a json object. Mutually
-  // excluse with trust_domains.
-  config.core.v3.DataSource trust_bundle_map = 2;
+  // This field specifies all trust bundles as a single file. If both
+  // trust_bundles and trust_domains are specified, trust_bundles will
+  // take precedence. Currently assumes file will be a SPIFFE Trust Bundle Map.
+  config.core.v3.DataSource trust_bundles = 2;
 
 }
diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc
@@ -33,7 +33,7 @@ namespace Tls {
 
 using SPIFFEConfig = envoy::extensions::transport_sockets::tls::v3::SPIFFECertValidatorConfig;
 
-std::shared_ptr<SpiffeData> SPIFFEValidator::loadTrustBundleMap() {
+std::shared_ptr<SpiffeData> SPIFFEValidator::loadTrustBundles() {
     std::ifstream file(trust_bundle_file_name_);
     if (file.fail()) {
       ENVOY_LOG(error, "Failed to open SPIFFE bundle map file '{}'", trust_bundle_file_name_);
@@ -152,7 +152,7 @@ void SPIFFEValidator::initializeCertificateRefresh(Server::Configuration::Common
     THROW_IF_NOT_OK(
         file_watcher_->addWatch(trust_bundle_file_name_, Filesystem::Watcher::Events::Modified, [this](uint32_t) {
             ENVOY_LOG(info, "Updating SPIFFE bundle map from file '{}'", trust_bundle_file_name_);
-            if (auto new_trust_bundle = loadTrustBundleMap()) {
+            if (auto new_trust_bundle = loadTrustBundles()) {
                 updateSpiffeDataAsync(new_trust_bundle);
             } else {
                 ENVOY_LOG(error, "Failed to load SPIFFE bundle map from '{}'", trust_bundle_file_name_);
@@ -190,19 +190,18 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC
   }
 
   const auto n_trust_domains = message.trust_domains().size();
-  if (message.has_trust_bundle_map() && n_trust_domains > 0 ) {
-      throw EnvoyException(
-          "Cannot configure both trust_domains and trust_bundle_map...");
-  }
 
   tls_->set([](Event::Dispatcher&) {
     return std::make_shared<ThreadLocalSpiffeState>();
   });
 
   // If a trust bundle map is provided, use that...
-  if (message.has_trust_bundle_map()) {
-    trust_bundle_file_name_ = message.trust_bundle_map().filename();
-    spiffe_data_ = loadTrustBundleMap();
+  if (message.has_trust_bundles()) {
+    if (!message.trust_bundles().has_filename()) {
+      throw EnvoyException("SPIFFE Bundle DataSource requires a filename");
+    }
+    trust_bundle_file_name_ = message.trust_bundles().filename();
+    spiffe_data_ = loadTrustBundles();
     if (!spiffe_data_) {
       throw EnvoyException("Failed to load SPIFFE Bundle map");
     }

diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h
@@ -98,7 +98,7 @@ class SPIFFEValidator : public CertValidator, Logger::Loggable<Logger::Id::secre
                                             std::string& error_details);
 
   void initializeCertificateRefresh(Server::Configuration::CommonFactoryContext& context);
-  std::shared_ptr<SpiffeData> loadTrustBundleMap();
+  std::shared_ptr<SpiffeData> loadTrustBundles();
 
   class ThreadLocalSpiffeState : public Envoy::ThreadLocal::ThreadLocalObject {
   public: