From 0542270804cef4b6a701f5350b9089c2c9053cfa Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Wed, 22 Apr 2026 16:49:04 +0530 Subject: [PATCH 01/11] BuildClientInterceptor --- internal/xds/httpfilter/extproc/config.go | 305 ++++++++++++++++++ internal/xds/httpfilter/extproc/ext_proc.go | 97 ++++++ .../xds/httpfilter/extproc/ext_proc_test.go | 290 +++++++++++++++++ 3 files changed, 692 insertions(+) create mode 100644 internal/xds/httpfilter/extproc/config.go create mode 100644 internal/xds/httpfilter/extproc/ext_proc.go create mode 100644 internal/xds/httpfilter/extproc/ext_proc_test.go diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go new file mode 100644 index 000000000000..b2d5b1b474fb --- /dev/null +++ b/internal/xds/httpfilter/extproc/config.go @@ -0,0 +1,305 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package extproc + +import ( + "fmt" + "regexp" + "time" + + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/internal/xds/httpfilter" + "google.golang.org/grpc/internal/xds/matcher" + "google.golang.org/grpc/metadata" + + v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" + v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" +) + +const defaultDeferredCloseTimeout = 5 * time.Second + +type baseConfig struct { + httpfilter.FilterConfig + config *v3procfilterpb.ExternalProcessor +} + +type overrideConfig struct { + httpfilter.FilterConfig + config *v3procfilterpb.ExtProcOverrides +} + +// interceptorConfig contains the configuration for the external processing +// client interceptor. +type interceptorConfig struct { + // The following fields can be set either in the filter config or the + // override config. If both are set, the override config will be used. + // + // server is the configuration for the external processing server. + server serverConfig + // failureModeAllow specifies the behavior when the RPC to the external + // processing server fails. If true, the dataplane PRC will be allowed to + // continue. If false, the data plane RPC will be failed with a grpc status + // code of UNAVAILABLE. + failureModeAllow bool + // processingMode specifies the processing mode for each dataplane event. + processingMode processingModes + // Attributes to be sent to the external processing server along with the + // request and response dataplane events. + requestAttributes []string + responseAttributes []string + + // The following fields can only be set in the base config. + // + // allowModeOverride specifies whether to allow the external processing + // server to dynamically override the processing mode. + allowModeOverride bool + // allowedOverrideModes specifies the processing modes that the external + // processing server is allowed to dynamically override to if + // allowModeOverride is true. If allowModeOverride is false, this field is + // ignored. If allowModeOverride is true and this field is empty, the + // external processing server can override to any processing mode. + allowedOverrideModes []*v3procfilterpb.ProcessingMode + // mutationRules specifies the rules for what modifications an external + // processing server may make to headers/trailers sent to it. + mutationRules headerMutationRules + // allowedHeaders specifies the headers that are allowed to be sent to the + // external processing server. If unset, all headers are allowed. + allowedHeaders []matcher.StringMatcher + // disallowedHeaders specifies the headers that will not be sent to the + // external processing server. This overrides the above AllowedHeaders if + // a header matches both. + disallowedHeaders []matcher.StringMatcher + // disableImmediateResponse specifies whether to disable immediate response + // from the external processing server. When true, if the response from + // external processing server has the `immediate_response` field set, the + // dataplane RPC will be failed with `UNAVAILABLE` status code. When false, + // the `immediate_response` field in the response from external processing + // server will be ignored. + disableImmediateResponse bool + // observabilityMode determines if the filter waits for the external + // processing server. If true, events are sent to the server in + // "observation-only" mode; the filter does not wait for a response. If + // false, the filter waits for a response, allowing the server to modify + // events before they reach the dataplane. + observabilityMode bool + // deferredCloseTimeout is the duration the filter waits before closing the + // external processing stream after the dataplane RPC completes. This is + // only applicable when observabilityMode is true; otherwise, it is ignored. + // The default value is 5 seconds. + deferredCloseTimeout time.Duration +} + +// processingMode defines how headers, trailers, and bodies are handled +// in relation to the external processing server. +type processingMode int + +const ( + // modeSkip indicates that the header/trailer/body should not be sent. + modeSkip processingMode = iota + // modeSend indicates that the header/trailer/body should be sent. + modeSend +) + +type processingModes struct { + requestHeaderMode processingMode + responseHeaderMode processingMode + responseTrailerMode processingMode + requestBodyMode processingMode + responseBodyMode processingMode +} + +// headerMutationRules specifies the rules for what modifications an external +// processing server may make to headers sent on the data plane RPC. +// +// Methods on this struct are safe to call on a nil pointer receiver, in which +// case all header mutations are permitted. +type headerMutationRules struct { + // allowExpr specifies a regular expression that matches the headers that + // can be mutated. + allowExpr *regexp.Regexp + // disallowExpr specifies a regular expression that matches the headers that + // cannot be mutated. This overrides the above allowExpr if a header matches + // both. + disallowExpr *regexp.Regexp + // disallowAll specifies that no header mutations are allowed. This + // overrides all other settings. + disallowAll bool + // disallowIsError specifies whether to return an error if a header mutation + // is disallowed. If true, the data plane RPC will be failed with a grpc + // status code of Unknown. + disallowIsError bool +} + +// serverConfig contains the configuration for an external server. +type serverConfig struct { + // targetURI is the name of the external server. + targetURI string + // channelCredentials specifies the transport credentials to use to connect + // to the external server. Must not be nil. + channelCredentials credentials.TransportCredentials + // callCredentials specifies the per-RPC credentials to use when making + // calls to the external server. + callCredentials []credentials.PerRPCCredentials + // timeout is the RPC timeout for the call to the external server. If unset, + // the timeout depends on the usage of this external server. For example, + // cases like ext_authz and ext_proc, where there is a 1:1 mapping between the + // data plane RPC and the external server call, the timeout will be capped by + // the timeout on the data plane RPC. For cases like RLQS where there is a + // side channel to the external server, an unset timeout will result in no + // timeout being applied to the external server call. + timeout time.Duration + // initialMetadata is the additional metadata to include in all RPCs sent to + // the external server. + initialMetadata metadata.MD +} + +// newInterceptorConfig creates the interceptor config from the base and +// override filter configs. The base config is required and the override config +// is optional. If a field is set in both the base and override configs, the +// value from the override config will be used. +func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3procfilterpb.ExtProcOverrides) (*interceptorConfig, error) { + iconfig := &interceptorConfig{ + failureModeAllow: base.GetFailureModeAllow(), + allowModeOverride: base.GetAllowModeOverride(), + requestAttributes: base.GetRequestAttributes(), + responseAttributes: base.GetResponseAttributes(), + allowedOverrideModes: base.GetAllowedOverrideModes(), + observabilityMode: base.GetObservabilityMode(), + disableImmediateResponse: base.GetDisableImmediateResponse(), + } + if base.GetDeferredCloseTimeout() != nil { + iconfig.deferredCloseTimeout = base.GetDeferredCloseTimeout().AsDuration() + } else { + iconfig.deferredCloseTimeout = defaultDeferredCloseTimeout + } + + var err error + if fr := base.GetForwardRules(); fr != nil { + if allowed := fr.GetAllowedHeaders(); allowed != nil { + iconfig.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()) + if err != nil { + return nil, fmt.Errorf("invalid allowed header matcher: %v", err) + } + } + if disallowed := fr.GetDisallowedHeaders(); disallowed != nil { + iconfig.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()) + if err != nil { + return nil, fmt.Errorf("invalid disallowed header matcher: %v", err) + } + } + } + + if mr := base.GetMutationRules(); mr != nil { + if allowexp := mr.GetAllowExpression(); allowexp != nil { + iconfig.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()) + if err != nil { + return nil, fmt.Errorf("invalid allow expression: %v", err) + } + } + if disallowexp := mr.GetDisallowExpression(); disallowexp != nil { + iconfig.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()) + if err != nil { + return nil, fmt.Errorf("invalid disallow expression: %v", err) + } + } + iconfig.mutationRules.disallowAll = mr.GetDisallowAll().GetValue() + iconfig.mutationRules.disallowIsError = mr.GetDisallowIsError().GetValue() + } + if iconfig.server, err = serverConfigFromGrpcService(base.GetGrpcService()); err != nil { + return nil, fmt.Errorf("failed to parse gRPC service config: %v", err) + } + if pm := base.GetProcessingMode(); pm != nil { + // The default processing mode is to send headers and skip body and + // trailers. + iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + } + + if override != nil { + if gs := override.GetGrpcService(); gs != nil { + serverCfg, err := serverConfigFromGrpcService(gs) + if err != nil { + return nil, err + } + iconfig.server = serverCfg + } + if fma := override.GetFailureModeAllow(); fma != nil { + iconfig.failureModeAllow = fma.GetValue() + } + if override.GetRequestAttributes() != nil { + iconfig.requestAttributes = override.GetRequestAttributes() + } + if override.GetResponseAttributes() != nil { + iconfig.responseAttributes = override.GetResponseAttributes() + } + if pm := override.GetProcessingMode(); pm != nil { + iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + } + } + return iconfig, nil +} + +// convertStringMatchers converts a slice of protobuf StringMatcher messages to +// a slice of matcher.StringMatcher. +func convertStringMatchers(patterns []*v3matcherpb.StringMatcher) ([]matcher.StringMatcher, error) { + var matchers []matcher.StringMatcher + for _, m := range patterns { + sm, err := matcher.StringMatcherFromProto(m) + if err != nil { + return nil, err + } + matchers = append(matchers, sm) + } + return matchers, nil +} + +// resolveHeaderMode resolves the processing mode for headers based on the +// protobuf enum value. If the mode is not set or set to Default, it returns the +// provided defaultMode. +func resolveHeaderMode(mode v3procfilterpb.ProcessingMode_HeaderSendMode, defaultMode processingMode) processingMode { + switch mode { + case v3procfilterpb.ProcessingMode_SEND: + return modeSend + case v3procfilterpb.ProcessingMode_SKIP: + return modeSkip + default: + return defaultMode + } +} + +// resolveBodyMode resolves the processing mode for body based on the protobuf +// enum value. If the mode is not set (i.e., default), it returns modeSkip, as +// the default for body is to skip. +func resolveBodyMode(mode v3procfilterpb.ProcessingMode_BodySendMode) processingMode { + switch mode { + case v3procfilterpb.ProcessingMode_GRPC: + return modeSend + case v3procfilterpb.ProcessingMode_NONE: + return modeSkip + default: + return modeSkip + } +} diff --git a/internal/xds/httpfilter/extproc/ext_proc.go b/internal/xds/httpfilter/extproc/ext_proc.go new file mode 100644 index 000000000000..462e87e9c85b --- /dev/null +++ b/internal/xds/httpfilter/extproc/ext_proc.go @@ -0,0 +1,97 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package extproc implements the Envoy external processing filter. +package extproc + +import ( + "fmt" + + "google.golang.org/grpc" + "google.golang.org/grpc/internal/resolver" + "google.golang.org/grpc/internal/xds/httpfilter" + + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3procservicepb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" +) + +var serverConfigFromGrpcService func(grpcService *v3corepb.GrpcService) (serverConfig, error) + +type builder struct{} + +func (builder) BuildClientFilter() httpfilter.ClientFilter { + return clientFilter{} +} + +var _ httpfilter.ClientFilterBuilder = builder{} + +type clientFilter struct{} + +func (clientFilter) Close() {} + +func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig) (resolver.ClientInterceptor, error) { + if cfg == nil { + return nil, fmt.Errorf("extproc: nil config provided") + } + + c, ok := cfg.(baseConfig) + if !ok { + return nil, fmt.Errorf("extproc: incorrect config type provided (%T): %v", cfg, cfg) + } + var ov overrideConfig + if override != nil { + ov, ok = override.(overrideConfig) + if !ok { + return nil, fmt.Errorf("extproc: incorrect override config type provided (%T): %v", override, override) + } + } + + config, err := newInterceptorConfig(c.config, ov.config) + if err != nil { + return nil, fmt.Errorf("extproc: %v", err) + } + + dOpts := []grpc.DialOption{grpc.WithTransportCredentials(config.server.channelCredentials)} + for _, creds := range config.server.callCredentials { + dOpts = append(dOpts, grpc.WithPerRPCCredentials(creds)) + } + cc, err := grpc.NewClient(config.server.targetURI, dOpts...) + if err != nil { + return nil, fmt.Errorf("extproc: failed to create client: %v", err) + } + extClient := v3procservicepb.NewExternalProcessorClient(cc) + + return &interceptor{ + config: config, + extClient: extClient, + cc: cc, + }, nil +} + +type interceptor struct { + resolver.ClientInterceptor + config *interceptorConfig + extClient v3procservicepb.ExternalProcessorClient + cc *grpc.ClientConn +} + +func (i *interceptor) Close() { + if i.cc != nil { + i.cc.Close() + } +} diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go new file mode 100644 index 000000000000..7d54aec3560a --- /dev/null +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -0,0 +1,290 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package extproc + +import ( + "fmt" + "reflect" + "strings" + "testing" + "time" + + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/xds/httpfilter" + "google.golang.org/grpc/internal/xds/matcher" + "google.golang.org/protobuf/types/known/durationpb" + "google.golang.org/protobuf/types/known/wrapperspb" + + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" + v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" +) + +const testBaseURI = "base-uri" + +type incorrectFilterConfig struct { + httpfilter.FilterConfig +} + +type s struct { + grpctest.Tester +} + +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) +} + +func (s) TestBuildClientInterceptor(t *testing.T) { + origServerConfigFromGrpcService := serverConfigFromGrpcService + defer func() { serverConfigFromGrpcService = origServerConfigFromGrpcService }() + + serverConfigFromGrpcService = func(grpcService *v3corepb.GrpcService) (serverConfig, error) { + if grpcService == nil { + return serverConfig{}, nil + } + if grpcService.GetGoogleGrpc() == nil { + return serverConfig{}, fmt.Errorf("missing google_grpc") + } + return serverConfig{ + targetURI: grpcService.GetGoogleGrpc().GetTargetUri(), + channelCredentials: insecure.NewCredentials(), + }, nil + } + + b := builder{} + f := b.BuildClientFilter() + defer f.Close() + + tests := []struct { + name string + cfg httpfilter.FilterConfig + override httpfilter.FilterConfig + wantConfig *interceptorConfig + wantErr string + }{ + { + name: "NilConfig", + cfg: nil, + wantErr: "extproc: nil config provided", + }, + { + name: "IncorrectConfigType", + cfg: incorrectFilterConfig{}, + wantErr: "extproc: incorrect config type provided", + }, + { + name: "IncorrectOverrideType", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{}}, + override: incorrectFilterConfig{}, + wantErr: "extproc: incorrect override config type provided", + }, + { + name: "DeferredCloseTimeoutDefault", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ + GrpcService: &v3corepb.GrpcService{ + TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ + GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ + TargetUri: testBaseURI, + }, + }, + }, + }}, + wantConfig: &interceptorConfig{ + server: serverConfig{ + targetURI: testBaseURI, + channelCredentials: insecure.NewCredentials(), + }, + deferredCloseTimeout: defaultDeferredCloseTimeout, + }, + }, + { + name: "CompleteBase", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ + FailureModeAllow: true, + AllowModeOverride: true, + RequestAttributes: []string{"attr1"}, + ResponseAttributes: []string{"attr2"}, + ObservabilityMode: true, + DisableImmediateResponse: true, + DeferredCloseTimeout: durationpb.New(10 * time.Second), + ProcessingMode: &v3procfilterpb.ProcessingMode{ + RequestHeaderMode: v3procfilterpb.ProcessingMode_SEND, + ResponseHeaderMode: v3procfilterpb.ProcessingMode_SKIP, + ResponseTrailerMode: v3procfilterpb.ProcessingMode_SEND, + RequestBodyMode: v3procfilterpb.ProcessingMode_GRPC, + ResponseBodyMode: v3procfilterpb.ProcessingMode_NONE, + }, + GrpcService: &v3corepb.GrpcService{ + TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ + GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ + TargetUri: testBaseURI, + }, + }, + }, + ForwardRules: &v3procfilterpb.HeaderForwardingRules{ + AllowedHeaders: &v3matcherpb.ListStringMatcher{ + Patterns: []*v3matcherpb.StringMatcher{ + { + MatchPattern: &v3matcherpb.StringMatcher_Exact{ + Exact: "allow-header", + }, + }, + }, + }, + }, + }}, + wantConfig: &interceptorConfig{ + failureModeAllow: true, + allowModeOverride: true, + requestAttributes: []string{"attr1"}, + responseAttributes: []string{"attr2"}, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingMode: processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSkip, + responseTrailerMode: modeSend, + requestBodyMode: modeSend, + responseBodyMode: modeSkip, + }, + server: serverConfig{ + targetURI: testBaseURI, + channelCredentials: insecure.NewCredentials(), + }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + }, + }, + { + name: "CompleteBaseAndOverride", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ + FailureModeAllow: false, + AllowModeOverride: true, + RequestAttributes: []string{"base-attr1"}, + ResponseAttributes: []string{"base-attr2"}, + ObservabilityMode: true, + DisableImmediateResponse: true, + DeferredCloseTimeout: durationpb.New(10 * time.Second), + ProcessingMode: &v3procfilterpb.ProcessingMode{ + RequestHeaderMode: v3procfilterpb.ProcessingMode_SEND, + ResponseHeaderMode: v3procfilterpb.ProcessingMode_SKIP, + ResponseTrailerMode: v3procfilterpb.ProcessingMode_SEND, + RequestBodyMode: v3procfilterpb.ProcessingMode_GRPC, + ResponseBodyMode: v3procfilterpb.ProcessingMode_NONE, + }, + GrpcService: &v3corepb.GrpcService{ + TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ + GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ + TargetUri: testBaseURI, + }, + }, + }, + ForwardRules: &v3procfilterpb.HeaderForwardingRules{ + AllowedHeaders: &v3matcherpb.ListStringMatcher{ + Patterns: []*v3matcherpb.StringMatcher{ + { + MatchPattern: &v3matcherpb.StringMatcher_Exact{ + Exact: "allow-header", + }, + }, + }, + }, + }, + }}, + override: overrideConfig{config: &v3procfilterpb.ExtProcOverrides{ + FailureModeAllow: wrapperspb.Bool(true), + RequestAttributes: []string{"override-attr1"}, + ResponseAttributes: []string{"override-attr2"}, + ProcessingMode: &v3procfilterpb.ProcessingMode{ + RequestHeaderMode: v3procfilterpb.ProcessingMode_SKIP, + ResponseHeaderMode: v3procfilterpb.ProcessingMode_SEND, + ResponseTrailerMode: v3procfilterpb.ProcessingMode_SKIP, + RequestBodyMode: v3procfilterpb.ProcessingMode_NONE, + ResponseBodyMode: v3procfilterpb.ProcessingMode_GRPC, + }, + GrpcService: &v3corepb.GrpcService{ + TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ + GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ + TargetUri: "override-uri", + }, + }, + }, + }}, + wantConfig: &interceptorConfig{ + failureModeAllow: true, + allowModeOverride: true, + requestAttributes: []string{"override-attr1"}, + responseAttributes: []string{"override-attr2"}, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingMode: processingModes{ + requestHeaderMode: modeSkip, + responseHeaderMode: modeSend, + responseTrailerMode: modeSkip, + requestBodyMode: modeSkip, + responseBodyMode: modeSend, + }, + server: serverConfig{ + targetURI: "override-uri", + channelCredentials: insecure.NewCredentials(), + // TODO : Remove these when timeout and metadata are used. Adding zero + // values here to satisfy the vet. + timeout: 0, + initialMetadata: nil, + }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + }, + }, + { + name: "GrpcServiceError", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ + GrpcService: &v3corepb.GrpcService{}, + }}, + wantErr: "failed to parse gRPC service config", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + intptr, err := f.BuildClientInterceptor(tc.cfg, tc.override) + if tc.wantErr == "" { + if err != nil { + t.Fatalf("BuildClientInterceptor() unexpected error: %v", err) + } + got, ok := intptr.(*interceptor) + if !ok { + t.Fatalf("BuildClientInterceptor() returned %T, want *interceptor", intptr) + } + if !reflect.DeepEqual(got.config, tc.wantConfig) { + t.Fatalf("interceptor.config = %+v, want %+v", got.config, tc.wantConfig) + } + intptr.Close() + } else { + if err == nil { + t.Fatalf("BuildClientInterceptor() expected error %v, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("BuildClientInterceptor() error = %v, want error containing %q", err, tc.wantErr) + } + } + }) + } +} From d31b4c92cc9824479a4b678f28a9c7c600cbe782 Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Wed, 22 Apr 2026 18:41:50 +0530 Subject: [PATCH 02/11] minor fixes --- internal/xds/httpfilter/extproc/config.go | 124 ++++++++---------- .../xds/httpfilter/extproc/ext_proc_test.go | 39 +++--- 2 files changed, 75 insertions(+), 88 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index b2d5b1b474fb..d857737d15c6 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -47,8 +47,8 @@ type overrideConfig struct { // interceptorConfig contains the configuration for the external processing // client interceptor. type interceptorConfig struct { - // The following fields can be set either in the filter config or the - // override config. If both are set, the override config will be used. + // The following fields can be set either in the filter config or the override + // config. If both are set, the override config will be used. // // server is the configuration for the external processing server. server serverConfig @@ -66,14 +66,14 @@ type interceptorConfig struct { // The following fields can only be set in the base config. // - // allowModeOverride specifies whether to allow the external processing - // server to dynamically override the processing mode. + // allowModeOverride specifies whether to allow the external processing server + // to dynamically override the processing mode. allowModeOverride bool // allowedOverrideModes specifies the processing modes that the external // processing server is allowed to dynamically override to if // allowModeOverride is true. If allowModeOverride is false, this field is - // ignored. If allowModeOverride is true and this field is empty, the - // external processing server can override to any processing mode. + // ignored. If allowModeOverride is true and this field is empty, the external + // processing server can override any processing mode. allowedOverrideModes []*v3procfilterpb.ProcessingMode // mutationRules specifies the rules for what modifications an external // processing server may make to headers/trailers sent to it. @@ -82,8 +82,8 @@ type interceptorConfig struct { // external processing server. If unset, all headers are allowed. allowedHeaders []matcher.StringMatcher // disallowedHeaders specifies the headers that will not be sent to the - // external processing server. This overrides the above AllowedHeaders if - // a header matches both. + // external processing server. This overrides the above AllowedHeaders if a + // header matches both. disallowedHeaders []matcher.StringMatcher // disableImmediateResponse specifies whether to disable immediate response // from the external processing server. When true, if the response from @@ -94,19 +94,19 @@ type interceptorConfig struct { disableImmediateResponse bool // observabilityMode determines if the filter waits for the external // processing server. If true, events are sent to the server in - // "observation-only" mode; the filter does not wait for a response. If - // false, the filter waits for a response, allowing the server to modify - // events before they reach the dataplane. + // "observation-only" mode; the filter does not wait for a response. If false, + // the filter waits for a response, allowing the server to modify events + // before they reach the dataplane. observabilityMode bool // deferredCloseTimeout is the duration the filter waits before closing the - // external processing stream after the dataplane RPC completes. This is - // only applicable when observabilityMode is true; otherwise, it is ignored. - // The default value is 5 seconds. + // external processing stream after the dataplane RPC completes. This is only + // applicable when observabilityMode is true; otherwise, it is ignored. The + // default value is 5 seconds. deferredCloseTimeout time.Duration } -// processingMode defines how headers, trailers, and bodies are handled -// in relation to the external processing server. +// processingMode defines how headers, trailers, and bodies are handled in +// relation to the external processing server. type processingMode int const ( @@ -126,19 +126,16 @@ type processingModes struct { // headerMutationRules specifies the rules for what modifications an external // processing server may make to headers sent on the data plane RPC. -// -// Methods on this struct are safe to call on a nil pointer receiver, in which -// case all header mutations are permitted. type headerMutationRules struct { - // allowExpr specifies a regular expression that matches the headers that - // can be mutated. + // allowExpr specifies a regular expression that matches the headers that can + // be mutated. allowExpr *regexp.Regexp // disallowExpr specifies a regular expression that matches the headers that // cannot be mutated. This overrides the above allowExpr if a header matches // both. disallowExpr *regexp.Regexp - // disallowAll specifies that no header mutations are allowed. This - // overrides all other settings. + // disallowAll specifies that no header mutations are allowed. This overrides + // all other settings. disallowAll bool // disallowIsError specifies whether to return an error if a header mutation // is disallowed. If true, the data plane RPC will be failed with a grpc @@ -150,11 +147,11 @@ type headerMutationRules struct { type serverConfig struct { // targetURI is the name of the external server. targetURI string - // channelCredentials specifies the transport credentials to use to connect - // to the external server. Must not be nil. + // channelCredentials specifies the transport credentials to use to connect to + // the external server. Must not be nil. channelCredentials credentials.TransportCredentials - // callCredentials specifies the per-RPC credentials to use when making - // calls to the external server. + // callCredentials specifies the per-RPC credentials to use when making calls + // to the external server. callCredentials []credentials.PerRPCCredentials // timeout is the RPC timeout for the call to the external server. If unset, // the timeout depends on the usage of this external server. For example, @@ -190,31 +187,25 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr } var err error - if fr := base.GetForwardRules(); fr != nil { - if allowed := fr.GetAllowedHeaders(); allowed != nil { - iconfig.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()) - if err != nil { - return nil, fmt.Errorf("invalid allowed header matcher: %v", err) - } + if allowed := base.GetForwardRules().GetAllowedHeaders(); allowed != nil { + if iconfig.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()); err != nil { + return nil, fmt.Errorf("invalid allowed header matcher: %v", err) } - if disallowed := fr.GetDisallowedHeaders(); disallowed != nil { - iconfig.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()) - if err != nil { - return nil, fmt.Errorf("invalid disallowed header matcher: %v", err) - } + } + if disallowed := base.GetForwardRules().GetDisallowedHeaders(); disallowed != nil { + if iconfig.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()); err != nil { + return nil, fmt.Errorf("invalid disallowed header matcher: %v", err) } } if mr := base.GetMutationRules(); mr != nil { if allowexp := mr.GetAllowExpression(); allowexp != nil { - iconfig.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()) - if err != nil { + if iconfig.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()); err != nil { return nil, fmt.Errorf("invalid allow expression: %v", err) } } if disallowexp := mr.GetDisallowExpression(); disallowexp != nil { - iconfig.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()) - if err != nil { + if iconfig.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()); err != nil { return nil, fmt.Errorf("invalid disallow expression: %v", err) } } @@ -233,31 +224,32 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) } - - if override != nil { - if gs := override.GetGrpcService(); gs != nil { - serverCfg, err := serverConfigFromGrpcService(gs) - if err != nil { - return nil, err - } - iconfig.server = serverCfg - } - if fma := override.GetFailureModeAllow(); fma != nil { - iconfig.failureModeAllow = fma.GetValue() - } - if override.GetRequestAttributes() != nil { - iconfig.requestAttributes = override.GetRequestAttributes() - } - if override.GetResponseAttributes() != nil { - iconfig.responseAttributes = override.GetResponseAttributes() - } - if pm := override.GetProcessingMode(); pm != nil { - iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + if override == nil { + return iconfig, nil + } + // Apply overrides if present. + if gs := override.GetGrpcService(); gs != nil { + serverCfg, err := serverConfigFromGrpcService(gs) + if err != nil { + return nil, err } + iconfig.server = serverCfg + } + if override.GetFailureModeAllow() != nil { + iconfig.failureModeAllow = override.GetFailureModeAllow().GetValue() + } + if override.GetRequestAttributes() != nil { + iconfig.requestAttributes = override.GetRequestAttributes() + } + if override.GetResponseAttributes() != nil { + iconfig.responseAttributes = override.GetResponseAttributes() + } + if pm := override.GetProcessingMode(); pm != nil { + iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) } return iconfig, nil } diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index 7d54aec3560a..9d261492c282 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -115,7 +115,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }, { - name: "CompleteBase", + name: "ConfigUsingOnlyBase", cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ FailureModeAllow: true, AllowModeOverride: true, @@ -173,7 +173,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }, { - name: "CompleteBaseAndOverride", + name: "ConfigUsingBaseAndOverride", cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ FailureModeAllow: false, AllowModeOverride: true, @@ -198,13 +198,11 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, ForwardRules: &v3procfilterpb.HeaderForwardingRules{ AllowedHeaders: &v3matcherpb.ListStringMatcher{ - Patterns: []*v3matcherpb.StringMatcher{ - { - MatchPattern: &v3matcherpb.StringMatcher_Exact{ - Exact: "allow-header", - }, + Patterns: []*v3matcherpb.StringMatcher{{ + MatchPattern: &v3matcherpb.StringMatcher_Exact{ + Exact: "allow-header", }, - }, + }}, }, }, }}, @@ -247,21 +245,18 @@ func (s) TestBuildClientInterceptor(t *testing.T) { channelCredentials: insecure.NewCredentials(), // TODO : Remove these when timeout and metadata are used. Adding zero // values here to satisfy the vet. - timeout: 0, - initialMetadata: nil, + timeout: 0, + initialMetadata: nil, }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, }, }, { - name: "GrpcServiceError", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ - GrpcService: &v3corepb.GrpcService{}, - }}, + name: "GrpcServiceError", + cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{GrpcService: &v3corepb.GrpcService{}}}, wantErr: "failed to parse gRPC service config", }, } - for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { intptr, err := f.BuildClientInterceptor(tc.cfg, tc.override) @@ -277,13 +272,13 @@ func (s) TestBuildClientInterceptor(t *testing.T) { t.Fatalf("interceptor.config = %+v, want %+v", got.config, tc.wantConfig) } intptr.Close() - } else { - if err == nil { - t.Fatalf("BuildClientInterceptor() expected error %v, got nil", tc.wantErr) - } - if !strings.Contains(err.Error(), tc.wantErr) { - t.Fatalf("BuildClientInterceptor() error = %v, want error containing %q", err, tc.wantErr) - } + return + } + if err == nil { + t.Fatalf("BuildClientInterceptor() expected error %v, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("BuildClientInterceptor() error = %v, want error containing %q", err, tc.wantErr) } }) } From d0d83d3fedc470186fced72467d3d107e8cfa88a Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Thu, 23 Apr 2026 18:45:03 +0530 Subject: [PATCH 03/11] minor --- internal/xds/httpfilter/extproc/config.go | 63 ++++++++++++----------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index d857737d15c6..1f0271708b0d 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -171,7 +171,7 @@ type serverConfig struct { // is optional. If a field is set in both the base and override configs, the // value from the override config will be used. func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3procfilterpb.ExtProcOverrides) (*interceptorConfig, error) { - iconfig := &interceptorConfig{ + ic := &interceptorConfig{ failureModeAllow: base.GetFailureModeAllow(), allowModeOverride: base.GetAllowModeOverride(), requestAttributes: base.GetRequestAttributes(), @@ -181,77 +181,78 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr disableImmediateResponse: base.GetDisableImmediateResponse(), } if base.GetDeferredCloseTimeout() != nil { - iconfig.deferredCloseTimeout = base.GetDeferredCloseTimeout().AsDuration() + ic.deferredCloseTimeout = base.GetDeferredCloseTimeout().AsDuration() } else { - iconfig.deferredCloseTimeout = defaultDeferredCloseTimeout + ic.deferredCloseTimeout = defaultDeferredCloseTimeout } var err error if allowed := base.GetForwardRules().GetAllowedHeaders(); allowed != nil { - if iconfig.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()); err != nil { + if ic.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()); err != nil { return nil, fmt.Errorf("invalid allowed header matcher: %v", err) } } if disallowed := base.GetForwardRules().GetDisallowedHeaders(); disallowed != nil { - if iconfig.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()); err != nil { + if ic.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()); err != nil { return nil, fmt.Errorf("invalid disallowed header matcher: %v", err) } } if mr := base.GetMutationRules(); mr != nil { if allowexp := mr.GetAllowExpression(); allowexp != nil { - if iconfig.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()); err != nil { + if ic.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()); err != nil { return nil, fmt.Errorf("invalid allow expression: %v", err) } } if disallowexp := mr.GetDisallowExpression(); disallowexp != nil { - if iconfig.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()); err != nil { + if ic.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()); err != nil { return nil, fmt.Errorf("invalid disallow expression: %v", err) } } - iconfig.mutationRules.disallowAll = mr.GetDisallowAll().GetValue() - iconfig.mutationRules.disallowIsError = mr.GetDisallowIsError().GetValue() + ic.mutationRules.disallowAll = mr.GetDisallowAll().GetValue() + ic.mutationRules.disallowIsError = mr.GetDisallowIsError().GetValue() } - if iconfig.server, err = serverConfigFromGrpcService(base.GetGrpcService()); err != nil { + if ic.server, err = serverConfigFromGrpcService(base.GetGrpcService()); err != nil { return nil, fmt.Errorf("failed to parse gRPC service config: %v", err) } - if pm := base.GetProcessingMode(); pm != nil { - // The default processing mode is to send headers and skip body and - // trailers. - iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) - } + + pm := base.GetProcessingMode() + // The default processing mode is to send headers and skip body and + // trailers. + ic.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + ic.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + ic.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + ic.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + ic.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + if override == nil { - return iconfig, nil + return ic, nil } // Apply overrides if present. if gs := override.GetGrpcService(); gs != nil { - serverCfg, err := serverConfigFromGrpcService(gs) + sc, err := serverConfigFromGrpcService(gs) if err != nil { return nil, err } - iconfig.server = serverCfg + ic.server = sc } if override.GetFailureModeAllow() != nil { - iconfig.failureModeAllow = override.GetFailureModeAllow().GetValue() + ic.failureModeAllow = override.GetFailureModeAllow().GetValue() } if override.GetRequestAttributes() != nil { - iconfig.requestAttributes = override.GetRequestAttributes() + ic.requestAttributes = override.GetRequestAttributes() } if override.GetResponseAttributes() != nil { - iconfig.responseAttributes = override.GetResponseAttributes() + ic.responseAttributes = override.GetResponseAttributes() } if pm := override.GetProcessingMode(); pm != nil { - iconfig.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - iconfig.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - iconfig.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - iconfig.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - iconfig.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + ic.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + ic.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + ic.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + ic.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + ic.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) } - return iconfig, nil + return ic, nil } // convertStringMatchers converts a slice of protobuf StringMatcher messages to From 02e1edb7199280146ccbbaef11c0727355513c09 Mon Sep 17 00:00:00 2001 From: Eshita Chandwani Date: Sun, 26 Apr 2026 15:43:19 +0530 Subject: [PATCH 04/11] remove mode override and fix test --- internal/xds/httpfilter/extproc/config.go | 13 +----- .../xds/httpfilter/extproc/ext_proc_test.go | 44 ++++++++++++++++--- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index 1f0271708b0d..9082b9ae8c43 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -66,15 +66,6 @@ type interceptorConfig struct { // The following fields can only be set in the base config. // - // allowModeOverride specifies whether to allow the external processing server - // to dynamically override the processing mode. - allowModeOverride bool - // allowedOverrideModes specifies the processing modes that the external - // processing server is allowed to dynamically override to if - // allowModeOverride is true. If allowModeOverride is false, this field is - // ignored. If allowModeOverride is true and this field is empty, the external - // processing server can override any processing mode. - allowedOverrideModes []*v3procfilterpb.ProcessingMode // mutationRules specifies the rules for what modifications an external // processing server may make to headers/trailers sent to it. mutationRules headerMutationRules @@ -173,10 +164,8 @@ type serverConfig struct { func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3procfilterpb.ExtProcOverrides) (*interceptorConfig, error) { ic := &interceptorConfig{ failureModeAllow: base.GetFailureModeAllow(), - allowModeOverride: base.GetAllowModeOverride(), requestAttributes: base.GetRequestAttributes(), responseAttributes: base.GetResponseAttributes(), - allowedOverrideModes: base.GetAllowedOverrideModes(), observabilityMode: base.GetObservabilityMode(), disableImmediateResponse: base.GetDisableImmediateResponse(), } @@ -215,7 +204,7 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr if ic.server, err = serverConfigFromGrpcService(base.GetGrpcService()); err != nil { return nil, fmt.Errorf("failed to parse gRPC service config: %v", err) } - + pm := base.GetProcessingMode() // The default processing mode is to send headers and skip body and // trailers. diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index 9d261492c282..46f357ac0096 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -21,6 +21,7 @@ package extproc import ( "fmt" "reflect" + "regexp" "strings" "testing" "time" @@ -35,6 +36,7 @@ import ( v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + mutation_rulesv3 "github.com/envoyproxy/go-control-plane/envoy/config/common/mutation_rules/v3" ) const testBaseURI = "base-uri" @@ -112,13 +114,16 @@ func (s) TestBuildClientInterceptor(t *testing.T) { channelCredentials: insecure.NewCredentials(), }, deferredCloseTimeout: defaultDeferredCloseTimeout, + processingMode: processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSend, + }, }, }, { name: "ConfigUsingOnlyBase", cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ FailureModeAllow: true, - AllowModeOverride: true, RequestAttributes: []string{"attr1"}, ResponseAttributes: []string{"attr2"}, ObservabilityMode: true, @@ -149,12 +154,23 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }, }, + MutationRules: &mutation_rulesv3.HeaderMutationRules{ + AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, + DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, + DisallowAll: wrapperspb.Bool(true), + DisallowIsError: wrapperspb.Bool(true), + }, }}, wantConfig: &interceptorConfig{ failureModeAllow: true, - allowModeOverride: true, requestAttributes: []string{"attr1"}, responseAttributes: []string{"attr2"}, + mutationRules: headerMutationRules{ + allowExpr: regexp.MustCompile("allow-.*"), + disallowExpr: regexp.MustCompile("disallow-.*"), + disallowAll: true, + disallowIsError: true, + }, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, @@ -176,7 +192,6 @@ func (s) TestBuildClientInterceptor(t *testing.T) { name: "ConfigUsingBaseAndOverride", cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ FailureModeAllow: false, - AllowModeOverride: true, RequestAttributes: []string{"base-attr1"}, ResponseAttributes: []string{"base-attr2"}, ObservabilityMode: true, @@ -204,6 +219,19 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }}, }, + DisallowedHeaders: &v3matcherpb.ListStringMatcher{ + Patterns: []*v3matcherpb.StringMatcher{{ + MatchPattern: &v3matcherpb.StringMatcher_Exact{ + Exact: "disallow-header", + }, + }}, + }, + }, + MutationRules: &mutation_rulesv3.HeaderMutationRules{ + AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, + DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, + DisallowAll: wrapperspb.Bool(true), + DisallowIsError: wrapperspb.Bool(true), }, }}, override: overrideConfig{config: &v3procfilterpb.ExtProcOverrides{ @@ -227,9 +255,14 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }}, wantConfig: &interceptorConfig{ failureModeAllow: true, - allowModeOverride: true, requestAttributes: []string{"override-attr1"}, responseAttributes: []string{"override-attr2"}, + mutationRules: headerMutationRules{ + allowExpr: regexp.MustCompile("allow-.*"), + disallowExpr: regexp.MustCompile("disallow-.*"), + disallowAll: true, + disallowIsError: true, + }, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, @@ -248,7 +281,8 @@ func (s) TestBuildClientInterceptor(t *testing.T) { timeout: 0, initialMetadata: nil, }, - allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, }, }, { From f36a2954ee92a107fbc00bc747f7dd08345bc91a Mon Sep 17 00:00:00 2001 From: Eshita Chandwani Date: Sun, 26 Apr 2026 15:54:14 +0530 Subject: [PATCH 05/11] vet --- internal/xds/httpfilter/extproc/ext_proc_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index 46f357ac0096..f5355ca8e9f6 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -33,10 +33,10 @@ import ( "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/wrapperspb" + v3mutationrulespb "github.com/envoyproxy/go-control-plane/envoy/config/common/mutation_rules/v3" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" - mutation_rulesv3 "github.com/envoyproxy/go-control-plane/envoy/config/common/mutation_rules/v3" ) const testBaseURI = "base-uri" @@ -154,7 +154,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }, }, - MutationRules: &mutation_rulesv3.HeaderMutationRules{ + MutationRules: &v3mutationrulespb.HeaderMutationRules{ AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, DisallowAll: wrapperspb.Bool(true), @@ -227,7 +227,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }}, }, }, - MutationRules: &mutation_rulesv3.HeaderMutationRules{ + MutationRules: &v3mutationrulespb.HeaderMutationRules{ AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, DisallowAll: wrapperspb.Bool(true), From 328deca6f14010e10c31aea0db1350f3cf8a001b Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Sun, 26 Apr 2026 18:53:39 +0530 Subject: [PATCH 06/11] test fix --- .../xds/httpfilter/extproc/ext_proc_test.go | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index f5355ca8e9f6..ffe922e0da65 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -113,11 +113,14 @@ func (s) TestBuildClientInterceptor(t *testing.T) { targetURI: testBaseURI, channelCredentials: insecure.NewCredentials(), }, - deferredCloseTimeout: defaultDeferredCloseTimeout, processingMode: processingModes{ - requestHeaderMode: modeSend, - responseHeaderMode: modeSend, + requestHeaderMode: modeSend, + responseHeaderMode: modeSend, + requestBodyMode: modeSkip, + responseBodyMode: modeSkip, + responseTrailerMode: modeSkip, }, + deferredCloseTimeout: defaultDeferredCloseTimeout, }, }, { @@ -162,9 +165,9 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }}, wantConfig: &interceptorConfig{ - failureModeAllow: true, - requestAttributes: []string{"attr1"}, - responseAttributes: []string{"attr2"}, + failureModeAllow: true, + requestAttributes: []string{"attr1"}, + responseAttributes: []string{"attr2"}, mutationRules: headerMutationRules{ allowExpr: regexp.MustCompile("allow-.*"), disallowExpr: regexp.MustCompile("disallow-.*"), @@ -254,9 +257,9 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, }}, wantConfig: &interceptorConfig{ - failureModeAllow: true, - requestAttributes: []string{"override-attr1"}, - responseAttributes: []string{"override-attr2"}, + failureModeAllow: true, + requestAttributes: []string{"override-attr1"}, + responseAttributes: []string{"override-attr2"}, mutationRules: headerMutationRules{ allowExpr: regexp.MustCompile("allow-.*"), disallowExpr: regexp.MustCompile("disallow-.*"), From 3319dae1649f0ab09d6558bc4a6ed8ce2b3af503 Mon Sep 17 00:00:00 2001 From: eshitachandwani <59800922+eshitachandwani@users.noreply.github.com> Date: Sun, 26 Apr 2026 22:33:10 +0530 Subject: [PATCH 07/11] Update internal/xds/httpfilter/extproc/ext_proc.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- internal/xds/httpfilter/extproc/ext_proc.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/xds/httpfilter/extproc/ext_proc.go b/internal/xds/httpfilter/extproc/ext_proc.go index 462e87e9c85b..191c70dee326 100644 --- a/internal/xds/httpfilter/extproc/ext_proc.go +++ b/internal/xds/httpfilter/extproc/ext_proc.go @@ -30,7 +30,9 @@ import ( v3procservicepb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" ) -var serverConfigFromGrpcService func(grpcService *v3corepb.GrpcService) (serverConfig, error) +var serverConfigFromGrpcService = func(grpcService *v3corepb.GrpcService) (serverConfig, error) { + return serverConfig{}, fmt.Errorf("extproc: serverConfigFromGrpcService not implemented") +} type builder struct{} From 7ada0bad50f34cdb2a8684acf182eb27d1440a04 Mon Sep 17 00:00:00 2001 From: Eshita Chandwani Date: Sun, 26 Apr 2026 22:38:11 +0530 Subject: [PATCH 08/11] gemini review --- internal/xds/httpfilter/extproc/config.go | 2 +- internal/xds/httpfilter/extproc/ext_proc.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index 9082b9ae8c43..32bc2f881561 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -247,7 +247,7 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr // convertStringMatchers converts a slice of protobuf StringMatcher messages to // a slice of matcher.StringMatcher. func convertStringMatchers(patterns []*v3matcherpb.StringMatcher) ([]matcher.StringMatcher, error) { - var matchers []matcher.StringMatcher + matchers := make([]matcher.StringMatcher, 0, len(patterns)) for _, m := range patterns { sm, err := matcher.StringMatcherFromProto(m) if err != nil { diff --git a/internal/xds/httpfilter/extproc/ext_proc.go b/internal/xds/httpfilter/extproc/ext_proc.go index 191c70dee326..a9033c8bd747 100644 --- a/internal/xds/httpfilter/extproc/ext_proc.go +++ b/internal/xds/httpfilter/extproc/ext_proc.go @@ -30,7 +30,7 @@ import ( v3procservicepb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" ) -var serverConfigFromGrpcService = func(grpcService *v3corepb.GrpcService) (serverConfig, error) { +var serverConfigFromGrpcService = func(*v3corepb.GrpcService) (serverConfig, error) { return serverConfig{}, fmt.Errorf("extproc: serverConfigFromGrpcService not implemented") } From f3e7083b3f48aa8e92e43bb2452e56a652fb7d62 Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Mon, 27 Apr 2026 12:40:16 +0530 Subject: [PATCH 09/11] minor fixes --- internal/xds/httpfilter/extproc/config.go | 40 ++++++++--------- .../xds/httpfilter/extproc/ext_proc_test.go | 45 +++++++++---------- 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index 32bc2f881561..72d716ea2018 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -57,8 +57,8 @@ type interceptorConfig struct { // continue. If false, the data plane RPC will be failed with a grpc status // code of UNAVAILABLE. failureModeAllow bool - // processingMode specifies the processing mode for each dataplane event. - processingMode processingModes + // processingModes specifies the processing mode for each dataplane event. + processingModes processingModes // Attributes to be sent to the external processing server along with the // request and response dataplane events. requestAttributes []string @@ -188,15 +188,13 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr } if mr := base.GetMutationRules(); mr != nil { + // Ignoring the error here because we have already verified it when + // parsing the proto. if allowexp := mr.GetAllowExpression(); allowexp != nil { - if ic.mutationRules.allowExpr, err = regexp.Compile(allowexp.GetRegex()); err != nil { - return nil, fmt.Errorf("invalid allow expression: %v", err) - } + ic.mutationRules.allowExpr, _ = regexp.Compile(allowexp.GetRegex()) } if disallowexp := mr.GetDisallowExpression(); disallowexp != nil { - if ic.mutationRules.disallowExpr, err = regexp.Compile(disallowexp.GetRegex()); err != nil { - return nil, fmt.Errorf("invalid disallow expression: %v", err) - } + ic.mutationRules.disallowExpr, _ = regexp.Compile(disallowexp.GetRegex()) } ic.mutationRules.disallowAll = mr.GetDisallowAll().GetValue() ic.mutationRules.disallowIsError = mr.GetDisallowIsError().GetValue() @@ -208,11 +206,11 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr pm := base.GetProcessingMode() // The default processing mode is to send headers and skip body and // trailers. - ic.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - ic.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - ic.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - ic.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - ic.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + ic.processingModes.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + ic.processingModes.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + ic.processingModes.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + ic.processingModes.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + ic.processingModes.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) if override == nil { return ic, nil @@ -235,11 +233,11 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr ic.responseAttributes = override.GetResponseAttributes() } if pm := override.GetProcessingMode(); pm != nil { - ic.processingMode.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - ic.processingMode.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - ic.processingMode.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - ic.processingMode.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - ic.processingMode.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) + ic.processingModes.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) + ic.processingModes.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) + ic.processingModes.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) + ic.processingModes.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) + ic.processingModes.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) } return ic, nil } @@ -247,9 +245,9 @@ func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3pr // convertStringMatchers converts a slice of protobuf StringMatcher messages to // a slice of matcher.StringMatcher. func convertStringMatchers(patterns []*v3matcherpb.StringMatcher) ([]matcher.StringMatcher, error) { - matchers := make([]matcher.StringMatcher, 0, len(patterns)) - for _, m := range patterns { - sm, err := matcher.StringMatcherFromProto(m) + var matchers []matcher.StringMatcher + for _, p := range patterns { + sm, err := matcher.StringMatcherFromProto(p) if err != nil { return nil, err } diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index ffe922e0da65..fc3895dc19f7 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -41,6 +41,9 @@ import ( const testBaseURI = "base-uri" +// incorrectFilterConfig embeds httpfilter.FilterConfig but is not of type +// baseConfig/overrideConfig, and is used to test incorrect config types being +// passed to BuildClientInterceptor. type incorrectFilterConfig struct { httpfilter.FilterConfig } @@ -57,6 +60,8 @@ func (s) TestBuildClientInterceptor(t *testing.T) { origServerConfigFromGrpcService := serverConfigFromGrpcService defer func() { serverConfigFromGrpcService = origServerConfigFromGrpcService }() + // Mocking serverConfigFromGrpcService to return a test target URI and + // insecure creds. serverConfigFromGrpcService = func(grpcService *v3corepb.GrpcService) (serverConfig, error) { if grpcService == nil { return serverConfig{}, nil @@ -67,6 +72,8 @@ func (s) TestBuildClientInterceptor(t *testing.T) { return serverConfig{ targetURI: grpcService.GetGoogleGrpc().GetTargetUri(), channelCredentials: insecure.NewCredentials(), + initialMetadata: nil, + timeout: 0, }, nil } @@ -113,7 +120,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { targetURI: testBaseURI, channelCredentials: insecure.NewCredentials(), }, - processingMode: processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSend, requestBodyMode: modeSkip, @@ -148,13 +155,9 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, ForwardRules: &v3procfilterpb.HeaderForwardingRules{ AllowedHeaders: &v3matcherpb.ListStringMatcher{ - Patterns: []*v3matcherpb.StringMatcher{ - { - MatchPattern: &v3matcherpb.StringMatcher_Exact{ - Exact: "allow-header", - }, - }, - }, + Patterns: []*v3matcherpb.StringMatcher{{ + MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "allow-header"}, + }}, }, }, MutationRules: &v3mutationrulespb.HeaderMutationRules{ @@ -177,7 +180,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingMode: processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSkip, responseTrailerMode: modeSend, @@ -217,16 +220,12 @@ func (s) TestBuildClientInterceptor(t *testing.T) { ForwardRules: &v3procfilterpb.HeaderForwardingRules{ AllowedHeaders: &v3matcherpb.ListStringMatcher{ Patterns: []*v3matcherpb.StringMatcher{{ - MatchPattern: &v3matcherpb.StringMatcher_Exact{ - Exact: "allow-header", - }, + MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "allow-header"}, }}, }, DisallowedHeaders: &v3matcherpb.ListStringMatcher{ Patterns: []*v3matcherpb.StringMatcher{{ - MatchPattern: &v3matcherpb.StringMatcher_Exact{ - Exact: "disallow-header", - }, + MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "disallow-header"}, }}, }, }, @@ -269,7 +268,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingMode: processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSkip, responseHeaderMode: modeSend, responseTrailerMode: modeSkip, @@ -279,10 +278,6 @@ func (s) TestBuildClientInterceptor(t *testing.T) { server: serverConfig{ targetURI: "override-uri", channelCredentials: insecure.NewCredentials(), - // TODO : Remove these when timeout and metadata are used. Adding zero - // values here to satisfy the vet. - timeout: 0, - initialMetadata: nil, }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, @@ -299,20 +294,20 @@ func (s) TestBuildClientInterceptor(t *testing.T) { intptr, err := f.BuildClientInterceptor(tc.cfg, tc.override) if tc.wantErr == "" { if err != nil { - t.Fatalf("BuildClientInterceptor() unexpected error: %v", err) + t.Fatalf("BuildClientInterceptor() returned unexpected error: %v", err) } - got, ok := intptr.(*interceptor) + ic, ok := intptr.(*interceptor) if !ok { t.Fatalf("BuildClientInterceptor() returned %T, want *interceptor", intptr) } - if !reflect.DeepEqual(got.config, tc.wantConfig) { - t.Fatalf("interceptor.config = %+v, want %+v", got.config, tc.wantConfig) + if !reflect.DeepEqual(ic.config, tc.wantConfig) { + t.Fatalf("Interceptor config = %+v, want %+v", ic.config, tc.wantConfig) } intptr.Close() return } if err == nil { - t.Fatalf("BuildClientInterceptor() expected error %v, got nil", tc.wantErr) + t.Fatalf("BuildClientInterceptor() returned nil error, want error containing %q", tc.wantErr) } if !strings.Contains(err.Error(), tc.wantErr) { t.Fatalf("BuildClientInterceptor() error = %v, want error containing %q", err, tc.wantErr) From 87fb4c70e4afdd1c038ab15a1f75b58f7b66bc12 Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Thu, 30 Apr 2026 22:12:51 +0530 Subject: [PATCH 10/11] change the parameters to be complete interceptorCfg not proto --- internal/xds/httpfilter/extproc/config.go | 143 ++-------- internal/xds/httpfilter/extproc/ext_proc.go | 14 +- .../xds/httpfilter/extproc/ext_proc_test.go | 244 +++++++----------- 3 files changed, 113 insertions(+), 288 deletions(-) diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index 72d716ea2018..4930095aaf9d 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -19,7 +19,6 @@ package extproc import ( - "fmt" "regexp" "time" @@ -27,21 +26,16 @@ import ( "google.golang.org/grpc/internal/xds/httpfilter" "google.golang.org/grpc/internal/xds/matcher" "google.golang.org/grpc/metadata" - - v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" - v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" ) -const defaultDeferredCloseTimeout = 5 * time.Second - type baseConfig struct { httpfilter.FilterConfig - config *v3procfilterpb.ExternalProcessor + config interceptorConfig } type overrideConfig struct { httpfilter.FilterConfig - config *v3procfilterpb.ExtProcOverrides + config interceptorConfig } // interceptorConfig contains the configuration for the external processing @@ -51,14 +45,14 @@ type interceptorConfig struct { // config. If both are set, the override config will be used. // // server is the configuration for the external processing server. - server serverConfig + server *serverConfig // failureModeAllow specifies the behavior when the RPC to the external // processing server fails. If true, the dataplane PRC will be allowed to // continue. If false, the data plane RPC will be failed with a grpc status // code of UNAVAILABLE. - failureModeAllow bool + failureModeAllow *bool // processingModes specifies the processing mode for each dataplane event. - processingModes processingModes + processingModes *processingModes // Attributes to be sent to the external processing server along with the // request and response dataplane events. requestAttributes []string @@ -161,125 +155,24 @@ type serverConfig struct { // override filter configs. The base config is required and the override config // is optional. If a field is set in both the base and override configs, the // value from the override config will be used. -func newInterceptorConfig(base *v3procfilterpb.ExternalProcessor, override *v3procfilterpb.ExtProcOverrides) (*interceptorConfig, error) { - ic := &interceptorConfig{ - failureModeAllow: base.GetFailureModeAllow(), - requestAttributes: base.GetRequestAttributes(), - responseAttributes: base.GetResponseAttributes(), - observabilityMode: base.GetObservabilityMode(), - disableImmediateResponse: base.GetDisableImmediateResponse(), - } - if base.GetDeferredCloseTimeout() != nil { - ic.deferredCloseTimeout = base.GetDeferredCloseTimeout().AsDuration() - } else { - ic.deferredCloseTimeout = defaultDeferredCloseTimeout - } - - var err error - if allowed := base.GetForwardRules().GetAllowedHeaders(); allowed != nil { - if ic.allowedHeaders, err = convertStringMatchers(allowed.GetPatterns()); err != nil { - return nil, fmt.Errorf("invalid allowed header matcher: %v", err) - } - } - if disallowed := base.GetForwardRules().GetDisallowedHeaders(); disallowed != nil { - if ic.disallowedHeaders, err = convertStringMatchers(disallowed.GetPatterns()); err != nil { - return nil, fmt.Errorf("invalid disallowed header matcher: %v", err) - } - } - - if mr := base.GetMutationRules(); mr != nil { - // Ignoring the error here because we have already verified it when - // parsing the proto. - if allowexp := mr.GetAllowExpression(); allowexp != nil { - ic.mutationRules.allowExpr, _ = regexp.Compile(allowexp.GetRegex()) - } - if disallowexp := mr.GetDisallowExpression(); disallowexp != nil { - ic.mutationRules.disallowExpr, _ = regexp.Compile(disallowexp.GetRegex()) - } - ic.mutationRules.disallowAll = mr.GetDisallowAll().GetValue() - ic.mutationRules.disallowIsError = mr.GetDisallowIsError().GetValue() - } - if ic.server, err = serverConfigFromGrpcService(base.GetGrpcService()); err != nil { - return nil, fmt.Errorf("failed to parse gRPC service config: %v", err) - } - - pm := base.GetProcessingMode() - // The default processing mode is to send headers and skip body and - // trailers. - ic.processingModes.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - ic.processingModes.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - ic.processingModes.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - ic.processingModes.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - ic.processingModes.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) +func newInterceptorConfig(base, override interceptorConfig) interceptorConfig { + ic := base - if override == nil { - return ic, nil - } // Apply overrides if present. - if gs := override.GetGrpcService(); gs != nil { - sc, err := serverConfigFromGrpcService(gs) - if err != nil { - return nil, err - } - ic.server = sc - } - if override.GetFailureModeAllow() != nil { - ic.failureModeAllow = override.GetFailureModeAllow().GetValue() - } - if override.GetRequestAttributes() != nil { - ic.requestAttributes = override.GetRequestAttributes() + if override.server != nil { + ic.server = override.server } - if override.GetResponseAttributes() != nil { - ic.responseAttributes = override.GetResponseAttributes() + if override.failureModeAllow != nil { + ic.failureModeAllow = override.failureModeAllow } - if pm := override.GetProcessingMode(); pm != nil { - ic.processingModes.requestHeaderMode = resolveHeaderMode(pm.GetRequestHeaderMode(), modeSend) - ic.processingModes.responseHeaderMode = resolveHeaderMode(pm.GetResponseHeaderMode(), modeSend) - ic.processingModes.responseTrailerMode = resolveHeaderMode(pm.GetResponseTrailerMode(), modeSkip) - ic.processingModes.requestBodyMode = resolveBodyMode(pm.GetRequestBodyMode()) - ic.processingModes.responseBodyMode = resolveBodyMode(pm.GetResponseBodyMode()) - } - return ic, nil -} - -// convertStringMatchers converts a slice of protobuf StringMatcher messages to -// a slice of matcher.StringMatcher. -func convertStringMatchers(patterns []*v3matcherpb.StringMatcher) ([]matcher.StringMatcher, error) { - var matchers []matcher.StringMatcher - for _, p := range patterns { - sm, err := matcher.StringMatcherFromProto(p) - if err != nil { - return nil, err - } - matchers = append(matchers, sm) + if override.requestAttributes != nil { + ic.requestAttributes = override.requestAttributes } - return matchers, nil -} - -// resolveHeaderMode resolves the processing mode for headers based on the -// protobuf enum value. If the mode is not set or set to Default, it returns the -// provided defaultMode. -func resolveHeaderMode(mode v3procfilterpb.ProcessingMode_HeaderSendMode, defaultMode processingMode) processingMode { - switch mode { - case v3procfilterpb.ProcessingMode_SEND: - return modeSend - case v3procfilterpb.ProcessingMode_SKIP: - return modeSkip - default: - return defaultMode + if override.responseAttributes != nil { + ic.responseAttributes = override.responseAttributes } -} - -// resolveBodyMode resolves the processing mode for body based on the protobuf -// enum value. If the mode is not set (i.e., default), it returns modeSkip, as -// the default for body is to skip. -func resolveBodyMode(mode v3procfilterpb.ProcessingMode_BodySendMode) processingMode { - switch mode { - case v3procfilterpb.ProcessingMode_GRPC: - return modeSend - case v3procfilterpb.ProcessingMode_NONE: - return modeSkip - default: - return modeSkip + if override.processingModes != nil { + ic.processingModes = override.processingModes } + return ic } diff --git a/internal/xds/httpfilter/extproc/ext_proc.go b/internal/xds/httpfilter/extproc/ext_proc.go index a9033c8bd747..3b2ac389cfa3 100644 --- a/internal/xds/httpfilter/extproc/ext_proc.go +++ b/internal/xds/httpfilter/extproc/ext_proc.go @@ -26,14 +26,9 @@ import ( "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/internal/xds/httpfilter" - v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3procservicepb "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" ) -var serverConfigFromGrpcService = func(*v3corepb.GrpcService) (serverConfig, error) { - return serverConfig{}, fmt.Errorf("extproc: serverConfigFromGrpcService not implemented") -} - type builder struct{} func (builder) BuildClientFilter() httpfilter.ClientFilter { @@ -55,6 +50,7 @@ func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig if !ok { return nil, fmt.Errorf("extproc: incorrect config type provided (%T): %v", cfg, cfg) } + var ov overrideConfig if override != nil { ov, ok = override.(overrideConfig) @@ -63,11 +59,9 @@ func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig } } - config, err := newInterceptorConfig(c.config, ov.config) - if err != nil { - return nil, fmt.Errorf("extproc: %v", err) - } + config := newInterceptorConfig(c.config, ov.config) + // Create a channel to the external processing server. dOpts := []grpc.DialOption{grpc.WithTransportCredentials(config.server.channelCredentials)} for _, creds := range config.server.callCredentials { dOpts = append(dOpts, grpc.WithPerRPCCredentials(creds)) @@ -87,7 +81,7 @@ func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig type interceptor struct { resolver.ClientInterceptor - config *interceptorConfig + config interceptorConfig extClient v3procservicepb.ExternalProcessorClient cc *grpc.ClientConn } diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index fc3895dc19f7..eea1e82c7e16 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -19,24 +19,17 @@ package extproc import ( - "fmt" - "reflect" "regexp" "strings" "testing" "time" + "github.com/google/go-cmp/cmp" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/xds/httpfilter" "google.golang.org/grpc/internal/xds/matcher" - "google.golang.org/protobuf/types/known/durationpb" - "google.golang.org/protobuf/types/known/wrapperspb" - - v3mutationrulespb "github.com/envoyproxy/go-control-plane/envoy/config/common/mutation_rules/v3" - v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" - v3procfilterpb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3" - v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + "google.golang.org/grpc/metadata" ) const testBaseURI = "base-uri" @@ -57,25 +50,6 @@ func Test(t *testing.T) { } func (s) TestBuildClientInterceptor(t *testing.T) { - origServerConfigFromGrpcService := serverConfigFromGrpcService - defer func() { serverConfigFromGrpcService = origServerConfigFromGrpcService }() - - // Mocking serverConfigFromGrpcService to return a test target URI and - // insecure creds. - serverConfigFromGrpcService = func(grpcService *v3corepb.GrpcService) (serverConfig, error) { - if grpcService == nil { - return serverConfig{}, nil - } - if grpcService.GetGoogleGrpc() == nil { - return serverConfig{}, fmt.Errorf("missing google_grpc") - } - return serverConfig{ - targetURI: grpcService.GetGoogleGrpc().GetTargetUri(), - channelCredentials: insecure.NewCredentials(), - initialMetadata: nil, - timeout: 0, - }, nil - } b := builder{} f := b.BuildClientFilter() @@ -100,75 +74,42 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, { name: "IncorrectOverrideType", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{}}, + cfg: baseConfig{config: interceptorConfig{}}, override: incorrectFilterConfig{}, wantErr: "extproc: incorrect override config type provided", }, - { - name: "DeferredCloseTimeoutDefault", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ - GrpcService: &v3corepb.GrpcService{ - TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ - GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ - TargetUri: testBaseURI, - }, - }, - }, - }}, - wantConfig: &interceptorConfig{ - server: serverConfig{ - targetURI: testBaseURI, - channelCredentials: insecure.NewCredentials(), - }, - processingModes: processingModes{ - requestHeaderMode: modeSend, - responseHeaderMode: modeSend, - requestBodyMode: modeSkip, - responseBodyMode: modeSkip, - responseTrailerMode: modeSkip, - }, - deferredCloseTimeout: defaultDeferredCloseTimeout, - }, - }, { name: "ConfigUsingOnlyBase", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ - FailureModeAllow: true, - RequestAttributes: []string{"attr1"}, - ResponseAttributes: []string{"attr2"}, - ObservabilityMode: true, - DisableImmediateResponse: true, - DeferredCloseTimeout: durationpb.New(10 * time.Second), - ProcessingMode: &v3procfilterpb.ProcessingMode{ - RequestHeaderMode: v3procfilterpb.ProcessingMode_SEND, - ResponseHeaderMode: v3procfilterpb.ProcessingMode_SKIP, - ResponseTrailerMode: v3procfilterpb.ProcessingMode_SEND, - RequestBodyMode: v3procfilterpb.ProcessingMode_GRPC, - ResponseBodyMode: v3procfilterpb.ProcessingMode_NONE, - }, - GrpcService: &v3corepb.GrpcService{ - TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ - GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ - TargetUri: testBaseURI, - }, + cfg: baseConfig{ + config: interceptorConfig{ + failureModeAllow: func() *bool { b := true; return &b }(), + requestAttributes: []string{"attr1"}, + responseAttributes: []string{"attr2"}, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingModes: &processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSkip, + responseTrailerMode: modeSend, + requestBodyMode: modeSend, + responseBodyMode: modeSkip, }, - }, - ForwardRules: &v3procfilterpb.HeaderForwardingRules{ - AllowedHeaders: &v3matcherpb.ListStringMatcher{ - Patterns: []*v3matcherpb.StringMatcher{{ - MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "allow-header"}, - }}, + server: &serverConfig{ + targetURI: testBaseURI, + channelCredentials: insecure.NewCredentials(), }, + mutationRules: headerMutationRules{ + allowExpr: regexp.MustCompile("allow-.*"), + disallowExpr: regexp.MustCompile("disallow-.*"), + disallowAll: true, + disallowIsError: true, + }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, }, - MutationRules: &v3mutationrulespb.HeaderMutationRules{ - AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, - DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, - DisallowAll: wrapperspb.Bool(true), - DisallowIsError: wrapperspb.Bool(true), - }, - }}, + }, wantConfig: &interceptorConfig{ - failureModeAllow: true, + failureModeAllow: func() *bool { b := true; return &b }(), requestAttributes: []string{"attr1"}, responseAttributes: []string{"attr2"}, mutationRules: headerMutationRules{ @@ -180,14 +121,14 @@ func (s) TestBuildClientInterceptor(t *testing.T) { observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: processingModes{ + processingModes: &processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSkip, responseTrailerMode: modeSend, requestBodyMode: modeSend, responseBodyMode: modeSkip, }, - server: serverConfig{ + server: &serverConfig{ targetURI: testBaseURI, channelCredentials: insecure.NewCredentials(), }, @@ -196,67 +137,57 @@ func (s) TestBuildClientInterceptor(t *testing.T) { }, { name: "ConfigUsingBaseAndOverride", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{ - FailureModeAllow: false, - RequestAttributes: []string{"base-attr1"}, - ResponseAttributes: []string{"base-attr2"}, - ObservabilityMode: true, - DisableImmediateResponse: true, - DeferredCloseTimeout: durationpb.New(10 * time.Second), - ProcessingMode: &v3procfilterpb.ProcessingMode{ - RequestHeaderMode: v3procfilterpb.ProcessingMode_SEND, - ResponseHeaderMode: v3procfilterpb.ProcessingMode_SKIP, - ResponseTrailerMode: v3procfilterpb.ProcessingMode_SEND, - RequestBodyMode: v3procfilterpb.ProcessingMode_GRPC, - ResponseBodyMode: v3procfilterpb.ProcessingMode_NONE, - }, - GrpcService: &v3corepb.GrpcService{ - TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ - GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ - TargetUri: testBaseURI, - }, + cfg: baseConfig{ + config: interceptorConfig{ + failureModeAllow: func() *bool { b := false; return &b }(), + requestAttributes: []string{"base-attr1"}, + responseAttributes: []string{"base-attr2"}, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingModes: &processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSkip, + responseTrailerMode: modeSend, + requestBodyMode: modeSend, + responseBodyMode: modeSkip, }, - }, - ForwardRules: &v3procfilterpb.HeaderForwardingRules{ - AllowedHeaders: &v3matcherpb.ListStringMatcher{ - Patterns: []*v3matcherpb.StringMatcher{{ - MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "allow-header"}, - }}, + server: &serverConfig{ + targetURI: testBaseURI, + channelCredentials: insecure.NewCredentials(), + timeout: time.Second, + initialMetadata: metadata.MD(metadata.Pairs("key1", "value1")), }, - DisallowedHeaders: &v3matcherpb.ListStringMatcher{ - Patterns: []*v3matcherpb.StringMatcher{{ - MatchPattern: &v3matcherpb.StringMatcher_Exact{Exact: "disallow-header"}, - }}, + mutationRules: headerMutationRules{ + allowExpr: regexp.MustCompile("allow-.*"), + disallowExpr: regexp.MustCompile("disallow-.*"), + disallowAll: true, + disallowIsError: true, }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, }, - MutationRules: &v3mutationrulespb.HeaderMutationRules{ - AllowExpression: &v3matcherpb.RegexMatcher{Regex: "allow-.*"}, - DisallowExpression: &v3matcherpb.RegexMatcher{Regex: "disallow-.*"}, - DisallowAll: wrapperspb.Bool(true), - DisallowIsError: wrapperspb.Bool(true), - }, - }}, - override: overrideConfig{config: &v3procfilterpb.ExtProcOverrides{ - FailureModeAllow: wrapperspb.Bool(true), - RequestAttributes: []string{"override-attr1"}, - ResponseAttributes: []string{"override-attr2"}, - ProcessingMode: &v3procfilterpb.ProcessingMode{ - RequestHeaderMode: v3procfilterpb.ProcessingMode_SKIP, - ResponseHeaderMode: v3procfilterpb.ProcessingMode_SEND, - ResponseTrailerMode: v3procfilterpb.ProcessingMode_SKIP, - RequestBodyMode: v3procfilterpb.ProcessingMode_NONE, - ResponseBodyMode: v3procfilterpb.ProcessingMode_GRPC, - }, - GrpcService: &v3corepb.GrpcService{ - TargetSpecifier: &v3corepb.GrpcService_GoogleGrpc_{ - GoogleGrpc: &v3corepb.GrpcService_GoogleGrpc{ - TargetUri: "override-uri", - }, + }, + override: overrideConfig{ + config: interceptorConfig{ + failureModeAllow: func() *bool { b := true; return &b }(), + requestAttributes: []string{"override-attr1"}, + responseAttributes: []string{"override-attr2"}, + processingModes: &processingModes{ + requestHeaderMode: modeSkip, + responseHeaderMode: modeSend, + responseTrailerMode: modeSkip, + requestBodyMode: modeSkip, + responseBodyMode: modeSend, + }, + server: &serverConfig{ + targetURI: "override-uri", + channelCredentials: insecure.NewCredentials(), }, }, - }}, + }, wantConfig: &interceptorConfig{ - failureModeAllow: true, + failureModeAllow: func() *bool { b := true; return &b }(), requestAttributes: []string{"override-attr1"}, responseAttributes: []string{"override-attr2"}, mutationRules: headerMutationRules{ @@ -268,14 +199,14 @@ func (s) TestBuildClientInterceptor(t *testing.T) { observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: processingModes{ + processingModes: &processingModes{ requestHeaderMode: modeSkip, responseHeaderMode: modeSend, responseTrailerMode: modeSkip, requestBodyMode: modeSkip, responseBodyMode: modeSend, }, - server: serverConfig{ + server: &serverConfig{ targetURI: "override-uri", channelCredentials: insecure.NewCredentials(), }, @@ -283,11 +214,6 @@ func (s) TestBuildClientInterceptor(t *testing.T) { disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, }, }, - { - name: "GrpcServiceError", - cfg: baseConfig{config: &v3procfilterpb.ExternalProcessor{GrpcService: &v3corepb.GrpcService{}}}, - wantErr: "failed to parse gRPC service config", - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -300,8 +226,20 @@ func (s) TestBuildClientInterceptor(t *testing.T) { if !ok { t.Fatalf("BuildClientInterceptor() returned %T, want *interceptor", intptr) } - if !reflect.DeepEqual(ic.config, tc.wantConfig) { - t.Fatalf("Interceptor config = %+v, want %+v", ic.config, tc.wantConfig) + cmpOpts := []cmp.Option{ + cmp.AllowUnexported(interceptorConfig{}, serverConfig{}, processingModes{}, headerMutationRules{}), + cmp.Transformer("RegexpToString", func(r *regexp.Regexp) string { + if r == nil { + return "" + } + return r.String() + }), + cmp.Comparer(func(x, y matcher.StringMatcher) bool { + return x.Equal(y) + }), + } + if diff := cmp.Diff(ic.config, *tc.wantConfig, cmpOpts...); diff != "" { + t.Fatalf("Interceptor config returned unexpected diff (-got +want):\n%s", diff) } intptr.Close() return From 76f6f0acc0cc944cf9dc97d9bd6f384cc304b673 Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Tue, 12 May 2026 15:40:39 +0530 Subject: [PATCH 11/11] change to use override --- experimental/optional/optional.go | 61 ++++++ experimental/optional/optional_test.go | 161 ++++++++++++++++ internal/xds/httpfilter/extconfig.go | 116 ++++++++++++ internal/xds/httpfilter/extproc/config.go | 95 ++++------ internal/xds/httpfilter/extproc/ext_proc.go | 10 +- .../xds/httpfilter/extproc/ext_proc_test.go | 177 +++++++++++++----- 6 files changed, 505 insertions(+), 115 deletions(-) create mode 100644 experimental/optional/optional.go create mode 100644 experimental/optional/optional_test.go create mode 100644 internal/xds/httpfilter/extconfig.go diff --git a/experimental/optional/optional.go b/experimental/optional/optional.go new file mode 100644 index 000000000000..b958f342c39f --- /dev/null +++ b/experimental/optional/optional.go @@ -0,0 +1,61 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package optional adds generic optional types. +// +// All APIs in this package are experimental. +package optional + +// Option represents an optional value of type T. +type Option[T any] struct { + val T + isSet bool +} + +// New creates a new Option that does not have a value set. This can also be +// done implicitly using a zero-value declaration: `var opt optional.Option[T]“ +func New[T any]() Option[T] { + return Option[T]{} +} + +// NewValue creates a new Option with the provided value. +func NewValue[T any](value T) Option[T] { + return Option[T]{ + val: value, + isSet: true, + } +} + +// Value returns the underlying value and a boolean indicating if the value is +// set. If the value is not set, it returns the zero value of T and false. +func (o Option[T]) Value() (T, bool) { + return o.val, o.isSet +} + +// WithValue returns a new Option containing the provided value. +func (o Option[T]) WithValue(value T) Option[T] { + return Option[T]{ + val: value, + isSet: true, + } +} + +// Clear returns an empty Option. +func (o Option[T]) Clear() Option[T] { + return Option[T]{} +} diff --git a/experimental/optional/optional_test.go b/experimental/optional/optional_test.go new file mode 100644 index 000000000000..dda76d002d68 --- /dev/null +++ b/experimental/optional/optional_test.go @@ -0,0 +1,161 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package optional_test + +import ( + "testing" + + "google.golang.org/grpc/experimental/optional" + "google.golang.org/grpc/internal/grpctest" +) + +type s struct { + grpctest.Tester +} + +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) +} + +type testStruct struct { + Name string + Age int +} + +// TestOption_Int tests the scenario of using integer optional values and +// verifies that default value, constructors, and mutation methods work as +// expected for primitive integers. +func (s) TestOption_Int(t *testing.T) { + var opt optional.Option[int] + // Test unset value. + if v, set := opt.Value(); set || v != 0 { + t.Fatalf("Zero-value Option[int] = (%v, %v); want (0, false)", v, set) + } + + // Test that New() function also returns an unset optional value. + optNew := optional.New[int]() + if v, set := optNew.Value(); set || v != 0 { + t.Fatalf("New[int]() = (%v, %v); want (0, false)", v, set) + } + + optVal := optional.NewValue(42) + if v, set := optVal.Value(); !set || v != 42 { + t.Fatalf("NewValue(42) = (%v, %v); want (42, true)", v, set) + } + + opt = opt.WithValue(100) + if v, set := opt.Value(); !set || v != 100 { + t.Fatalf("WithValue(100) = (%v, %v); want (100, true)", v, set) + } + + opt = opt.Clear() + if v, set := opt.Value(); set || v != 0 { + t.Fatalf("Clear() = (%v, %v); want (0, false)", v, set) + } +} + +// TestOption_String tests the scenario of using string optional values and +// verifies that default value, constructors, and mutation methods work as +// expected for text strings. +func (s) TestOption_String(t *testing.T) { + var opt optional.Option[string] + // Test unset value. + if v, set := opt.Value(); set || v != "" { + t.Fatalf("Zero-value Option[string] = (%q, %v); want (%q, false)", v, set, "") + } + + // Test that New() function also returns an unset optional value. + optNew := optional.New[string]() + if v, set := optNew.Value(); set || v != "" { + t.Fatalf("New Option[string] = (%q, %v); want (%q, false)", v, set, "") + } + + wantString := "test-string" + optVal := optional.NewValue(wantString) + if v, set := optVal.Value(); !set || v != wantString { + t.Fatalf("NewValue(%q) = (%q, %v); want (%q, true)", wantString, v, set, wantString) + } + + wantStringNew := "world" + opt = opt.WithValue(wantStringNew) + if v, set := opt.Value(); !set || v != wantStringNew { + t.Fatalf("WithValue(%q) = (%q, %v); want (%q, true)", wantStringNew, v, set, wantStringNew) + } + + opt = opt.Clear() + if v, set := opt.Value(); set || v != "" { + t.Fatalf("Clear() = (%q, %v); want (%q, false)", v, set, "") + } +} + +// TestOption_Struct tests the scenario of using a custom struct type inside an +// option type and verifies that custom struct field values are preserved, +// modified, and cleared correctly. +func (s) TestOption_Struct(t *testing.T) { + val1 := testStruct{Name: "Alice", Age: 30} + val2 := testStruct{Name: "Bob", Age: 40} + + var opt optional.Option[testStruct] + if v, set := opt.Value(); set || v != (testStruct{}) { + t.Fatalf("Zero-value Option[struct] = (%v, %v); want (empty, false)", v, set) + } + + optVal := optional.NewValue(val1) + if v, set := optVal.Value(); !set || v != val1 { + t.Fatalf("NewValue(val1) = (%v, %v); want (%v, true)", v, set, val1) + } + + opt = opt.WithValue(val2) + if v, set := opt.Value(); !set || v != val2 { + t.Fatalf("WithValue(val2) = (%v, %v); want (%v, true)", v, set, val2) + } + + opt = opt.Clear() + if v, set := opt.Value(); set || v != (testStruct{}) { + t.Fatalf("Clear() = (%v, %v); want (empty, false)", v, set) + } +} + +// TestOption_Pointer tests the scenario of using a pointer type inside an +// option type and verifies that nil status, address preservation, and +// underlying value dereferencing work as expected. +func (s) TestOption_Pointer(t *testing.T) { + val1 := 42 + val2 := 100 + + var opt optional.Option[*int] + if v, set := opt.Value(); set || v != nil { + t.Fatalf("Zero-value Option[*int] = (%v, %v); want (nil, false)", v, set) + } + + optVal := optional.NewValue(&val1) + if v, set := optVal.Value(); !set || v != &val1 || *v != val1 { + t.Fatalf("NewValue(%v) = (%v, %v); want (%v, true)", &val1, v, set, &val1) + } + + opt = opt.WithValue(&val2) + if v, set := opt.Value(); !set || v != &val2 || *v != val2 { + t.Fatalf("WithValue(%v) = (%v, %v); want (%v, true)", &val2, v, set, &val2) + } + + opt = opt.Clear() + if v, set := opt.Value(); set || v != nil { + t.Fatalf("Clear() = (%v, %v); want (nil, false)", v, set) + } +} diff --git a/internal/xds/httpfilter/extconfig.go b/internal/xds/httpfilter/extconfig.go new file mode 100644 index 000000000000..ac40fccb7d8f --- /dev/null +++ b/internal/xds/httpfilter/extconfig.go @@ -0,0 +1,116 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package httpfilter contains interface definitions for xDS-based HTTP filters +// and a registry for filter builders. +package httpfilter + +import ( + "encoding/json" + "fmt" + "regexp" + "time" + + "google.golang.org/grpc/internal/xds/matcher" + "google.golang.org/grpc/metadata" + + v3mutationpb "github.com/envoyproxy/go-control-plane/envoy/config/common/mutation_rules/v3" + v3matcherpb "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" +) + +// HeaderMutationRules specifies the rules for what modifications an external +// processing server may make to headers sent on the data plane RPC. +type HeaderMutationRules struct { + // AllowExpr specifies a regular expression that matches the headers that can + // be mutated. + AllowExpr *regexp.Regexp + // DisallowExpr specifies a regular expression that matches the headers that + // cannot be mutated. This overrides the above allowExpr if a header matches + // both. + DisallowExpr *regexp.Regexp + // DisallowAll specifies that no header mutations are allowed. This overrides + // all other settings. + DisallowAll bool + // DisallowIsError specifies whether to return an error if a header mutation + // is disallowed. If true, the data plane RPC will be failed with a grpc + // status code of Unknown. + DisallowIsError bool +} + +// ServerConfig contains the configuration for an external server. +type ServerConfig struct { + // TargetURI is the name of the external server. + TargetURI string + // ChannelCredentials specifies the transport credentials to use to connect to + // the external server. Must not be nil. + ChannelCredentials json.RawMessage + // CallCredentials specifies the per-RPC credentials to use when making calls + // to the external server. + CallCredentials []json.RawMessage + // Timeout is the RPC Timeout for the call to the external server. If unset, + // the Timeout depends on the usage of this external server. For example, + // cases like ext_authz and ext_proc, where there is a 1:1 mapping between the + // data plane RPC and the external server call, the Timeout will be capped by + // the Timeout on the data plane RPC. For cases like RLQS where there is a + // side channel to the external server, an unset Timeout will result in no + // Timeout being applied to the external server call. + Timeout time.Duration + // InitialMetadata is the additional metadata to include in all RPCs sent to + // the external server. + InitialMetadata metadata.MD +} + +// ConvertStringMatchers converts a slice of protobuf StringMatcher messages to +// a slice of matcher.StringMatcher. +func ConvertStringMatchers(patterns []*v3matcherpb.StringMatcher) ([]matcher.StringMatcher, error) { + matchers := make([]matcher.StringMatcher, 0, len(patterns)) + for _, p := range patterns { + sm, err := matcher.StringMatcherFromProto(p) + if err != nil { + return nil, err + } + matchers = append(matchers, sm) + } + return matchers, nil +} + +// HeaderMutationRulesFromProto converts a protobuf HeaderMutationRules message +// to a headerMutationRules struct. +func HeaderMutationRulesFromProto(mr *v3mutationpb.HeaderMutationRules) (HeaderMutationRules, error) { + var rules HeaderMutationRules + if mr == nil { + return rules, nil + } + if allowExpr := mr.GetAllowExpression(); allowExpr != nil { + re, err := regexp.Compile(allowExpr.GetRegex()) + if err != nil { + return rules, fmt.Errorf("extproc: %v", err) + } + rules.AllowExpr = re + } + if disallowExpr := mr.GetDisallowExpression(); disallowExpr != nil { + re, err := regexp.Compile(disallowExpr.GetRegex()) + if err != nil { + return rules, fmt.Errorf("extproc: %v", err) + } + rules.DisallowExpr = re + } + rules.DisallowAll = mr.GetDisallowAll().GetValue() + rules.DisallowIsError = mr.GetDisallowIsError().GetValue() + return rules, nil +} diff --git a/internal/xds/httpfilter/extproc/config.go b/internal/xds/httpfilter/extproc/config.go index 4930095aaf9d..594977427bce 100644 --- a/internal/xds/httpfilter/extproc/config.go +++ b/internal/xds/httpfilter/extproc/config.go @@ -19,13 +19,11 @@ package extproc import ( - "regexp" "time" - "google.golang.org/grpc/credentials" + "google.golang.org/grpc/experimental/optional" "google.golang.org/grpc/internal/xds/httpfilter" "google.golang.org/grpc/internal/xds/matcher" - "google.golang.org/grpc/metadata" ) type baseConfig struct { @@ -35,7 +33,28 @@ type baseConfig struct { type overrideConfig struct { httpfilter.FilterConfig - config interceptorConfig + config interceptorOverrideConfig +} + +// interceptorOverrideConfig contains the configuration for the external +// processing client interceptor override. This is used for overriding the base +// config. If a particular field is set , that will be used instead of the base +// config. +type interceptorOverrideConfig struct { + // server is the configuration for the external processing server. + server optional.Option[httpfilter.ServerConfig] + // processingModes specifies the processing mode for each dataplane event. + + processingModes optional.Option[processingModes] + // failureModeAllow specifies the behavior when the RPC to the external + // processing server fails. If true, the dataplane RPC will be allowed to + // continue. If false, the data plane RPC will be failed with a grpc status + // code of UNAVAILABLE. + failureModeAllow optional.Option[bool] + // Attributes to be sent to the external processing server along with the + // request and response dataplane events. + requestAttributes []string + responseAttributes []string } // interceptorConfig contains the configuration for the external processing @@ -45,14 +64,14 @@ type interceptorConfig struct { // config. If both are set, the override config will be used. // // server is the configuration for the external processing server. - server *serverConfig + server httpfilter.ServerConfig // failureModeAllow specifies the behavior when the RPC to the external - // processing server fails. If true, the dataplane PRC will be allowed to + // processing server fails. If true, the dataplane RPC will be allowed to // continue. If false, the data plane RPC will be failed with a grpc status // code of UNAVAILABLE. - failureModeAllow *bool + failureModeAllow bool // processingModes specifies the processing mode for each dataplane event. - processingModes *processingModes + processingModes processingModes // Attributes to be sent to the external processing server along with the // request and response dataplane events. requestAttributes []string @@ -62,12 +81,12 @@ type interceptorConfig struct { // // mutationRules specifies the rules for what modifications an external // processing server may make to headers/trailers sent to it. - mutationRules headerMutationRules + mutationRules httpfilter.HeaderMutationRules // allowedHeaders specifies the headers that are allowed to be sent to the // external processing server. If unset, all headers are allowed. allowedHeaders []matcher.StringMatcher // disallowedHeaders specifies the headers that will not be sent to the - // external processing server. This overrides the above AllowedHeaders if a + // external processing server. This overrides the above allowedHeaders if a // header matches both. disallowedHeaders []matcher.StringMatcher // disableImmediateResponse specifies whether to disable immediate response @@ -109,61 +128,19 @@ type processingModes struct { responseBodyMode processingMode } -// headerMutationRules specifies the rules for what modifications an external -// processing server may make to headers sent on the data plane RPC. -type headerMutationRules struct { - // allowExpr specifies a regular expression that matches the headers that can - // be mutated. - allowExpr *regexp.Regexp - // disallowExpr specifies a regular expression that matches the headers that - // cannot be mutated. This overrides the above allowExpr if a header matches - // both. - disallowExpr *regexp.Regexp - // disallowAll specifies that no header mutations are allowed. This overrides - // all other settings. - disallowAll bool - // disallowIsError specifies whether to return an error if a header mutation - // is disallowed. If true, the data plane RPC will be failed with a grpc - // status code of Unknown. - disallowIsError bool -} - -// serverConfig contains the configuration for an external server. -type serverConfig struct { - // targetURI is the name of the external server. - targetURI string - // channelCredentials specifies the transport credentials to use to connect to - // the external server. Must not be nil. - channelCredentials credentials.TransportCredentials - // callCredentials specifies the per-RPC credentials to use when making calls - // to the external server. - callCredentials []credentials.PerRPCCredentials - // timeout is the RPC timeout for the call to the external server. If unset, - // the timeout depends on the usage of this external server. For example, - // cases like ext_authz and ext_proc, where there is a 1:1 mapping between the - // data plane RPC and the external server call, the timeout will be capped by - // the timeout on the data plane RPC. For cases like RLQS where there is a - // side channel to the external server, an unset timeout will result in no - // timeout being applied to the external server call. - timeout time.Duration - // initialMetadata is the additional metadata to include in all RPCs sent to - // the external server. - initialMetadata metadata.MD -} - // newInterceptorConfig creates the interceptor config from the base and // override filter configs. The base config is required and the override config // is optional. If a field is set in both the base and override configs, the // value from the override config will be used. -func newInterceptorConfig(base, override interceptorConfig) interceptorConfig { +func newInterceptorConfig(base interceptorConfig, override interceptorOverrideConfig) interceptorConfig { ic := base // Apply overrides if present. - if override.server != nil { - ic.server = override.server + if val, ok := override.server.Value(); ok { + ic.server = val } - if override.failureModeAllow != nil { - ic.failureModeAllow = override.failureModeAllow + if val, ok := override.failureModeAllow.Value(); ok { + ic.failureModeAllow = val } if override.requestAttributes != nil { ic.requestAttributes = override.requestAttributes @@ -171,8 +148,8 @@ func newInterceptorConfig(base, override interceptorConfig) interceptorConfig { if override.responseAttributes != nil { ic.responseAttributes = override.responseAttributes } - if override.processingModes != nil { - ic.processingModes = override.processingModes + if val, ok := override.processingModes.Value(); ok { + ic.processingModes = val } return ic } diff --git a/internal/xds/httpfilter/extproc/ext_proc.go b/internal/xds/httpfilter/extproc/ext_proc.go index 3b2ac389cfa3..42e05babb396 100644 --- a/internal/xds/httpfilter/extproc/ext_proc.go +++ b/internal/xds/httpfilter/extproc/ext_proc.go @@ -41,6 +41,10 @@ type clientFilter struct{} func (clientFilter) Close() {} +var createExtProcChannel = func(httpfilter.ServerConfig) (*grpc.ClientConn, error) { + return nil, fmt.Errorf("dialing external processing server with raw JSON credentials is not yet supported") +} + func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig) (resolver.ClientInterceptor, error) { if cfg == nil { return nil, fmt.Errorf("extproc: nil config provided") @@ -62,11 +66,7 @@ func (clientFilter) BuildClientInterceptor(cfg, override httpfilter.FilterConfig config := newInterceptorConfig(c.config, ov.config) // Create a channel to the external processing server. - dOpts := []grpc.DialOption{grpc.WithTransportCredentials(config.server.channelCredentials)} - for _, creds := range config.server.callCredentials { - dOpts = append(dOpts, grpc.WithPerRPCCredentials(creds)) - } - cc, err := grpc.NewClient(config.server.targetURI, dOpts...) + cc, err := createExtProcChannel(config.server) if err != nil { return nil, fmt.Errorf("extproc: failed to create client: %v", err) } diff --git a/internal/xds/httpfilter/extproc/ext_proc_test.go b/internal/xds/httpfilter/extproc/ext_proc_test.go index eea1e82c7e16..13b33c1418f1 100644 --- a/internal/xds/httpfilter/extproc/ext_proc_test.go +++ b/internal/xds/httpfilter/extproc/ext_proc_test.go @@ -25,7 +25,9 @@ import ( "time" "github.com/google/go-cmp/cmp" + "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/experimental/optional" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/xds/httpfilter" "google.golang.org/grpc/internal/xds/matcher" @@ -50,6 +52,11 @@ func Test(t *testing.T) { } func (s) TestBuildClientInterceptor(t *testing.T) { + origCreateExtProcChannel := createExtProcChannel + defer func() { createExtProcChannel = origCreateExtProcChannel }() + createExtProcChannel = func(cfg httpfilter.ServerConfig) (*grpc.ClientConn, error) { + return grpc.NewClient(cfg.TargetURI, grpc.WithTransportCredentials(insecure.NewCredentials())) + } b := builder{} f := b.BuildClientFilter() @@ -82,55 +89,55 @@ func (s) TestBuildClientInterceptor(t *testing.T) { name: "ConfigUsingOnlyBase", cfg: baseConfig{ config: interceptorConfig{ - failureModeAllow: func() *bool { b := true; return &b }(), + failureModeAllow: true, requestAttributes: []string{"attr1"}, responseAttributes: []string{"attr2"}, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: &processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSkip, responseTrailerMode: modeSend, requestBodyMode: modeSend, responseBodyMode: modeSkip, }, - server: &serverConfig{ - targetURI: testBaseURI, - channelCredentials: insecure.NewCredentials(), + server: httpfilter.ServerConfig{ + TargetURI: testBaseURI, + ChannelCredentials: []byte("{}"), }, - mutationRules: headerMutationRules{ - allowExpr: regexp.MustCompile("allow-.*"), - disallowExpr: regexp.MustCompile("disallow-.*"), - disallowAll: true, - disallowIsError: true, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, }, }, wantConfig: &interceptorConfig{ - failureModeAllow: func() *bool { b := true; return &b }(), + failureModeAllow: true, requestAttributes: []string{"attr1"}, responseAttributes: []string{"attr2"}, - mutationRules: headerMutationRules{ - allowExpr: regexp.MustCompile("allow-.*"), - disallowExpr: regexp.MustCompile("disallow-.*"), - disallowAll: true, - disallowIsError: true, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, }, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: &processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSkip, responseTrailerMode: modeSend, requestBodyMode: modeSend, responseBodyMode: modeSkip, }, - server: &serverConfig{ - targetURI: testBaseURI, - channelCredentials: insecure.NewCredentials(), + server: httpfilter.ServerConfig{ + TargetURI: testBaseURI, + ChannelCredentials: []byte("{}"), }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, }, @@ -139,76 +146,144 @@ func (s) TestBuildClientInterceptor(t *testing.T) { name: "ConfigUsingBaseAndOverride", cfg: baseConfig{ config: interceptorConfig{ - failureModeAllow: func() *bool { b := false; return &b }(), + failureModeAllow: false, requestAttributes: []string{"base-attr1"}, responseAttributes: []string{"base-attr2"}, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: &processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSend, responseHeaderMode: modeSkip, responseTrailerMode: modeSend, requestBodyMode: modeSend, responseBodyMode: modeSkip, }, - server: &serverConfig{ - targetURI: testBaseURI, - channelCredentials: insecure.NewCredentials(), - timeout: time.Second, - initialMetadata: metadata.MD(metadata.Pairs("key1", "value1")), + server: httpfilter.ServerConfig{ + TargetURI: testBaseURI, + ChannelCredentials: []byte("{}"), + Timeout: time.Second, + InitialMetadata: metadata.MD(metadata.Pairs("key1", "value1")), }, - mutationRules: headerMutationRules{ - allowExpr: regexp.MustCompile("allow-.*"), - disallowExpr: regexp.MustCompile("disallow-.*"), - disallowAll: true, - disallowIsError: true, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, }, }, override: overrideConfig{ - config: interceptorConfig{ - failureModeAllow: func() *bool { b := true; return &b }(), + config: interceptorOverrideConfig{ + failureModeAllow: optional.NewValue(true), requestAttributes: []string{"override-attr1"}, responseAttributes: []string{"override-attr2"}, - processingModes: &processingModes{ + processingModes: optional.NewValue(processingModes{ requestHeaderMode: modeSkip, responseHeaderMode: modeSend, responseTrailerMode: modeSkip, requestBodyMode: modeSkip, responseBodyMode: modeSend, - }, - server: &serverConfig{ - targetURI: "override-uri", - channelCredentials: insecure.NewCredentials(), - }, + }), + server: optional.NewValue(httpfilter.ServerConfig{ + TargetURI: "override-uri", + ChannelCredentials: []byte("{}"), + }), }, }, wantConfig: &interceptorConfig{ - failureModeAllow: func() *bool { b := true; return &b }(), + failureModeAllow: true, requestAttributes: []string{"override-attr1"}, responseAttributes: []string{"override-attr2"}, - mutationRules: headerMutationRules{ - allowExpr: regexp.MustCompile("allow-.*"), - disallowExpr: regexp.MustCompile("disallow-.*"), - disallowAll: true, - disallowIsError: true, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, }, observabilityMode: true, disableImmediateResponse: true, deferredCloseTimeout: 10 * time.Second, - processingModes: &processingModes{ + processingModes: processingModes{ requestHeaderMode: modeSkip, responseHeaderMode: modeSend, responseTrailerMode: modeSkip, requestBodyMode: modeSkip, responseBodyMode: modeSend, }, - server: &serverConfig{ - targetURI: "override-uri", - channelCredentials: insecure.NewCredentials(), + server: httpfilter.ServerConfig{ + TargetURI: "override-uri", + ChannelCredentials: []byte("{}"), + }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, + }, + }, + { + name: "ConfigUsingBaseAndPartialOverride", + cfg: baseConfig{ + config: interceptorConfig{ + failureModeAllow: false, + requestAttributes: []string{"base-attr1"}, + responseAttributes: []string{"base-attr2"}, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingModes: processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSkip, + responseTrailerMode: modeSend, + requestBodyMode: modeSend, + responseBodyMode: modeSkip, + }, + server: httpfilter.ServerConfig{ + TargetURI: testBaseURI, + ChannelCredentials: []byte("{}"), + Timeout: time.Second, + InitialMetadata: metadata.MD(metadata.Pairs("key1", "value1")), + }, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, + }, + allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, + disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, + }, + }, + override: overrideConfig{ + config: interceptorOverrideConfig{ + failureModeAllow: optional.NewValue(true), + }, + }, + wantConfig: &interceptorConfig{ + failureModeAllow: true, + requestAttributes: []string{"base-attr1"}, + responseAttributes: []string{"base-attr2"}, + mutationRules: httpfilter.HeaderMutationRules{ + AllowExpr: regexp.MustCompile("allow-.*"), + DisallowExpr: regexp.MustCompile("disallow-.*"), + DisallowAll: true, + DisallowIsError: true, + }, + observabilityMode: true, + disableImmediateResponse: true, + deferredCloseTimeout: 10 * time.Second, + processingModes: processingModes{ + requestHeaderMode: modeSend, + responseHeaderMode: modeSkip, + responseTrailerMode: modeSend, + requestBodyMode: modeSend, + responseBodyMode: modeSkip, + }, + server: httpfilter.ServerConfig{ + TargetURI: testBaseURI, + ChannelCredentials: []byte("{}"), + Timeout: time.Second, + InitialMetadata: metadata.MD(metadata.Pairs("key1", "value1")), }, allowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("allow-header", false)}, disallowedHeaders: []matcher.StringMatcher{matcher.NewExactStringMatcher("disallow-header", false)}, @@ -227,7 +302,7 @@ func (s) TestBuildClientInterceptor(t *testing.T) { t.Fatalf("BuildClientInterceptor() returned %T, want *interceptor", intptr) } cmpOpts := []cmp.Option{ - cmp.AllowUnexported(interceptorConfig{}, serverConfig{}, processingModes{}, headerMutationRules{}), + cmp.AllowUnexported(interceptorConfig{}, processingModes{}), cmp.Transformer("RegexpToString", func(r *regexp.Regexp) string { if r == nil { return ""