golang filter: do not clear route cache in ·HeaderMap.Set` by default.

changelogs/current.yaml

@@ -98,6 +98,9 @@ minor_behavior_changes:
  change: |
  Delta SDS removals will no longer result in a "Missing SDS resources" NACK
  and instead will be ignored.
+- area: golang
+ change: |
+ Not implicitly clearing route cache in ``HeaderMap.Set``, introduce a new API ``ClearRouteCache`` to do it.
 
 bug_fixes:
 # *Changes expected to improve the state of the world and are unlikely to have negative effects*

contrib/golang/common/go/api/api.h

@@ -53,6 +53,7 @@ typedef enum { // NOLINT(modernize-use-using)
  CAPISerializationFailure = -8,
 } CAPIStatus;
 
+CAPIStatus envoyGoFilterHttpClearRouteCache(void* r);
 CAPIStatus envoyGoFilterHttpContinue(void* r, int status);
 CAPIStatus envoyGoFilterHttpSendLocalReply(void* r, int response_code, void* body_text_data,
  int body_text_len, void* headers, int headers_num,

contrib/golang/common/go/api/capi.go

@@ -20,6 +20,7 @@ package api
 import "unsafe"
 
 type HttpCAPI interface {
+ ClearRouteCache(r unsafe.Pointer)
  HttpContinue(r unsafe.Pointer, status uint64)
  HttpSendLocalReply(r unsafe.Pointer, responseCode int, bodyText string, headers map[string][]string, grpcStatus int64, details string)
 

contrib/golang/common/go/api/filter.go

@@ -156,6 +156,9 @@ type StreamFilterCallbacks interface {
 
 type FilterCallbacks interface {
  StreamFilterCallbacks
+ // ClearRouteCache clears the route cache for the current request, and filtermanager will re-fetch the route in the next filter.
+ // Please be careful to invoke it, since filtermanager will raise an 404 route_not_found response when failed to re-fetch a route.
+ ClearRouteCache()
  // Continue or SendLocalReply should be last API invoked, no more code after them.
  Continue(StatusType)
  SendLocalReply(responseCode int, bodyText string, headers map[string][]string, grpcStatus int64, details string)

contrib/golang/common/go/api/type.go

@@ -109,16 +109,19 @@ type HeaderMap interface {
 
  // Set key-value pair in header map, the previous pair will be replaced if exists.
  // It may not take affects immediately in the Envoy thread side when it's invoked in a Go thread.
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
  Set(key, value string)
 
  // Add value for given key.
  // Multiple headers with the same key may be added with this function.
  // Use Set for setting a single header for the given key.
  // It may not take affects immediately in the Envoy thread side when it's invoked in a Go thread.
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
  Add(key, value string)
 
  // Del delete pair of specified key
  // It may not take affects immediately in the Envoy thread side when it's invoked in a Go thread.
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
  Del(key string)
 
  // Range calls f sequentially for each key and value present in the map.
@@ -136,6 +139,16 @@ type RequestHeaderMap interface {
  Method() string
  Host() string
  Path() string
+ // SetMethod set method in header map
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
+ SetMethod(method string)
+ // SetHost set host in header map
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
+ SetHost(host string)
+ // SetPath set path in header map
+ // This won't refresh route cache, please invoke ClearRouteCache if needed.
+ SetPath(path string)
+ // Note: Scheme is the downstream protocol, we'd better not override it.
 }
 
 type RequestTrailerMap interface {

contrib/golang/filters/http/source/cgo.cc

@@ -71,6 +71,11 @@ envoyGoConfigHandlerWrapper(void* c, std::function<CAPIStatus(std::shared_ptr<Fi
  return CAPIStatus::CAPIFilterIsGone;
 }
 
+CAPIStatus envoyGoFilterHttpClearRouteCache(void* r) {
+ return envoyGoFilterHandlerWrapper(
+ r, [](std::shared_ptr<Filter>& filter) -> CAPIStatus { return filter->clearRouteCache(); });
+}
+
 CAPIStatus envoyGoFilterHttpContinue(void* r, int status) {
  return envoyGoFilterHandlerWrapper(r, [status](std::shared_ptr<Filter>& filter) -> CAPIStatus {
  return filter->continueStatus(static_cast<GolangStatus>(status));

contrib/golang/filters/http/source/go/pkg/http/capi_impl.go

@@ -110,6 +110,11 @@ func capiStatusToErr(status C.CAPIStatus) error {
  return errors.New("unknown status")
 }
 
+func (c *httpCApiImpl) ClearRouteCache(r unsafe.Pointer) {
+ res := C.envoyGoFilterHttpClearRouteCache(r)
+ handleCApiStatus(res)
+}
+
 func (c *httpCApiImpl) HttpContinue(r unsafe.Pointer, status uint64) {
  res := C.envoyGoFilterHttpContinue(r, C.int(status))
  handleCApiStatus(res)

contrib/golang/filters/http/source/go/pkg/http/filter.go

@@ -157,6 +157,10 @@ func (r *httpRequest) RecoverPanic() {
  }
 }
 
+func (r *httpRequest) ClearRouteCache() {
+ cAPI.ClearRouteCache(unsafe.Pointer(r.req))
+}
+
 func (r *httpRequest) Continue(status api.StatusType) {
  if status == api.LocalReply {
  fmt.Printf("warning: LocalReply status is useless after sendLocalReply, ignoring")

contrib/golang/filters/http/source/go/pkg/http/type.go

@@ -182,6 +182,18 @@ func (h *requestHeaderMapImpl) Host() string {
  return v
 }
 
+func (h *requestHeaderMapImpl) SetMethod(method string) {
+ h.Set(":method", method)
+}
+
+func (h *requestHeaderMapImpl) SetPath(path string) {
+ h.Set(":path", path)
+}
+
+func (h *requestHeaderMapImpl) SetHost(host string) {
+ h.Set(":authority", host)
+}
+
 // api.ResponseHeaderMap
 type responseHeaderMapImpl struct {
  requestOrResponseHeaderMapImpl

contrib/golang/filters/http/source/golang_filter.cc

@@ -31,13 +31,6 @@ namespace Extensions {
 namespace HttpFilters {
 namespace Golang {
 
-void Filter::onHeadersModified() {
- // Any changes to request headers can affect how the request is going to be
- // routed. If we are changing the headers we also need to clear the route
- // cache.
- decoding_state_.getFilterCallbacks()->downstreamCallbacks()->clearRouteCache();
-}
-
 Http::LocalErrorStatus Filter::onLocalReply(const LocalReplyData& data) {
  auto& state = getProcessorState();
  ASSERT(state.isThreadSafe());
@@ -402,6 +395,22 @@ bool Filter::doTrailer(ProcessorState& state, Http::HeaderMap& trailers) {
 
 /*** APIs for go call C ***/
 
+CAPIStatus Filter::clearRouteCache() {
+ Thread::LockGuard lock(mutex_);
+ if (has_destroyed_) {
+ ENVOY_LOG(debug, "golang filter has been destroyed");
+ return CAPIStatus::CAPIFilterIsDestroy;
+ }
+ auto& state = getProcessorState();
+ if (!state.isProcessingInGo()) {
+ ENVOY_LOG(debug, "golang filter is not processing Go");
+ return CAPIStatus::CAPINotInGo;
+ }
+ ENVOY_LOG(debug, "golang filter clearing route cache");
+ decoding_state_.getFilterCallbacks()->downstreamCallbacks()->clearRouteCache();
+ return CAPIStatus::CAPIOK;
+}
+
 void Filter::continueEncodeLocalReply(ProcessorState& state) {
  ENVOY_LOG(debug,
  "golang filter continue encodeHeader(local reply from other filters) after return from "
@@ -702,8 +711,6 @@ CAPIStatus Filter::setHeader(absl::string_view key, absl::string_view value, hea
  default:
  RELEASE_ASSERT(false, absl::StrCat("unknown header action: ", act));
  }
-
- onHeadersModified();
  } else {
  // should deep copy the string_view before post to dipatcher callback.
  auto key_str = std::string(key);
@@ -728,8 +735,6 @@ CAPIStatus Filter::setHeader(absl::string_view key, absl::string_view value, hea
  default:
  RELEASE_ASSERT(false, absl::StrCat("unknown header action: ", act));
  }
-
- onHeadersModified();
  } else {
  ENVOY_LOG(debug, "golang filter has gone or destroyed in setHeader");
  }
@@ -759,7 +764,6 @@ CAPIStatus Filter::removeHeader(absl::string_view key) {
  if (state.isThreadSafe()) {
  // it's safe to write header in the safe thread.
  headers_->remove(Http::LowerCaseString(key));
- onHeadersModified();
  } else {
  // should deep copy the string_view before post to dipatcher callback.
  auto key_str = std::string(key);
@@ -772,7 +776,6 @@ CAPIStatus Filter::removeHeader(absl::string_view key) {
  if (!weak_ptr.expired() && !hasDestroyed()) {
  Thread::LockGuard lock(mutex_);
  headers_->remove(Http::LowerCaseString(key_str));
- onHeadersModified();
  } else {
  ENVOY_LOG(debug, "golang filter has gone or destroyed in removeHeader");
  }

contrib/golang/filters/http/source/golang_filter.h

@@ -211,6 +211,7 @@ class Filter : public Http::StreamFilter,
 
  void onStreamComplete() override {}
 
+ CAPIStatus clearRouteCache();
  CAPIStatus continueStatus(GolangStatus status);
 
  CAPIStatus sendLocalReply(Http::Code response_code, std::string body_text,
@@ -264,8 +265,6 @@ class Filter : public Http::StreamFilter,
  void continueStatusInternal(GolangStatus status);
  void continueData(ProcessorState& state);
 
- void onHeadersModified();
-
  void sendLocalReplyInternal(Http::Code response_code, absl::string_view body_text,
  std::function<void(Http::ResponseHeaderMap& headers)> modify_headers,
  Grpc::Status::GrpcStatus grpc_status, absl::string_view details);

contrib/golang/filters/http/test/golang_integration_test.cc

@@ -506,6 +506,35 @@ name: golang
  cleanup();
  }
 
+ void testRouteCache(std::string path, bool clear) {
+ initializeBasicFilter(BASIC);
+
+ codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http")));
+ Http::TestRequestHeaderMapImpl request_headers{
+ {":method", "POST"}, {":path", path}, {":scheme", "http"}, {":authority", "test.com"}};
+
+ auto encoder_decoder = codec_client_->startRequest(request_headers, true);
+ auto response = std::move(encoder_decoder.second);
+
+ // no route found after clearing
+ if (!clear) {
+ waitForNextUpstreamRequest();
+ Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}};
+ upstream_request_->encodeHeaders(response_headers, true);
+ }
+
+ ASSERT_TRUE(response->waitForEndStream());
+
+ // check resp status
+ if (clear) {
+ EXPECT_EQ("404", response->headers().getStatusValue());
+ } else {
+ EXPECT_EQ("200", response->headers().getStatusValue());
+ }
+
+ cleanup();
+ }
+
  void testSendLocalReply(std::string path, std::string phase) {
  initializeBasicFilter(BASIC);
 
@@ -1107,6 +1136,16 @@ TEST_P(GolangIntegrationTest, RouteConfig_Route) {
  testRouteConfig("test.com", "/route-config-test", false, "baz");
 }
 
+// Set new path without clear route cache, will get 200 response status
+TEST_P(GolangIntegrationTest, RouteCache_noClear) {
+ testRouteCache("/test?newPath=/not-found-path", false);
+}
+
+// Set new path with clear route cache, will get 404 response status
+TEST_P(GolangIntegrationTest, RouteCache_Clear) {
+ testRouteCache("/test?newPath=/not-found-path&clearRoute=1", true);
+}
+
 // Out of range in decode header phase
 TEST_P(GolangIntegrationTest, PanicRecover_DecodeHeader) {
  testPanicRecover("/test?panic=decode-header", "decode-header");

contrib/golang/filters/http/test/test_data/basic/filter.go

@@ -32,6 +32,8 @@ type filter struct {
  databuffer string // return api.Stop
  panic string // hit panic in which phase
  badapi bool // bad api call
+ newPath string // set new path
+ clearRoute bool // clear route cache
 }
 
 func parseQuery(path string) url.Values {
@@ -76,6 +78,8 @@ func (f *filter) initRequest(header api.RequestHeaderMap) {
  f.localreplay = f.query_params.Get("localreply")
  f.panic = f.query_params.Get("panic")
  f.badapi = f.query_params.Get("badapi") != ""
+ f.newPath = f.query_params.Get("newPath")
+ f.clearRoute = f.query_params.Get("clearRoute") != ""
 }
 
 func (f *filter) fail(msg string, a ...any) api.StatusType {
@@ -227,6 +231,13 @@ func (f *filter) decodeHeaders(header api.RequestHeaderMap, endStream bool) api.
  if f.panic == "decode-header" {
  badcode()
  }
+
+ if f.newPath != "" {
+ header.SetPath(f.newPath)
+ }
+ if f.clearRoute {
+ f.callbacks.ClearRouteCache()
+ }
  return api.Continue
 }