fix: use TextFormat for message-valued CEL attributes (restore pre-1.37 behavior under Protobuf 30+)

changelogs/current.yaml

@@ -143,6 +143,14 @@ bug_fixes:
   change: |
     Fixed a bug to support two ext_proc filters configured in the chain. This change can be reverted by setting
     the runtime guard ``envoy.reloadable_features.ext_proc_inject_data_with_state_update`` to ``false``.
+- area: ext_proc
+  change: |
+    Fixed message-valued CEL attribute serialization (for example
+    ``xds.virtual_host_metadata``) to use protobuf text format instead of debug string output.
+    This restores ext_proc compatibility with protobuf 30+ where debug-string output is
+    intentionally not parseable (for example ``goo.gle/debugonly`` prefixes). This change can
+    be reverted by setting runtime guard
+    ``envoy.reloadable_features.cel_message_serialize_text_format`` to ``false``.
 - area: ext_authz
   change: |
     Fixed a bug where headers from a denied authorization response (non-200) were not properly propagated

source/common/runtime/runtime_features.cc

@@ -35,6 +35,7 @@
 // ASAP by filing a bug on github. Overriding non-buggy code is strongly discouraged to avoid the
 // problem of the bugs being found after the old code path has been removed.
 RUNTIME_GUARD(envoy_reloadable_features_async_host_selection);
+RUNTIME_GUARD(envoy_reloadable_features_cel_message_serialize_text_format);
 RUNTIME_GUARD(envoy_reloadable_features_coalesce_lb_rebuilds_on_batch_update);
 RUNTIME_GUARD(envoy_reloadable_features_codec_client_enable_idle_timer_only_when_connected);
 RUNTIME_GUARD(envoy_reloadable_features_decouple_explicit_drain_pools_and_dns_refresh);

source/extensions/filters/common/expr/evaluator.cc

@@ -303,8 +303,22 @@ std::string print(CelValue value) {
     return std::string(value.StringOrDie().value());
   case CelValue::Type::kBytes:
     return std::string(value.BytesOrDie().value());
-  case CelValue::Type::kMessage:
-    return value.IsNull() ? "NULL" : value.MessageOrDie()->ShortDebugString();
+  case CelValue::Type::kMessage: {
+    if (value.IsNull()) {
+      return "NULL";
+    }
+    if (!Runtime::runtimeFeatureEnabled(
+            "envoy.reloadable_features.cel_message_serialize_text_format")) {
+      return value.MessageOrDie()->ShortDebugString();
+    }
+    std::string textproto;
+    Protobuf::TextFormat::Printer printer;
+    printer.SetSingleLineMode(true);
+    if (printer.PrintToString(*value.MessageOrDie(), &textproto)) {
+      return textproto;
+    }
+    return "";
+  }
   case CelValue::Type::kDuration:
     return absl::FormatDuration(value.DurationOrDie());
   case CelValue::Type::kTimestamp:

test/extensions/filters/common/expr/evaluator_test.cc

@@ -32,8 +32,12 @@ TEST(Evaluator, Print) {
   std::string node_yaml = "id: test";
   TestUtility::loadFromYaml(node_yaml, node);
   EXPECT_EQ(print(CelValue::CreateNull()), "NULL");
-  EXPECT_THAT(print(google::api::expr::runtime::CelProtoWrapper::CreateMessage(&node, &arena)),
-              MatchesRegex(".*id:\\s+\"test\""));
+  const std::string node_textproto =
+      print(google::api::expr::runtime::CelProtoWrapper::CreateMessage(&node, &arena));
+  EXPECT_THAT(node_textproto, ::testing::HasSubstr("id: \"test\""));
+  envoy::config::core::v3::Node parsed;
+  EXPECT_TRUE(Protobuf::TextFormat::ParseFromString(node_textproto, &parsed));
+  EXPECT_EQ(parsed.id(), "test");
 
   EXPECT_EQ(print(CelValue::CreateDuration(absl::Minutes(1))), "1m");
   absl::Time time = TestUtility::parseTime("Dec 22 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT");

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc

@@ -3952,6 +3952,59 @@ TEST_P(ExtProcIntegrationTest, RequestAttributesInResponseOnlyProcessing) {
   verifyDownstreamResponse(*response, 200);
 }
 
+TEST_P(ExtProcIntegrationTest, RequestAttributeVirtualHostMetadataIsTextProto) {
+  proto_config_.mutable_processing_mode()->set_request_header_mode(ProcessingMode::SEND);
+  proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP);
+  proto_config_.mutable_request_attributes()->Add("xds.virtual_host_metadata");
+
+  config_helper_.addConfigModifier([](HttpConnectionManager& cm) {
+    auto* vh = cm.mutable_route_config()->mutable_virtual_hosts()->Mutable(0);
+    auto* metadata = vh->mutable_metadata();
+
+    Protobuf::Struct struct_val;
+    (*struct_val.mutable_fields())["apiIdentifier"].set_string_value("test-api");
+    (*struct_val.mutable_fields())["extHost"].set_string_value("test-host");
+    (*metadata->mutable_filter_metadata())["someKey"] = struct_val;
+  });
+
+  initializeConfig();
+  HttpIntegrationTest::initialize();
+
+  auto response = sendDownstreamRequest(absl::nullopt);
+
+  processGenericMessage(
+      *grpc_upstreams_[0], true,
+      [](const ProcessingRequest& req, ProcessingResponse& resp) -> bool {
+        // Send a valid request-headers response for this request-headers processing step.
+        resp.mutable_request_headers();
+
+        EXPECT_TRUE(req.has_request_headers());
+        EXPECT_EQ(req.attributes().size(), 1);
+        const auto& proto_struct = req.attributes().at("envoy.filters.http.ext_proc");
+        EXPECT_TRUE(proto_struct.fields().contains("xds.virtual_host_metadata"));
+        const auto& metadata_textproto =
+            proto_struct.fields().at("xds.virtual_host_metadata").string_value();
+        envoy::config::core::v3::Metadata parsed_metadata;
+        const bool parsed =
+            Protobuf::TextFormat::ParseFromString(metadata_textproto, &parsed_metadata);
+        EXPECT_TRUE(parsed);
+        EXPECT_TRUE(parsed_metadata.filter_metadata().contains("someKey"));
+        EXPECT_EQ(parsed_metadata.filter_metadata()
+                      .at("someKey")
+                      .fields()
+                      .at("apiIdentifier")
+                      .string_value(),
+                  "test-api");
+        EXPECT_EQ(
+            parsed_metadata.filter_metadata().at("someKey").fields().at("extHost").string_value(),
+            "test-host");
+        return true;
+      });
+
+  handleUpstreamRequest();
+  verifyDownstreamResponse(*response, 200);
+}
+
 TEST_P(ExtProcIntegrationTest, MappedAttributeBuilder) {
   proto_config_.mutable_processing_mode()->set_request_body_mode(ProcessingMode::STREAMED);
   proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SEND);