diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index ad6923f460..761d938b45 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -1040,7 +1040,7 @@ func processConfigMaps(kubeClient *kubernetes.Clientset, cfgParams *configs.Conf if err != nil { nl.Fatalf(l, "Error when getting %v: %v", *nginxConfigMaps, err) } - cfgParams, _ = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, *enableDirectiveAutoadjust, eventLog) + cfgParams, _ = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, *enableDirectiveAutoadjust, *enableSnippets, eventLog) if cfgParams.MainServerSSLDHParamFileContent != nil { fileName, err := nginxManager.CreateDHParam(*cfgParams.MainServerSSLDHParamFileContent) if err != nil { diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go index 6ad24bd6e6..fd34c731f8 100644 --- a/internal/configs/config_params.go +++ b/internal/configs/config_params.go @@ -31,6 +31,7 @@ type ConfigParams struct { LocationSnippets []string MainAccessLog string MainAddHeaders []version2.AddHeader + DisableForwardedHeaders bool MainErrorLogLevel string MainHTTPSnippets []string MainKeepaliveRequests int64 @@ -283,6 +284,7 @@ func NewDefaultConfigParams(ctx context.Context, isPlus bool) *ConfigParams { MainKeepaliveRequests: 1000, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + DisableForwardedHeaders: false, LimitReqKey: "${binary_remote_addr}", LimitReqZoneSize: "10m", LimitReqLogLevel: "error", diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index abd9ee0332..63ad84478f 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -31,7 +31,7 @@ const ( // ParseConfigMap parses ConfigMap into ConfigParams. // //nolint:gocyclo -func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, enableDirectiveAutoadjust bool, eventLog record.EventRecorder) (*ConfigParams, bool) { +func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, enableDirectiveAutoadjust bool, enableSnippets bool, eventLog record.EventRecorder) (*ConfigParams, bool) { l := nl.LoggerFromContext(ctx) cfgParams := NewDefaultConfigParams(ctx, nginxPlus) configOk := true @@ -113,6 +113,21 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has } } + if disableForwardedHeaders, exists, err := GetMapKeyAsBool(cfgm.Data, "disable-forwarded-headers", cfgm); exists { + if !enableSnippets { + errorText := fmt.Sprintf("ConfigMap %s/%s: 'disable-forwarded-headers' requires -enable-snippets, ignoring", cfgm.GetNamespace(), cfgm.GetName()) + nl.Error(l, errorText) + eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText) + configOk = false + } else if err != nil { + nl.Error(l, err) + eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, err.Error()) + configOk = false + } else { + cfgParams.DisableForwardedHeaders = disableForwardedHeaders + } + } + if clientMaxBodySize, exists := cfgm.Data["client-max-body-size"]; exists { cfgParams.ClientMaxBodySize = clientMaxBodySize } diff --git a/internal/configs/configmaps_test.go b/internal/configs/configmaps_test.go index d3a04acdb6..6ac3cd8168 100644 --- a/internal/configs/configmaps_test.go +++ b/internal/configs/configmaps_test.go @@ -53,7 +53,7 @@ func TestParseConfigMapWithAppProtectCompressedRequestsAction(t *testing.T) { "app-protect-compressed-requests-action": test.action, }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.MainAppProtectCompressedRequestsAction != test.expect { t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectCompressedRequestsAction, test.expect, test.msg) } @@ -123,7 +123,7 @@ func TestParseConfigMapWithAppProtectReconnectPeriod(t *testing.T) { "app-protect-reconnect-period-seconds": test.period, }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.MainAppProtectReconnectPeriod != test.expect { t.Errorf("ParseConfigMap() returned %q but expected %q for the case %s", result.MainAppProtectReconnectPeriod, test.expect, test.msg) } @@ -165,7 +165,7 @@ func TestParseConfigMapWithTLSPassthroughProxyProtocol(t *testing.T) { "real-ip-header": test.realIPheader, }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.RealIPHeader != test.want { t.Errorf("want %q, got %q", test.want, result.RealIPHeader) } @@ -208,7 +208,7 @@ func TestParseConfigMapWithoutTLSPassthroughProxyProtocol(t *testing.T) { "real-ip-header": test.realIPheader, }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.RealIPHeader != test.want { t.Errorf("want %q, got %q", test.want, result.RealIPHeader) } @@ -256,7 +256,7 @@ func TestParseConfigMapAccessLog(t *testing.T) { "access-log-off": test.accessLogOff, }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.MainAccessLog != test.want { t.Errorf("want %q, got %q", test.want, result.MainAccessLog) } @@ -289,7 +289,7 @@ func TestParseConfigMapAccessLogDefault(t *testing.T) { "access-log-off": "False", }, } - result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.MainAccessLog != test.want { t.Errorf("want %q, got %q", test.want, result.MainAccessLog) } @@ -470,7 +470,7 @@ func TestParseConfigMapOIDC(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if !configOk { t.Error("want configOk true, got configOk false") } @@ -639,7 +639,7 @@ func TestParseConfigMapOIDCErrors(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - _, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + _, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if test.expectedErr && configOk { t.Errorf("want configOk false, got configOk true for %s", test.msg) @@ -1595,7 +1595,7 @@ func TestParseZoneSync(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - result, _ := ParseConfigMap(context.Background(), test.configMap, true, false, false, false, true, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), test.configMap, true, false, false, false, true, true, makeEventLogger()) if result.ZoneSync.Enable != test.want.Enable { t.Errorf("Enable: want %v, got %v", test.want.Enable, result.ZoneSync) } @@ -1638,7 +1638,7 @@ func TestParseZoneSyncForOSS(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - _, configOk := ParseConfigMap(context.Background(), test.configMap, false, false, false, false, true, makeEventLogger()) + _, configOk := ParseConfigMap(context.Background(), test.configMap, false, false, false, false, true, true, makeEventLogger()) if configOk { t.Errorf("Expected config not valid, got valid") } @@ -1679,7 +1679,7 @@ func TestParseZoneSyncPort(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - result, _ := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, _ := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if result.ZoneSync.Port != test.want.Port { t.Errorf("Port: want %v, got %v", test.want.Port, result.ZoneSync.Port) } @@ -1714,7 +1714,7 @@ func TestZoneSyncPortSetToDefaultOnZoneSyncEnabledAndPortNotProvided(t *testing. directiveAutoadjustEnabled := false for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if !configOk { t.Error("zone-sync: want configOk true, got configOk false ") } @@ -1786,7 +1786,7 @@ func TestParseZoneSyncPortErrors(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - _, ok := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + _, ok := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if ok { t.Error("Expected config not valid, got valid") } @@ -1863,7 +1863,7 @@ func TestParseZoneSyncResolverErrors(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - _, ok := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + _, ok := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if ok { t.Error("Expected config not valid, got valid") } @@ -1921,7 +1921,7 @@ func TestParseZoneSyncResolverIPV6MapResolverIPV6(t *testing.T) { hasTLSPassthrough := false directiveAutoadjustEnabled := false - result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if !configOk { t.Errorf("zone-sync-resolver-ipv6: want configOk true, got configOk %v ", configOk) @@ -2035,7 +2035,7 @@ func TestOpenTelemetryConfigurationSuccess(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { result, configOk := ParseConfigMap(context.Background(), test.configMap, isPlus, - hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if configOk != expectedConfigOk { t.Errorf("configOk: want %v, got %v", expectedConfigOk, configOk) } @@ -2267,7 +2267,7 @@ func TestOpenTelemetryConfigurationInvalid(t *testing.T) { for _, test := range tests { t.Run(test.msg, func(t *testing.T) { result, configOk := ParseConfigMap(context.Background(), test.configMap, isPlus, - hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if configOk != expectedConfigOk { t.Errorf("configOk: want %v, got %v", expectedConfigOk, configOk) } @@ -2349,7 +2349,7 @@ func TestParseProxyBuffers(t *testing.T) { t.Parallel() eventRecorder := makeEventLogger() - result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, eventRecorder) + result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, eventRecorder) if !configOk { t.Errorf("%s: expected config to be valid but got invalid", test.description) @@ -2428,7 +2428,7 @@ func TestParseProxyBuffers(t *testing.T) { t.Parallel() eventRecorder := makeEventLogger() - result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, eventRecorder) + result, configOk := ParseConfigMap(context.Background(), test.configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, eventRecorder) if !configOk { t.Errorf("%s: expected config to be valid but got invalid", test.description) @@ -2521,7 +2521,7 @@ func TestParseProxyBuffersInvalidFormat(t *testing.T) { } eventRecorder := makeEventLogger() - result, configOk := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, eventRecorder) + result, configOk := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, eventRecorder) if configOk != test.expectValid { t.Errorf("%s: expected configOk=%v, got configOk=%v", test.description, test.expectValid, configOk) @@ -2580,7 +2580,7 @@ func TestParseProxyBuffersInvalidFormat(t *testing.T) { } eventRecorder := makeEventLogger() - result, configOk := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, eventRecorder) + result, configOk := ParseConfigMap(context.Background(), cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, eventRecorder) // When auto-adjust is disabled, config should always be valid since no validation occurs if !configOk { @@ -2680,6 +2680,7 @@ func TestParseConfigMapClientBodyBufferSizeValid(t *testing.T) { hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, + true, makeEventLogger(), ) @@ -2779,6 +2780,7 @@ func TestParseConfigMapClientBodyBufferSizeInvalid(t *testing.T) { hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, + true, makeEventLogger(), ) @@ -2814,7 +2816,7 @@ func TestParseErrorLogLevelToVirtualServer(t *testing.T) { eventRecorder := makeEventLogger() - result, configOk := ParseConfigMap(context.Background(), cm, true, false, false, false, false, eventRecorder) + result, configOk := ParseConfigMap(context.Background(), cm, true, false, false, false, false, true, eventRecorder) if !configOk { t.Errorf("expected config map with error-log-level set to be %s to be valid", testLevel) @@ -2974,7 +2976,7 @@ func TestParseConfigMapWithHTTPRedirectCode(t *testing.T) { Data: test.configMap, } - result, configOK := ParseConfigMap(context.Background(), configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, makeEventLogger()) + result, configOK := ParseConfigMap(context.Background(), configMap, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough, directiveAutoadjustEnabled, true, makeEventLogger()) if test.expectError { assert.False(t, configOK, test.msg) @@ -3081,7 +3083,7 @@ func TestParseConfigMapAddHeader(t *testing.T) { t.Parallel() cm := &v1.ConfigMap{Data: tc.data} result, configOk := ParseConfigMap(context.Background(), cm, - false, false, false, false, false, makeEventLogger()) + false, false, false, false, false, true, makeEventLogger()) if configOk != tc.wantConfigOk { t.Errorf("configOk: want %v, got %v", tc.wantConfigOk, configOk) @@ -3201,7 +3203,7 @@ func TestParseConfigMapWithAddHeaderInherit(t *testing.T) { Data: map[string]string{}, } cm.Data["add-header-inherit"] = test.value - result, configOK := ParseConfigMap(context.Background(), cm, false, false, false, false, false, makeEventLogger()) + result, configOK := ParseConfigMap(context.Background(), cm, false, false, false, false, false, true, makeEventLogger()) if result.AddHeaderInherit != test.expect { t.Errorf("ParseConfigMap() returned AddHeaderInherit=%q but expected %q for the case: %s", result.AddHeaderInherit, test.expect, test.msg) } @@ -3213,3 +3215,64 @@ func TestParseConfigMapWithAddHeaderInherit(t *testing.T) { } } } + +func TestParseConfigMapDisableForwardedHeaders(t *testing.T) { + t.Parallel() + tests := []struct { + msg string + data map[string]string + enableSnippets bool + wantDisabled bool + wantConfigOk bool + }{ + { + msg: "disable-forwarded-headers true with snippets enabled", + data: map[string]string{"disable-forwarded-headers": "true"}, + enableSnippets: true, + wantDisabled: true, + wantConfigOk: true, + }, + { + msg: "disable-forwarded-headers false with snippets enabled", + data: map[string]string{"disable-forwarded-headers": "false"}, + enableSnippets: true, + wantDisabled: false, + wantConfigOk: true, + }, + { + msg: "disable-forwarded-headers true with snippets disabled", + data: map[string]string{"disable-forwarded-headers": "true"}, + enableSnippets: false, + wantDisabled: false, + wantConfigOk: false, + }, + { + msg: "disable-forwarded-headers invalid bool with snippets enabled", + data: map[string]string{"disable-forwarded-headers": "notabool"}, + enableSnippets: true, + wantDisabled: false, + wantConfigOk: false, + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + t.Parallel() + cm := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "default", + }, + Data: tc.data, + } + recorder := makeEventLogger() + cfgParams, configOk := ParseConfigMap(context.Background(), cm, false, false, false, false, false, tc.enableSnippets, recorder) + if configOk != tc.wantConfigOk { + t.Errorf("configOk: want %v, got %v", tc.wantConfigOk, configOk) + } + if cfgParams.DisableForwardedHeaders != tc.wantDisabled { + t.Errorf("DisableForwardedHeaders: want %v, got %v", tc.wantDisabled, cfgParams.DisableForwardedHeaders) + } + }) + } +} diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 2896753fd0..77378a0163 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -1069,6 +1069,7 @@ func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, ProxyBufferSize: cfg.ProxyBufferSize, ProxyBusyBuffersSize: cfg.ProxyBusyBuffersSize, ProxyMaxTempFileSize: cfg.ProxyMaxTempFileSize, + DisableForwardedHeaders: cfg.DisableForwardedHeaders, ProxySSLName: proxySSLName, ProxyNextUpstream: cfg.ProxyNextUpstream, ProxyNextUpstreamTimeout: cfg.ProxyNextUpstreamTimeout, diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go index b99c33f1af..cddaaa8ef5 100644 --- a/internal/configs/ingress_test.go +++ b/internal/configs/ingress_test.go @@ -2993,9 +2993,10 @@ func createExpectedConfigForCafeIngressWithUseClusterIPNamedPorts() version1.Ing ProxyReadTimeout: "60s", ProxySendTimeout: "60s", ClientMaxBodySize: "1m", - ProxyBuffering: true, - ProxySSLName: "coffee-svc.default.svc", - ProxyPass: "http://default-cafe-ingress-cafe.example.com-coffee-svc-custom-port-name", + + ProxyBuffering: true, + ProxySSLName: "coffee-svc.default.svc", + ProxyPass: "http://default-cafe-ingress-cafe.example.com-coffee-svc-custom-port-name", }, { Path: "/tea", diff --git a/internal/configs/version1/__snapshots__/template_test.snap b/internal/configs/version1/__snapshots__/template_test.snap index 40fa13e786..ec71e6c40c 100644 --- a/internal/configs/version1/__snapshots__/template_test.snap +++ b/internal/configs/version1/__snapshots__/template_test.snap @@ -5428,6 +5428,53 @@ server { --- +[TestExecuteTemplate_ForIngressWithDisableForwardedHeaders - 1] +# configuration for default/cafe-ingress +upstream test { + zone test 256k; + server 127.0.0.1:8181 max_fails=0 fail_timeout=1s max_conns=0; + keepalive 16; +} + + + +server { + listen 443 ssl;listen [::]:443 ssl; + ssl_certificate secret.pem; + ssl_certificate_key secret.pem; + + server_tokens off; + + server_name test.example.com; + set $resource_type "ingress"; + set $resource_name "cafe-ingress"; + set $resource_namespace "default"; + set $service "-"; + if ($scheme = http) { + return 301 https://$host:443$request_uri; + } + location /tea { + set $service ""; + # location for minion default/tea-minion + set $resource_name "tea-minion"; + set $resource_namespace "default"; + proxy_http_version 1.1; + proxy_set_header Connection ""; + proxy_connect_timeout 10s; + proxy_read_timeout 10s; + proxy_send_timeout 10s; + client_max_body_size 2m; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_buffering off; + proxy_pass http://test; + + } + +} + +--- + [TestExecuteTemplate_ForIngressWithEmptyHostForNGINX - 1] # configuration for default/cafe-ingress upstream test { diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index 8bde790dc6..479c781045 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -229,6 +229,7 @@ type Location struct { BasicAuth *BasicAuth ServiceName string LimitReq *LimitReq + DisableForwardedHeaders bool CORSEnabled bool AuthRequestOff bool diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index b898318348..26ad6656db 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -424,10 +424,12 @@ server { grpc_send_timeout {{$location.ProxySendTimeout}}; grpc_set_header Host $host; grpc_set_header X-Real-IP $remote_addr; + {{- if not $location.DisableForwardedHeaders }} grpc_set_header X-Forwarded-For $proxy_add_x_forwarded_for; grpc_set_header X-Forwarded-Host $host; grpc_set_header X-Forwarded-Port $server_port; grpc_set_header X-Forwarded-Proto $scheme; + {{- end}} {{- if $location.ProxyBufferSize}} grpc_buffer_size {{$location.ProxyBufferSize}}; @@ -482,10 +484,12 @@ server { {{- end}} proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + {{- if not $location.DisableForwardedHeaders }} proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; proxy_set_header X-Forwarded-Port $server_port; proxy_set_header X-Forwarded-Proto {{if $server.RedirectToHTTPS}}https{{else}}$scheme{{end}}; + {{- end}} proxy_buffering {{if $location.ProxyBuffering}}on{{else}}off{{end}}; {{- if $location.ProxyBuffers}} proxy_buffers {{$location.ProxyBuffers}}; diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index e431f73e6c..a864f8eff0 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -326,11 +326,12 @@ server { grpc_send_timeout {{$location.ProxySendTimeout}}; grpc_set_header Host $host; grpc_set_header X-Real-IP $remote_addr; + {{- if not $location.DisableForwardedHeaders }} grpc_set_header X-Forwarded-For $proxy_add_x_forwarded_for; grpc_set_header X-Forwarded-Host $host; grpc_set_header X-Forwarded-Port $server_port; grpc_set_header X-Forwarded-Proto {{if $server.RedirectToHTTPS}}https{{else}}$scheme{{end}}; - + {{- end}} {{- if $location.ProxyBufferSize}} grpc_buffer_size {{$location.ProxyBufferSize}}; {{- end}} @@ -374,10 +375,12 @@ server { {{- end}} proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; + {{- if not $location.DisableForwardedHeaders }} proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; proxy_set_header X-Forwarded-Port $server_port; proxy_set_header X-Forwarded-Proto {{if $server.RedirectToHTTPS}}https{{else}}$scheme{{end}}; + {{- end}} proxy_buffering {{if $location.ProxyBuffering}}on{{else}}off{{end}}; {{- if $location.ProxyBuffers}} proxy_buffers {{$location.ProxyBuffers}}; diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 5c1ef50562..a000024a7f 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -4039,6 +4039,34 @@ func TestExecuteTemplate_ForIngressWithAddHeaderInherit(t *testing.T) { } } +func TestExecuteTemplate_ForIngressWithDisableForwardedHeaders(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + buf := &bytes.Buffer{} + + err := tmpl.Execute(buf, ingressCfgForwardedHeaderEnabled) + t.Log(buf.String()) + if err != nil { + t.Fatal(err) + } + + notWantDirectives := []string{ + "proxy_set_header X-Forwarded-For", + "proxy_set_header X-Forwarded-Host", + "proxy_set_header X-Forwarded-Port", + "proxy_set_header X-Forwarded-Proto", + } + + rendered := buf.String() + for _, notWant := range notWantDirectives { + if strings.Contains(rendered, notWant) { + t.Errorf("not want %q in generated config", notWant) + } + } + snaps.MatchSnapshot(t, buf.String()) +} + var ( // Ingress Config example without added annotations ingressCfg = IngressNginxConfig{ @@ -4425,6 +4453,44 @@ var ( }, } + ingressCfgForwardedHeaderEnabled = IngressNginxConfig{ + Servers: []Server{ + { + Name: "test.example.com", + ServerTokens: "off", + StatusZone: "test.example.com", + SSL: true, + SSLCertificate: "secret.pem", + SSLCertificateKey: "secret.pem", + SSLPorts: []int{443}, + SSLRedirect: true, + HTTPRedirectCode: 301, + Locations: []Location{ + { + Path: "/tea", + Upstream: testUpstream, + ProxyConnectTimeout: "10s", + DisableForwardedHeaders: true, + ProxyReadTimeout: "10s", + ProxySendTimeout: "10s", + ClientMaxBodySize: "2m", + MinionIngress: &Ingress{ + Name: "tea-minion", + Namespace: "default", + }, + ProxyPass: "http://test", + }, + }, + }, + }, + Upstreams: []Upstream{testUpstream}, + Keepalive: "16", + Ingress: Ingress{ + Name: "cafe-ingress", + Namespace: "default", + }, + } + // Ingress Config example with ssl-redirect and redirect-to-https enabled with custom http-redirect-code ingressCfgWithHTTPRedirectCode = IngressNginxConfig{ Servers: []Server{ diff --git a/internal/configs/version2/__snapshots__/templates_test.snap b/internal/configs/version2/__snapshots__/templates_test.snap index 347687a43e..28324cccc7 100644 --- a/internal/configs/version2/__snapshots__/templates_test.snap +++ b/internal/configs/version2/__snapshots__/templates_test.snap @@ -3101,6 +3101,52 @@ server { --- +[TestExecuteVirtualServerTemplate_RendersTemplateWithDisableForwardedHeadersTrue - 1] + + +server { + listen 80; + listen [::]:80; + + + server_name example.com; + status_zone example.com; + set $resource_type "virtualserver"; + set $resource_name ""; + set $resource_namespace ""; + set $service "-"; + + server_tokens ""; + + + + + location / { + set $service ""; + status_zone ""; + + + set $default_connection_header close; + proxy_connect_timeout ; + proxy_read_timeout ; + proxy_send_timeout ; + client_max_body_size ; + + proxy_buffering off; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $vs_connection_header; + proxy_pass_request_headers off; + proxy_set_header X-Real-IP $remote_addr; + proxy_pass http://test-upstream; + proxy_next_upstream ; + proxy_next_upstream_timeout ; + proxy_next_upstream_tries 0; + } +} + +--- + [TestExecuteVirtualServerTemplate_RendersTemplateWithRateLimitJWTClaim - 1] auth_jwt_claim_set $jwt_default_webapp_group_consumer_group_type consumer_group type; diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 94ec13384a..159f39926b 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -250,6 +250,7 @@ type Location struct { VSRNamespace string GRPCPass string CORSEnabled bool + DisableForwardedHeaders bool AddHeaderInherit string ProxySSLVerify bool ProxySSLVerifyDepth int diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 5bd1036578..5df3372482 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -756,7 +756,7 @@ server { {{- if not ($custom_headers | hasCIKey "X-Real-IP") }} {{ $proxyOrGRPC }}_set_header X-Real-IP $remote_addr; {{- end }} - + {{- if not $l.DisableForwardedHeaders }} {{- if not ($custom_headers | hasCIKey "X-Forwarded-For") }} {{ $proxyOrGRPC }}_set_header X-Forwarded-For $proxy_add_x_forwarded_for; {{- end }} @@ -772,6 +772,7 @@ server { {{- if not ($custom_headers | hasCIKey "X-Forwarded-Proto") }} {{ $proxyOrGRPC }}_set_header X-Forwarded-Proto {{ with $s.TLSRedirect }}{{ .BasedOn }}{{ else }}$scheme{{ end }}; {{- end }} + {{- end }} {{- range $h := $l.ProxySetHeaders }} {{ $proxyOrGRPC }}_set_header {{ $h.Name }} {{ printf "%q" $h.Value }}; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index bccc9b74c3..1ba814eaad 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -466,6 +466,7 @@ server { {{ $proxyOrGRPC }}_set_header X-Real-IP $remote_addr; {{- end }} + {{- if not $l.DisableForwardedHeaders }} {{- if not ($custom_headers | hasCIKey "X-Forwarded-For") }} {{ $proxyOrGRPC }}_set_header X-Forwarded-For $proxy_add_x_forwarded_for; {{- end }} @@ -481,6 +482,7 @@ server { {{- if not ($custom_headers | hasCIKey "X-Forwarded-Proto") }} {{ $proxyOrGRPC }}_set_header X-Forwarded-Proto {{ with $s.TLSRedirect }}{{ .BasedOn }}{{ else }}$scheme{{ end }}; {{- end }} + {{- end }} {{- range $h := $l.ProxySetHeaders }} {{ $proxyOrGRPC }}_set_header {{ $h.Name }} {{ printf "%q" $h.Value }}; diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 456aecaf23..f23f5c851d 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -537,6 +537,30 @@ func TestExecuteVirtualServerTemplate_RendersTemplateWithClientBodyBufferSize(t t.Log(string(got)) } +func TestExecuteVirtualServerTemplate_RendersTemplateWithDisableForwardedHeadersTrue(t *testing.T) { + t.Parallel() + executor := newTmplExecutorNGINXPlus(t) + + got, err := executor.ExecuteVirtualServerTemplate(&virtualServerCfgWithDisableForwardedHeadersTrue) + if err != nil { + t.Error(err) + } + if bytes.Contains(got, []byte("proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for")) { + t.Error("don't want `proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for` directive in generated template") + } + if bytes.Contains(got, []byte("proxy_set_header X-Forwarded-Host $host")) { + t.Error("don't want `proxy_set_header X-Forwarded-Host $host` directive in generated template") + } + if bytes.Contains(got, []byte("proxy_set_header X-Forwarded-Port $server_port")) { + t.Error("don't want `proxy_set_header X-Forwarded-Port $server_port` directive in generated template") + } + if bytes.Contains(got, []byte("proxy_set_header X-Forwarded-Proto $scheme")) { + t.Error("don't want `proxy_set_header X-Forwarded-Proto $scheme` directive in generated template") + } + snaps.MatchSnapshot(t, string(got)) + t.Log(string(got)) +} + func TestExecuteVirtualServerTemplate_RendersOSSTemplateWithHTTP2On(t *testing.T) { t.Parallel() executor := newTmplExecutorNGINX(t) @@ -2501,6 +2525,20 @@ var ( }, } + virtualServerCfgWithDisableForwardedHeadersTrue = VirtualServerConfig{ + Server: Server{ + ServerName: "example.com", + StatusZone: "example.com", + Locations: []Location{ + { + Path: "/", + ProxyPass: "http://test-upstream", + DisableForwardedHeaders: true, + }, + }, + }, + } + virtualServerCfgWithRateLimitJWTClaim = VirtualServerConfig{ LimitReqZones: []LimitReqZone{ { diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 2fe4d5b3d6..d3bdd4285d 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -2043,6 +2043,7 @@ func generateLocationForProxying(path string, upstreamName string, upstream conf ServiceName: serviceName, IsVSR: isVSR, VSRName: vsrName, + DisableForwardedHeaders: cfgParams.DisableForwardedHeaders, VSRNamespace: vsrNamespace, GRPCPass: generateGRPCPass(isGRPC(upstream.Type), upstream.TLS.Enable, upstreamName), } diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 82e8e0abc5..c43cee5968 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -1047,7 +1047,7 @@ func (lbc *LoadBalancerController) updateAllConfigs() { var reloadNginx bool if lbc.configMap != nil { - cfgParams, isNGINXConfigValid = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.configuration.isDirectiveAutoadjustEnabled, lbc.recorder) + cfgParams, isNGINXConfigValid = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.configuration.isDirectiveAutoadjustEnabled, lbc.configuration.snippetsEnabled, lbc.recorder) } if lbc.mgmtConfigMap != nil && lbc.isNginxPlus { mgmtCfgParams, mgmtConfigHasWarnings, mgmtErr = configs.ParseMGMTConfigMap(ctx, lbc.mgmtConfigMap, lbc.recorder) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 421a1b46fc..872c58804d 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -100,6 +100,7 @@ var configMapFilteredKeys = []string{ "zone-sync-resolver-addresses", "zone-sync-resolver-valid", "zone-sync-resolver-ipv6", + "disable-forwarded-headers", } var mgmtConfigMapFilteredKeys = []string{