[Audit Logging] Xds Audit Logger Registry.

src/core/ext/xds/xds_audit_logger_registry.cc

@@ -66,12 +66,12 @@ Json XdsAuditLoggerRegistry::ConvertXdsAuditLoggerConfig(
   const auto* typed_extension_config =
       envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig_audit_logger(
           logger_config);
-  // It is okay if this is not present.
+  ValidationErrors::ScopedField field(errors, ".audit_logger");
   if (typed_extension_config == nullptr) {
+    errors->AddError("this field is required");
     return Json();  // A null Json object.
   } else {
-    ValidationErrors::ScopedField field(errors,
-                                        ".typed_extension_config.typed_config");
+    ValidationErrors::ScopedField field(errors, ".typed_config");
     const auto* typed_config =
         envoy_config_core_v3_TypedExtensionConfig_typed_config(
             typed_extension_config);
@@ -84,12 +84,18 @@ Json XdsAuditLoggerRegistry::ConvertXdsAuditLoggerConfig(
       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 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();
+    }
   }
-  // TODO(lwge): Check for third-party audit logger type.
   // Add validation error only if the config is not marked optional.
   if (!envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig_is_optional(
           logger_config)) {

src/core/ext/xds/xds_http_rbac_filter.cc

@@ -399,7 +399,7 @@ Json ParseAuditLoggerConfigsToJson(
               audit_logging_options, &size);
   for (size_t i = 0; i < size; ++i) {
     ValidationErrors::ScopedField field(
-        errors, absl::StrCat(".audit_logging_options.logger_configs[", i, "]"));
+        errors, absl::StrCat(".logger_configs[", i, "]"));
     logger_configs_json.emplace_back(registry.ConvertXdsAuditLoggerConfig(
         context, logger_configs[i], errors));
   }
@@ -440,12 +440,23 @@ Json ParseHttpRbacToJson(const XdsResourceType::DecodeContext& context,
     }
     // Flatten the nested messages defined in rbac.proto
     if (envoy_config_rbac_v3_RBAC_has_audit_logging_options(rules)) {
+      ValidationErrors::ScopedField field(errors, ".audit_logging_options");
       const auto* audit_logging_options =
           envoy_config_rbac_v3_RBAC_audit_logging_options(rules);
-      inner_rbac_json.emplace(
-          "audit_condition",
+      int32_t audit_condition =
           envoy_config_rbac_v3_RBAC_AuditLoggingOptions_audit_condition(
-              audit_logging_options));
+              audit_logging_options);
+      switch (audit_condition) {
+        case envoy_config_rbac_v3_RBAC_AuditLoggingOptions_NONE:
+        case envoy_config_rbac_v3_RBAC_AuditLoggingOptions_ON_DENY:
+        case envoy_config_rbac_v3_RBAC_AuditLoggingOptions_ON_ALLOW:
+        case envoy_config_rbac_v3_RBAC_AuditLoggingOptions_ON_DENY_AND_ALLOW:
+          inner_rbac_json.emplace("audit_condition", audit_condition);
+          break;
+        default:
+          ValidationErrors::ScopedField field(errors, ".audit_condition");
+          errors->AddError("invalid audit condition");
+      }
       if (envoy_config_rbac_v3_RBAC_AuditLoggingOptions_has_logger_configs(
               audit_logging_options)) {
         inner_rbac_json.emplace("audit_loggers",

src/proto/grpc/testing/xds/v3/BUILD

@@ -387,9 +387,9 @@ grpc_proto_library(
 
 # Contains stdout audit logger.
 grpc_proto_library(
-    name = "stream_proto",
+    name = "audit_logger_stream_proto",
     srcs = [
-        "stream.proto",
+        "audit_logger_stream.proto",
     ],
 )
 

test/core/xds/BUILD

@@ -137,8 +137,9 @@ grpc_cc_test(
     deps = [
         "//:gpr",
         "//:grpc",
+        "//src/proto/grpc/testing/xds/v3:audit_logger_stream_proto",
         "//src/proto/grpc/testing/xds/v3:rbac_proto",
-        "//src/proto/grpc/testing/xds/v3:stream_proto",
+        "//src/proto/grpc/testing/xds/v3:typed_struct_proto",
         "//test/core/util:grpc_test_util",
         "//test/cpp/util:grpc_cli_utils",
     ],

test/core/xds/xds_audit_logger_registry_test.cc

@@ -26,27 +26,27 @@
 #include "absl/status/statusor.h"
 #include "envoy/config/rbac/v3/rbac.upb.h"
 #include "gtest/gtest.h"
-#include "upb/def.hpp"  // IWYU pragma: keep
+#include "upb/reflection/def.hpp"
 #include "upb/upb.hpp"
 
 #include <grpc/grpc.h>
 
 #include "src/core/ext/xds/xds_bootstrap_grpc.h"
 #include "src/core/lib/json/json_writer.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"
-#include "src/proto/grpc/testing/xds/v3/stream.pb.h"
+#include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h"
 #include "test/core/util/test_config.h"
 
-// IWYU pragma: no_include "upb/reflection/def.hpp"
-
 namespace grpc_core {
 namespace testing {
 namespace {
 
 using AuditLoggerConfigProto =
     ::envoy::config::rbac::v3::RBAC::AuditLoggingOptions::AuditLoggerConfig;
 using ::envoy::extensions::rbac::audit_loggers::stream::v3::StdoutAuditLog;
+using ::xds::type::v3::TypedStruct;
 
 absl::StatusOr<std::string> ConvertAuditLoggerConfig(
     const AuditLoggerConfigProto& config) {
@@ -60,7 +60,7 @@ absl::StatusOr<std::string> ConvertAuditLoggerConfig(
       envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig_parse(
           serialized_config.data(), serialized_config.size(), arena.ptr());
   ValidationErrors errors;
-  ValidationErrors::ScopedField field(&errors, ".logger_config");
+  ValidationErrors::ScopedField field(&errors, ".logger_configs");
   auto config_json = XdsAuditLoggerRegistry().ConvertXdsAuditLoggerConfig(
       context, upb_config, &errors);
   if (!errors.ok()) {
@@ -83,14 +83,36 @@ TEST(StdoutLoggerTest, Basic) {
   EXPECT_EQ(*result, "{\"stdout_logger\":{}}");
 }
 
+//
+// ThirdPartyLoggerTest
+//
+
+TEST(XdsAuditLoggerRegistryTest, ThirdPartyLogger) {
+  AuditLoggerConfigProto config;
+  TypedStruct logger;
+  logger.set_type_url("myorg/foo/bar/test.UnknownAuditLogger");
+  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:logger_configs.audit_logger.typed_config.value"
+            "[xds.type.v3.TypedStruct].value[test.UnknownAuditLogger] "
+            "error:third-party audit logger is not supported]")
+      << result.status();
+}
+
 //
 // XdsAuditLoggerRegistryTest
 //
 
 TEST(XdsAuditLoggerRegistryTest, EmptyAuditLoggerConfig) {
   auto result = ConvertAuditLoggerConfig(AuditLoggerConfigProto());
-  EXPECT_EQ(result.status().code(), absl::StatusCode::kOk);
-  EXPECT_EQ(*result, "null");
+  EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
+  EXPECT_EQ(result.status().message(),
+            "validation errors: [field:logger_configs.audit_logger error:this "
+            "field is required]")
+      << result.status();
 }
 
 TEST(XdsAuditLoggerRegistryTest, NoSupportedType) {
@@ -100,11 +122,22 @@ TEST(XdsAuditLoggerRegistryTest, NoSupportedType) {
   auto result = ConvertAuditLoggerConfig(config);
   EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
   EXPECT_EQ(result.status().message(),
-            "validation errors: [field:logger_config error:unsupported audit "
+            "validation errors: [field:logger_configs.audit_logger "
+            "error:unsupported audit "
             "logger type]")
       << result.status();
 }
 
+TEST(XdsAuditLoggerRegistryTest, NoSupportedTypeButIsOptional) {
+  AuditLoggerConfigProto config;
+  config.mutable_audit_logger()->mutable_typed_config()->PackFrom(
+      AuditLoggerConfigProto());
+  config.set_is_optional(true);
+  auto result = ConvertAuditLoggerConfig(config);
+  EXPECT_EQ(result.status().code(), absl::StatusCode::kOk);
+  EXPECT_EQ(*result, "null");
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc_core

test/core/xds/xds_http_filters_test.cc

@@ -907,6 +907,28 @@ TEST_P(XdsRbacFilterConfigTest, AuditLoggingOptions) {
             "}}");
 }
 
+TEST_P(XdsRbacFilterConfigTest, InvalidAuditCondition) {
+  RBAC rbac;
+  auto* rules = rbac.mutable_rules();
+  rules->set_action(rules->ALLOW);
+  auto* logging_options = rules->mutable_audit_logging_options();
+  logging_options->set_audit_condition(
+      static_cast<
+          envoy::config::rbac::v3::RBAC_AuditLoggingOptions_AuditCondition>(
+          100));
+  auto config = GenerateConfig(rbac);
+  absl::Status status = errors_.status(absl::StatusCode::kInvalidArgument,
+                                       "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.audit_condition "
+                         "error:invalid audit condition]"))
+      << status;
+}
+
 TEST_P(XdsRbacFilterConfigTest, InvalidAuditLoggerConfig) {
   RBAC rbac;
   auto* rules = rbac.mutable_rules();
@@ -922,11 +944,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] "
-                         "error:unsupported audit logger type]"))
+            absl::StrCat(
+                "errors validating filter config: ["
+                "field:",
+                FieldPrefix(),
+                ".rules.audit_logging_options.logger_configs[0].audit_logger "
+                "error:unsupported audit logger type]"))
       << status;
 }