[Audit Logging] Custom audit logger parsing in xDS registry.

src/core/BUILD

@@ -4067,6 +4067,7 @@ grpc_cc_library(
         "envoy_type_upb",
         "error",
         "google_rpc_status_upb",
+        "grpc_audit_logging",
         "grpc_fake_credentials",
         "grpc_fault_injection_filter",
         "grpc_lb_xds_channel_args",

src/core/ext/xds/xds_audit_logger_registry.cc

@@ -18,34 +18,40 @@
 
 #include "src/core/ext/xds/xds_audit_logger_registry.h"
 
+#include <string>
 #include <utility>
 
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
-#include "absl/types/variant.h"
 #include "envoy/config/core/v3/extension.upb.h"
 #include "envoy/config/rbac/v3/rbac.upb.h"
 
-#include <grpc/support/json.h>
-
 #include "src/core/ext/xds/xds_common_types.h"
+#include "src/core/lib/gprpp/match.h"
 #include "src/core/lib/gprpp/validation_errors.h"
-#include "src/core/lib/json/json.h"
+#include "src/core/lib/security/authorization/audit_logging.h"
 
 namespace grpc_core {
 
 namespace {
 
+using experimental::AuditLoggerRegistry;
+
 class StdoutLoggerConfigFactory : public XdsAuditLoggerRegistry::ConfigFactory {
  public:
   Json::Object ConvertXdsAuditLoggerConfig(
       const XdsResourceType::DecodeContext& /*context*/,
       absl::string_view /*configuration*/,
       ValidationErrors* /*errors*/) override {
-    return Json::Object{{"stdout_logger", Json::FromObject({})}};
+    // Stdout logger has no configuration right now. So we don't process the
+    // config protobuf.
+    return {};
   }
 
   absl::string_view type() override { return Type(); }
+  absl::string_view name() override { return "stdout_logger"; }
 
   static absl::string_view Type() {
     return "envoy.extensions.rbac.audit_loggers.stream.v3.StdoutAuditLog";
@@ -72,38 +78,45 @@ Json XdsAuditLoggerRegistry::ConvertXdsAuditLoggerConfig(
   if (typed_extension_config == nullptr) {
     errors->AddError("field not present");
     return Json();  // A null Json object.
-  } else {
-    ValidationErrors::ScopedField field(errors, ".typed_config");
-    const auto* typed_config =
-        envoy_config_core_v3_TypedExtensionConfig_typed_config(
-            typed_extension_config);
-    auto extension = ExtractXdsExtension(context, typed_config, errors);
-    if (!extension.has_value()) return Json();
-    // Check for registered audit logger type.
-    absl::string_view* serialized_value =
-        absl::get_if<absl::string_view>(&extension->value);
-    if (serialized_value != nullptr) {
-      auto config_factory_it =
-          audit_logger_config_factories_.find(extension->type);
-      if (config_factory_it != audit_logger_config_factories_.end()) {
-        // TODO(lwge): Parse the config with the gRPC audit logger registry.
-        return Json::FromObject(
-            config_factory_it->second->ConvertXdsAuditLoggerConfig(
-                context, *serialized_value, errors));
-      }
-    }
-    // TODO(lwge): Check for third-party audit logger type. For now, we disallow
-    // it by rejecting TypedStruct entries.
-    if (absl::get_if<Json>(&extension->value) != nullptr) {
-      errors->AddError("third-party audit logger is not supported");
-      return Json();
+  }
+  ValidationErrors::ScopedField field2(errors, ".typed_config");
+  const auto* typed_config =
+      envoy_config_core_v3_TypedExtensionConfig_typed_config(
+          typed_extension_config);
+  auto extension = ExtractXdsExtension(context, typed_config, errors);
+  if (!extension.has_value()) return Json();
+  absl::string_view name;
+  Json config;
+  Match(
+      extension->value,
+      // Built-in logger types.
+      [&](absl::string_view serialized_value) {
+        auto it = audit_logger_config_factories_.find(extension->type);
+        if (it == audit_logger_config_factories_.end()) return;
+        name = it->second->name();
+        config = Json::FromObject(it->second->ConvertXdsAuditLoggerConfig(
+            context, serialized_value, errors));
+      },
+      // Custom logger types.
+      [&](Json json) {
+        if (!AuditLoggerRegistry::FactoryExists(extension->type)) return;
+        name = extension->type;
+        config = json;
+      });
+  // Config not found in either case if name is empty.
+  if (name.empty()) {
+    if (!envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig_is_optional(
+            logger_config)) {
+      errors->AddError("unsupported audit logger type");
     }
+    return Json();
   }
-  // Add validation error only if the config is not marked optional.
-  if (!envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig_is_optional(
-          logger_config)) {
-    errors->AddError("unsupported audit logger type");
+  // Validate the converted config.
+  auto result = AuditLoggerRegistry::ParseConfig(name, config);
+  if (!result.ok()) {
+    errors->AddError(result.status().message());
+    return Json();
   }
-  return Json();
+  return Json::FromObject({{std::string(name), std::move(config)}});
 }
 }  // namespace grpc_core

src/core/ext/xds/xds_audit_logger_registry.h

@@ -41,7 +41,10 @@ class XdsAuditLoggerRegistry {
     virtual Json::Object ConvertXdsAuditLoggerConfig(
         const XdsResourceType::DecodeContext& context,
         absl::string_view configuration, ValidationErrors* errors) = 0;
+    // The full proto message name for the logger config.
     virtual absl::string_view type() = 0;
+    // The logger name used for the gRPC registry.
+    virtual absl::string_view name() = 0;
   };
 
   XdsAuditLoggerRegistry();

test/core/xds/BUILD

@@ -136,6 +136,7 @@ grpc_cc_test(
     deps = [
         "//:gpr",
         "//:grpc",
+        "//src/core:grpc_audit_logging",
         "//src/proto/grpc/testing/xds/v3:audit_logger_stream_proto",
         "//src/proto/grpc/testing/xds/v3:rbac_proto",
         "//src/proto/grpc/testing/xds/v3:typed_struct_proto",

test/core/xds/xds_audit_logger_registry_test.cc

@@ -18,21 +18,32 @@
 
 #include "src/core/ext/xds/xds_audit_logger_registry.h"
 
+#include <stdint.h>
+
+#include <initializer_list>
+#include <memory>
 #include <string>
 
 #include <google/protobuf/any.pb.h>
 
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
+#include "absl/strings/str_format.h"
+#include "absl/strings/string_view.h"
 #include "envoy/config/rbac/v3/rbac.upb.h"
+#include "google/protobuf/struct.pb.h"
 #include "gtest/gtest.h"
 #include "upb/reflection/def.hpp"
 #include "upb/upb.hpp"
 
 #include <grpc/grpc.h>
+#include <grpc/grpc_audit_logging.h>
 
 #include "src/core/ext/xds/xds_bootstrap_grpc.h"
+#include "src/core/lib/gprpp/crash.h"
+#include "src/core/lib/json/json.h"
 #include "src/core/lib/json/json_writer.h"
+#include "src/core/lib/security/authorization/audit_logging.h"
 #include "src/proto/grpc/testing/xds/v3/audit_logger_stream.pb.h"
 #include "src/proto/grpc/testing/xds/v3/extension.pb.h"
 #include "src/proto/grpc/testing/xds/v3/rbac.pb.h"
@@ -43,11 +54,16 @@ namespace grpc_core {
 namespace testing {
 namespace {
 
+using experimental::AuditLogger;
+using experimental::AuditLoggerFactory;
+using experimental::AuditLoggerRegistry;
 using AuditLoggerConfigProto =
     ::envoy::config::rbac::v3::RBAC::AuditLoggingOptions::AuditLoggerConfig;
 using ::envoy::extensions::rbac::audit_loggers::stream::v3::StdoutAuditLog;
 using ::xds::type::v3::TypedStruct;
 
+constexpr absl::string_view kName = "test_logger";
+
 absl::StatusOr<std::string> ConvertAuditLoggerConfig(
     const AuditLoggerConfigProto& config) {
   std::string serialized_config = config.SerializeAsString();
@@ -69,11 +85,38 @@ absl::StatusOr<std::string> ConvertAuditLoggerConfig(
   return JsonDump(config_json);
 }
 
+class TestAuditLoggerFactory : public AuditLoggerFactory {
+ public:
+  absl::string_view name() const override { return kName; }
+  absl::StatusOr<std::unique_ptr<AuditLoggerFactory::Config>>
+  ParseAuditLoggerConfig(const Json& json) override {
+    if (json.object().find("bad") != json.object().end()) {
+      return absl::InvalidArgumentError("invalid test_logger config");
+    }
+    return nullptr;
+  }
+  std::unique_ptr<AuditLogger> CreateAuditLogger(
+      std::unique_ptr<AuditLoggerFactory::Config>) override {
+    Crash("unreachable");
+    return nullptr;
+  }
+};
+
+class XdsAuditLoggerRegistryTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+    AuditLoggerRegistry::RegisterFactory(
+        std::make_unique<TestAuditLoggerFactory>());
+  }
+
+  void TearDown() override { AuditLoggerRegistry::TestOnlyResetRegistry(); }
+};
+
 //
 // StdoutLoggerTest
 //
 
-TEST(StdoutLoggerTest, Basic) {
+TEST(StdoutLoggerTest, BasicStdoutLogger) {
   AuditLoggerConfigProto config;
   config.mutable_audit_logger()->mutable_typed_config()->PackFrom(
       StdoutAuditLog());
@@ -86,34 +129,48 @@ TEST(StdoutLoggerTest, Basic) {
 // ThirdPartyLoggerTest
 //
 
-TEST(XdsAuditLoggerRegistryTest, ThirdPartyLogger) {
+TEST_F(XdsAuditLoggerRegistryTest, ValidThirdPartyLogger) {
   AuditLoggerConfigProto config;
   TypedStruct logger;
-  logger.set_type_url("myorg/foo/bar/test.UnknownAuditLogger");
+  logger.set_type_url(absl::StrFormat("myorg/foo/bar/%s", kName));
+  auto* fields = logger.mutable_value()->mutable_fields();
+  (*fields)["foo"].set_string_value("bar");
+  config.mutable_audit_logger()->mutable_typed_config()->PackFrom(logger);
+  auto result = ConvertAuditLoggerConfig(config);
+  ASSERT_TRUE(result.ok()) << result.status();
+  EXPECT_EQ(*result, "{\"test_logger\":{\"foo\":\"bar\"}}");
+}
+
+TEST_F(XdsAuditLoggerRegistryTest, InvalidThirdPartyLoggerConfig) {
+  AuditLoggerConfigProto config;
+  TypedStruct logger;
+  logger.set_type_url(absl::StrFormat("myorg/foo/bar/%s", kName));
+  auto* fields = logger.mutable_value()->mutable_fields();
+  (*fields)["bad"].set_string_value("true");
   config.mutable_audit_logger()->mutable_typed_config()->PackFrom(logger);
   auto result = ConvertAuditLoggerConfig(config);
   EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
   EXPECT_EQ(result.status().message(),
             "validation errors: "
             "[field:audit_logger.typed_config.value"
-            "[xds.type.v3.TypedStruct].value[test.UnknownAuditLogger] "
-            "error:third-party audit logger is not supported]")
+            "[xds.type.v3.TypedStruct].value[test_logger] "
+            "error:invalid test_logger config]")
       << result.status();
 }
 
 //
 // XdsAuditLoggerRegistryTest
 //
 
-TEST(XdsAuditLoggerRegistryTest, EmptyAuditLoggerConfig) {
+TEST_F(XdsAuditLoggerRegistryTest, EmptyAuditLoggerConfig) {
   auto result = ConvertAuditLoggerConfig(AuditLoggerConfigProto());
   EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
   EXPECT_EQ(result.status().message(),
             "validation errors: [field:audit_logger error:field not present]")
       << result.status();
 }
 
-TEST(XdsAuditLoggerRegistryTest, MissingTypedConfig) {
+TEST_F(XdsAuditLoggerRegistryTest, MissingTypedConfig) {
   AuditLoggerConfigProto config;
   config.mutable_audit_logger();
   auto result = ConvertAuditLoggerConfig(config);
@@ -124,19 +181,21 @@ TEST(XdsAuditLoggerRegistryTest, MissingTypedConfig) {
       << result.status();
 }
 
-TEST(XdsAuditLoggerRegistryTest, NoSupportedType) {
+TEST_F(XdsAuditLoggerRegistryTest, NoSupportedType) {
   AuditLoggerConfigProto config;
   config.mutable_audit_logger()->mutable_typed_config()->PackFrom(
       AuditLoggerConfigProto());
   auto result = ConvertAuditLoggerConfig(config);
   EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
   EXPECT_EQ(result.status().message(),
-            "validation errors: [field:audit_logger error:unsupported audit "
+            "validation errors: "
+            "[field:audit_logger.typed_config.value[envoy.config.rbac.v3.RBAC."
+            "AuditLoggingOptions.AuditLoggerConfig] error:unsupported audit "
             "logger type]")
       << result.status();
 }
 
-TEST(XdsAuditLoggerRegistryTest, NoSupportedTypeButIsOptional) {
+TEST_F(XdsAuditLoggerRegistryTest, NoSupportedTypeButIsOptional) {
   AuditLoggerConfigProto config;
   config.mutable_audit_logger()->mutable_typed_config()->PackFrom(
       AuditLoggerConfigProto());

test/core/xds/xds_http_filters_test.cc

@@ -987,12 +987,12 @@ TEST_P(XdsRbacFilterConfigTest, InvalidAuditLoggerConfig) {
                                        "errors validating filter config");
   EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
   EXPECT_EQ(status.message(),
-            absl::StrCat(
-                "errors validating filter config: ["
-                "field:",
-                FieldPrefix(),
-                ".rules.audit_logging_options.logger_configs[0].audit_logger "
-                "error:unsupported audit logger type]"))
+            absl::StrCat("errors validating filter config: ["
+                         "field:",
+                         FieldPrefix(),
+                         ".rules.audit_logging_options.logger_configs[0].audit_"
+                         "logger.typed_config.value[foo_logger] "
+                         "error:unsupported audit logger type]"))
       << status;
 }