Revert "dynamic_modules: add gRPC capability to HTTP module (#43827)" (#43897)

This reverts commit 559dfff.

We have a consensus on moving these to a utility instead.

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>

Author: agrawroh

changelogs/current.yaml

@@ -262,11 +262,6 @@ new_features:
     and added brotli and zlib to TCP TLS. Controlled by runtime flag
     ``envoy.reloadable_features.tls_certificate_compression_brotli`` (defaults to ``true``).
     When disabled, QUIC retains zlib-only compression, while TCP TLS has no compression.
-- area: dynamic_modules
-  change: |
-    Added gRPC status support for the HTTP dynamic module filter SDKs. The ``send_response`` SDK
-    methods now accept an optional gRPC status code which is passed as a ``grpc-status`` header,
-    and the ``ResponseGrpcStatus`` attribute is now available to read gRPC status from responses.
 - area: dynamic_modules
   change: |
     Added custom metrics (counters, gauges, histograms) support to load balancer dynamic modules.

source/extensions/dynamic_modules/sdk/cpp/sdk.h

@@ -470,15 +470,11 @@ class HttpFilterHandle {
   /**
    * Sends a local response with status code, body, and detail.
    * @param status The HTTP status code.
-   * @param headers The response headers.
    * @param body The response body.
-   * @param grpc_status The gRPC status code for gRPC requests. Use -1 to indicate no gRPC status
-   * (Envoy will infer from the HTTP status code). Values 0-16 are standard gRPC status codes.
    * @param detail The response detail.
    */
   virtual void sendLocalResponse(uint32_t status, std::span<const HeaderView> headers,
-                                 std::string_view body, int32_t grpc_status,
-                                 std::string_view detail) = 0;
+                                 std::string_view body, std::string_view detail) = 0;
 
   /**
    * Sends response headers. This is used for streaming local replies.

source/extensions/dynamic_modules/sdk/cpp/sdk_internal.cc

@@ -384,26 +384,12 @@ class HttpFilterHandleImpl : public HttpFilterHandle {
   }
 
   void sendLocalResponse(uint32_t status, std::span<const HeaderView> headers,
-                         std::string_view body, int32_t grpc_status,
-                         std::string_view detail) override {
-    // When gRPC status is specified, include it as a grpc-status header so Envoy's sendLocalReply
-    // picks it up via modify_headers without requiring an ABI change.
-    std::string grpc_status_str;
-    std::vector<HeaderView> merged_headers;
-    const HeaderView* headers_ptr = headers.data();
-    size_t headers_size = headers.size();
-    if (grpc_status >= 0) {
-      grpc_status_str = std::to_string(grpc_status);
-      merged_headers.assign(headers.begin(), headers.end());
-      merged_headers.emplace_back("grpc-status", grpc_status_str);
-      headers_ptr = merged_headers.data();
-      headers_size = merged_headers.size();
-    }
+                         std::string_view body, std::string_view detail) override {
     envoy_dynamic_module_callback_http_send_response(
         host_plugin_ptr_, status,
         const_cast<envoy_dynamic_module_type_module_http_header*>(
-            reinterpret_cast<const envoy_dynamic_module_type_module_http_header*>(headers_ptr)),
-        headers_size, envoy_dynamic_module_type_module_buffer{body.data(), body.size()},
+            reinterpret_cast<const envoy_dynamic_module_type_module_http_header*>(headers.data())),
+        headers.size(), envoy_dynamic_module_type_module_buffer{body.data(), body.size()},
         envoy_dynamic_module_type_module_buffer{detail.data(), detail.size()});
   }
 

source/extensions/dynamic_modules/sdk/go/abi/internal.go

@@ -720,17 +720,11 @@ func (h *dymHttpFilterHandle) SendLocalResponse(
 	statusCode uint32,
 	headers [][2]string,
 	body []byte,
-	grpcStatus int32,
 	detail string,
 ) {
 	h.localResponseSent = true
 
-	// When gRPC status is specified, include it as a grpc-status header so Envoy's sendLocalReply
-	// picks it up via modify_headers without requiring an ABI change.
-	if grpcStatus >= 0 {
-		headers = append(headers, [2]string{"grpc-status", strconv.Itoa(int(grpcStatus))})
-	}
-
+	// Prepare headers.
 	headerViews := headersToModuleHttpHeaderSlice(headers)
 	C.envoy_dynamic_module_callback_http_send_response(
 		h.hostPluginPtr,

source/extensions/dynamic_modules/sdk/go/shared/base.go

@@ -414,10 +414,8 @@ type HttpFilterHandle interface {
 	// @Param status the HTTP status code.
 	// @Param headers the response headers.
 	// @Param body the response body.
-	// @Param grpcStatus the gRPC status code for gRPC requests. Use -1 to indicate no gRPC status
-	// (Envoy will infer from the HTTP status code). Values 0-16 are standard gRPC status codes.
 	// @Param detail a short description to the response for debugging purposes.
-	SendLocalResponse(status uint32, headers [][2]string, body []byte, grpcStatus int32, detail string)
+	SendLocalResponse(status uint32, headers [][2]string, body []byte, detail string)
 
 	// SendResponseHeaders sends response headers to the client. This is used for
 	// streaming local replies.

source/extensions/dynamic_modules/sdk/go/shared/mocks/mock_base.go

@@ -846,16 +846,11 @@ pub trait EnvoyHttpFilter {
   /// Send a response to the downstream with the given status code, headers, and body.
   ///
   /// The headers are passed as a list of key-value pairs.
-  ///
-  /// The `grpc_status` parameter is the gRPC status code for gRPC requests. Use `None` to indicate
-  /// no gRPC status (Envoy will infer from the HTTP status code). Values 0-16 are standard gRPC
-  /// status codes. This is only meaningful when the downstream request is a gRPC request.
   fn send_response<'a>(
     &mut self,
     status_code: u32,
     headers: &'a [(&'a str, &'a [u8])],
     body: Option<&'a [u8]>,
-    grpc_status: Option<i32>,
     details: Option<&'a str>,
   );
 
@@ -1221,14 +1216,6 @@ pub trait EnvoyHttpFilter {
     attribute_id: abi::envoy_dynamic_module_type_attribute_id,
   ) -> Option<bool>;
 
-  /// Get the gRPC status code from the response.
-  ///
-  /// This checks response trailers, response headers, and infers from the HTTP status code.
-  /// Returns `None` if no gRPC status is available (e.g., the response is not a gRPC response).
-  fn get_response_grpc_status(&self) -> Option<i64> {
-    self.get_attribute_int(abi::envoy_dynamic_module_type_attribute_id::ResponseGrpcStatus)
-  }
-
   /// Send an HTTP callout to the given cluster with the given headers and body.
   /// Multiple callouts can be made from the same filter. Different callouts can be
   /// distinguished by the returned callout id.
@@ -2140,37 +2127,21 @@ impl EnvoyHttpFilter for EnvoyHttpFilterImpl {
     status_code: u32,
     headers: &[(&str, &[u8])],
     body: Option<&[u8]>,
-    grpc_status: Option<i32>,
     details: Option<&str>,
   ) {
     let body_ptr = body.map(|s| s.as_ptr()).unwrap_or(std::ptr::null());
     let body_length = body.map(|s| s.len()).unwrap_or(0);
     let details_ptr = details.map(|s| s.as_ptr()).unwrap_or(std::ptr::null());
     let details_length = details.map(|s| s.len()).unwrap_or(0);
 
-    // When gRPC status is specified, include it as a grpc-status header so Envoy's sendLocalReply
-    // picks it up via modify_headers without requiring an ABI change.
-    let grpc_status_str;
-    let merged_headers;
-    let (final_headers_ptr, final_headers_len) = if let Some(status) = grpc_status {
-      grpc_status_str = status.to_string();
-      let mut h: Vec<(&str, &[u8])> = Vec::with_capacity(headers.len() + 1);
-      h.extend_from_slice(headers);
-      h.push(("grpc-status", grpc_status_str.as_bytes()));
-      merged_headers = h;
-      let HeaderPairSlice(ptr, len) = merged_headers.as_slice().into();
-      (ptr, len)
-    } else {
-      let HeaderPairSlice(ptr, len) = headers.into();
-      (ptr, len)
-    };
+    let HeaderPairSlice(headers_ptr, headers_len) = headers.into();
 
     unsafe {
       abi::envoy_dynamic_module_callback_http_send_response(
         self.raw_ptr,
         status_code,
-        final_headers_ptr as *mut _,
-        final_headers_len,
+        headers_ptr as *mut _,
+        headers_len,
         abi::envoy_dynamic_module_type_module_buffer {
           ptr: body_ptr as *mut _,
           length: body_length,

source/extensions/dynamic_modules/sdk/rust/src/http.rs

@@ -58,7 +58,6 @@ envoy_cc_library(
     deps = [
         ":filter_lib",
         "//envoy/registry",
-        "//source/common/grpc:common_lib",
         "//source/common/http:utility_lib",
         "//source/common/network:upstream_socket_options_filter_state_lib",
         "//source/common/router:string_accessor_lib",

source/extensions/filters/http/dynamic_modules/BUILD

@@ -5,7 +5,6 @@
 #include "envoy/config/core/v3/socket_option.pb.h"
 #include "envoy/registry/registry.h"
 
-#include "source/common/grpc/common.h"
 #include "source/common/http/header_map_impl.h"
 #include "source/common/http/message_impl.h"
 #include "source/common/http/utility.h"
@@ -666,8 +665,8 @@ void envoy_dynamic_module_callback_http_send_response(
     details_view = "dynamic_module";
   }
 
-  filter->sendLocalReply(static_cast<Http::Code>(status_code), body_view, modify_headers,
-                         absl::nullopt, details_view);
+  filter->sendLocalReply(static_cast<Http::Code>(status_code), body_view, modify_headers, 0,
+                         details_view);
 }
 
 void envoy_dynamic_module_callback_http_send_response_headers(
@@ -1426,22 +1425,6 @@ bool envoy_dynamic_module_callback_http_filter_get_attribute_int(
     }
     break;
   }
-  case envoy_dynamic_module_type_attribute_id_ResponseGrpcStatus: {
-    auto* stream_info = filter->streamInfo();
-    if (stream_info) {
-      auto trailers = filter->responseTrailers();
-      auto headers = filter->responseHeaders();
-      auto grpc_status = Grpc::Common::getGrpcStatus(
-          trailers.has_value() ? *trailers : *Http::StaticEmptyHeaders::get().response_trailers,
-          headers.has_value() ? *headers : *Http::StaticEmptyHeaders::get().response_headers,
-          *stream_info);
-      if (grpc_status.has_value()) {
-        *result = grpc_status.value();
-        ok = true;
-      }
-    }
-    break;
-  }
   default:
     ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::dynamic_modules), error,
                         "Unsupported attribute ID {} as int", static_cast<int64_t>(attribute_id));

source/extensions/filters/http/dynamic_modules/abi_impl.cc

@@ -403,9 +403,8 @@ TEST_P(DynamicModuleHttpFilterHeaderTest, GetHeaders) {
 }
 
 TEST_F(DynamicModuleHttpFilterTest, SendResponseNullptr) {
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _, testing::Eq(absl::nullopt),
-                             testing::Eq("dynamic_module")));
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _,
+                                                 testing::Eq(0), testing::Eq("dynamic_module")));
   envoy_dynamic_module_callback_http_send_response(filter_.get(), 200, nullptr, 0, {nullptr, 0},
                                                    {nullptr, 0});
 }
@@ -416,9 +415,8 @@ TEST_F(DynamicModuleHttpFilterTest, SendResponseEmptyResponse) {
       .WillRepeatedly(testing::Return(makeOptRef<ResponseHeaderMap>(response_headers)));
 
   // Test with empty response.
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _, testing::Eq(absl::nullopt),
-                             testing::Eq("dynamic_module")));
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _,
+                                                 testing::Eq(0), testing::Eq("dynamic_module")));
   EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _));
 
   envoy_dynamic_module_callback_http_send_response(filter_.get(), 200, nullptr, 0, {nullptr, 0},
@@ -440,9 +438,8 @@ TEST_F(DynamicModuleHttpFilterTest, SendResponse) {
     header_array[index].value_ptr = const_cast<char*>(value.c_str());
     ++index;
   }
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _, testing::Eq(absl::nullopt),
-                             testing::Eq("dynamic_module")));
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Envoy::Http::Code::OK, testing::Eq(""), _,
+                                                 testing::Eq(0), testing::Eq("dynamic_module")));
   EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).WillOnce(Invoke([](auto& headers, auto) {
     EXPECT_EQ(headers.get(Http::LowerCaseString("single"))[0]->value().getStringView(), "value");
     EXPECT_EQ(headers.get(Http::LowerCaseString("multi"))[0]->value().getStringView(), "value1");
@@ -470,9 +467,8 @@ TEST_F(DynamicModuleHttpFilterTest, SendResponseWithBody) {
   }
 
   const std::string body_str = "body";
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::OK, testing::Eq("body"), _,
-                             testing::Eq(absl::nullopt), testing::Eq("dynamic_module")));
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Envoy::Http::Code::OK, testing::Eq("body"), _,
+                                                 testing::Eq(0), testing::Eq("dynamic_module")));
   EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)).WillOnce(Invoke([](auto& headers, auto) {
     EXPECT_EQ(headers.get(Http::LowerCaseString("single"))[0]->value().getStringView(), "value");
     EXPECT_EQ(headers.get(Http::LowerCaseString("multi"))[0]->value().getStringView(), "value1");
@@ -485,41 +481,13 @@ TEST_F(DynamicModuleHttpFilterTest, SendResponseWithBody) {
 TEST_F(DynamicModuleHttpFilterTest, SendResponseWithCustomResponseCodeDetails) {
   const std::string body_str = "body";
   absl::string_view test_details = "test_details";
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::OK, testing::Eq("body"), _,
-                             testing::Eq(absl::nullopt), testing::Eq("test_details")));
+  EXPECT_CALL(decoder_callbacks_, sendLocalReply(Envoy::Http::Code::OK, testing::Eq("body"), _,
+                                                 testing::Eq(0), testing::Eq("test_details")));
   envoy_dynamic_module_callback_http_send_response(filter_.get(), 200, nullptr, 0,
                                                    {body_str.data(), body_str.size()},
                                                    {test_details.data(), test_details.size()});
 }
 
-TEST_F(DynamicModuleHttpFilterTest, SendResponseWithGrpcStatusHeader) {
-  // Verify that grpc-status passed as a header is forwarded via modify_headers.
-  std::string grpc_status_key = "grpc-status";
-  std::string grpc_status_value = "14";
-  envoy_dynamic_module_type_module_http_header header_array[1];
-  header_array[0].key_ptr = grpc_status_key.data();
-  header_array[0].key_length = grpc_status_key.size();
-  header_array[0].value_ptr = grpc_status_value.data();
-  header_array[0].value_length = grpc_status_value.size();
-
-  EXPECT_CALL(decoder_callbacks_,
-              sendLocalReply(Envoy::Http::Code::ServiceUnavailable, testing::Eq(""), _,
-                             testing::Eq(absl::nullopt), testing::Eq("dynamic_module")))
-      .WillOnce(Invoke([](Http::Code, absl::string_view,
-                          std::function<void(Http::ResponseHeaderMap & headers)> modify_headers,
-                          absl::optional<Grpc::Status::GrpcStatus>, absl::string_view) {
-        Http::TestResponseHeaderMapImpl headers;
-        if (modify_headers) {
-          modify_headers(headers);
-        }
-        EXPECT_EQ(headers.get(Http::LowerCaseString("grpc-status"))[0]->value().getStringView(),
-                  "14");
-      }));
-  envoy_dynamic_module_callback_http_send_response(filter_.get(), 503, header_array, 1,
-                                                   {nullptr, 0}, {nullptr, 0});
-}
-
 TEST_F(DynamicModuleHttpFilterTest, AddCustomFlag) {
   // Test with empty response.
   EXPECT_CALL(decoder_callbacks_.stream_info_, addCustomFlag(testing::Eq("XXX")));
@@ -2016,67 +1984,6 @@ TEST(ABIImpl, GetAttributes) {
   EXPECT_EQ(result_number, 8386);
 }
 
-TEST(ABIImpl, GetAttributeResponseGrpcStatus) {
-  Stats::SymbolTableImpl symbol_table;
-  DynamicModuleHttpFilter filter{nullptr, symbol_table, 0};
-  NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_callbacks;
-  NiceMock<Http::MockStreamEncoderFilterCallbacks> encoder_callbacks;
-  StreamInfo::MockStreamInfo stream_info;
-  EXPECT_CALL(decoder_callbacks, streamInfo()).WillRepeatedly(testing::ReturnRef(stream_info));
-  filter.setDecoderFilterCallbacks(decoder_callbacks);
-  filter.setEncoderFilterCallbacks(encoder_callbacks);
-
-  uint64_t result = 0;
-
-  // Without response trailers or headers, gRPC status should be inferred from HTTP response code.
-  EXPECT_CALL(encoder_callbacks, responseTrailers())
-      .WillRepeatedly(testing::Return(ResponseTrailerMapOptRef()));
-  EXPECT_CALL(encoder_callbacks, responseHeaders())
-      .WillRepeatedly(testing::Return(ResponseHeaderMapOptRef()));
-  EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(testing::Return(503));
-  EXPECT_TRUE(envoy_dynamic_module_callback_http_filter_get_attribute_int(
-      &filter, envoy_dynamic_module_type_attribute_id_ResponseGrpcStatus, &result));
-  // HTTP 503 maps to gRPC Unavailable (14).
-  EXPECT_EQ(result, 14);
-}
-
-TEST(ABIImpl, GetAttributeResponseGrpcStatusFromTrailers) {
-  Stats::SymbolTableImpl symbol_table;
-  DynamicModuleHttpFilter filter{nullptr, symbol_table, 0};
-  NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_callbacks;
-  NiceMock<Http::MockStreamEncoderFilterCallbacks> encoder_callbacks;
-  StreamInfo::MockStreamInfo stream_info;
-  EXPECT_CALL(decoder_callbacks, streamInfo()).WillRepeatedly(testing::ReturnRef(stream_info));
-  filter.setDecoderFilterCallbacks(decoder_callbacks);
-  filter.setEncoderFilterCallbacks(encoder_callbacks);
-
-  uint64_t result = 0;
-
-  // With grpc-status in response trailers, that should take precedence.
-  Http::TestResponseTrailerMapImpl trailers{{"grpc-status", "7"}};
-  EXPECT_CALL(encoder_callbacks, responseTrailers())
-      .WillRepeatedly(testing::Return(makeOptRef<ResponseTrailerMap>(trailers)));
-  EXPECT_CALL(encoder_callbacks, responseHeaders())
-      .WillRepeatedly(testing::Return(ResponseHeaderMapOptRef()));
-  EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(testing::Return(200));
-  EXPECT_TRUE(envoy_dynamic_module_callback_http_filter_get_attribute_int(
-      &filter, envoy_dynamic_module_type_attribute_id_ResponseGrpcStatus, &result));
-  // grpc-status: 7 = PermissionDenied.
-  EXPECT_EQ(result, 7);
-}
-
-TEST(ABIImpl, GetAttributeResponseGrpcStatusNotAvailable) {
-  Stats::SymbolTableImpl symbol_table;
-  DynamicModuleHttpFilter filter_without_callbacks{nullptr, symbol_table, 0};
-
-  uint64_t result = 0;
-
-  // Without callbacks, stream info is not available.
-  EXPECT_FALSE(envoy_dynamic_module_callback_http_filter_get_attribute_int(
-      &filter_without_callbacks, envoy_dynamic_module_type_attribute_id_ResponseGrpcStatus,
-      &result));
-}
-
 TEST(ABIImpl, HttpCallout) {
   Stats::SymbolTableImpl symbol_table;
   DynamicModuleHttpFilter filter{nullptr, symbol_table, 0};