Move proxy values balancing to the beginning of parsing

charts/nginx-ingress/templates/_helpers.tpl

@@ -304,6 +304,9 @@ Build the args for the service binary.
 - -enable-custom-resources={{ .Values.controller.enableCustomResources }}
 - -enable-snippets={{ .Values.controller.enableSnippets }}
 - -disable-ipv6={{ .Values.controller.disableIPV6 }}
+{{- if .Values.controller.directiveAutoAdjust }}
+- -enable-directive-autoadjust={{ .Values.controller.directiveAutoAdjust }}
+{{- end }}
 {{- if .Values.controller.enableCustomResources }}
 - -enable-tls-passthrough={{ .Values.controller.enableTLSPassthrough }}
 {{- if .Values.controller.enableTLSPassthrough }}

charts/nginx-ingress/values.schema.json

@@ -657,6 +657,15 @@
             "json"
           ]
         },
+        "directiveAutoAdjust": {
+          "type": "boolean",
+          "default": false,
+          "title": "Enables automatic adjustment of the NGINX buffers directives",
+          "examples": [
+            false,
+            true
+          ]
+        },
         "customPorts": {
           "type": "array",
           "default": [],

charts/nginx-ingress/values.yaml

@@ -164,6 +164,9 @@ controller:
   ## Sets the log format of Ingress Controller. Options include: glog, json, text
   logFormat: glog
 
+  ## Enables auto adjusting some of the NGINX directives to help with safe configuration and prevent NGINX misconfigurations.
+  directiveAutoAdjust: false
+
   ## Cache configuration options
   cache:
     ## Enables shared cache across multiple pods using an external persistent volume

cmd/nginx-ingress/flags.go

@@ -225,6 +225,8 @@ var (
 
 	enableDynamicWeightChangesReload = flag.Bool(dynamicWeightChangesParam, false, "Enable changing weights of split clients without reloading NGINX. Requires -nginx-plus")
 
+	enableDirectiveAutoadjust = flag.Bool("enable-directive-autoadjust", false, "Enable automatic adjustment of NGINX directives to make sure no conflicts will be present. This is an experimental feature and may not work as expected.")
+
 	startupCheckFn func() error
 )
 

cmd/nginx-ingress/main.go

@@ -265,6 +265,7 @@ func main() {
 		IsLatencyMetricsEnabled:             *enableLatencyMetrics,
 		IsDynamicSSLReloadEnabled:           *enableDynamicSSLReload,
 		IsDynamicWeightChangesReloadEnabled: *enableDynamicWeightChangesReload,
+		IsDirectiveAutoadjustEnabled:        *enableDirectiveAutoadjust,
 		NginxVersion:                        nginxVersion,
 	})
 
@@ -274,6 +275,7 @@ func main() {
 		cr_validation.IsDosEnabled(*appProtectDos),
 		cr_validation.IsCertManagerEnabled(*enableCertManager),
 		cr_validation.IsExternalDNSEnabled(*enableExternalDNS),
+		cr_validation.IsDirectiveAutoadjustEnabled(*enableDirectiveAutoadjust),
 	)
 
 	if *enableServiceInsight {
@@ -324,6 +326,7 @@ func main() {
 		CertManagerEnabled:           *enableCertManager,
 		ExternalDNSEnabled:           *enableExternalDNS,
 		IsIPV6Disabled:               *disableIPV6,
+		IsDirectiveAutoadjustEnabled: *enableDirectiveAutoadjust,
 		WatchNamespaceLabel:          *watchNamespaceLabel,
 		EnableTelemetryReporting:     *enableTelemetryReporting,
 		TelemetryReportingEndpoint:   telemetryEndpoint,
@@ -996,7 +999,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, eventLog)
+		cfgParams, _ = configs.ParseConfigMap(cfgParams.Context, cfm, *nginxPlus, *appProtect, *appProtectDos, *enableTLSPassthrough, *enableDirectiveAutoadjust, eventLog)
 		if cfgParams.MainServerSSLDHParamFileContent != nil {
 			fileName, err := nginxManager.CreateDHParam(*cfgParams.MainServerSSLDHParamFileContent)
 			if err != nil {

config/crd/bases/k8s.nginx.org_virtualserverroutes.yaml

@@ -872,6 +872,12 @@ spec:
                             is set in the proxy-buffers ConfigMap key.
                           type: string
                       type: object
+                    busy-buffers-size:
+                      description: Sets the size of the buffers used for reading a
+                        response from the upstream server when the proxy_buffering
+                        is enabled. The default is set in the proxy-busy-buffers-size
+                        ConfigMap key.'
+                      type: string
                     client-max-body-size:
                       description: Sets the maximum allowed size of the client request
                         body. The default is set in the client-max-body-size ConfigMap

config/crd/bases/k8s.nginx.org_virtualservers.yaml

@@ -1061,6 +1061,12 @@ spec:
                             is set in the proxy-buffers ConfigMap key.
                           type: string
                       type: object
+                    busy-buffers-size:
+                      description: Sets the size of the buffers used for reading a
+                        response from the upstream server when the proxy_buffering
+                        is enabled. The default is set in the proxy-busy-buffers-size
+                        ConfigMap key.'
+                      type: string
                     client-max-body-size:
                       description: Sets the maximum allowed size of the client request
                         body. The default is set in the client-max-body-size ConfigMap

deploy/crds.yaml

@@ -1891,6 +1891,12 @@ spec:
                             is set in the proxy-buffers ConfigMap key.
                           type: string
                       type: object
+                    busy-buffers-size:
+                      description: Sets the size of the buffers used for reading a
+                        response from the upstream server when the proxy_buffering
+                        is enabled. The default is set in the proxy-busy-buffers-size
+                        ConfigMap key.'
+                      type: string
                     client-max-body-size:
                       description: Sets the maximum allowed size of the client request
                         body. The default is set in the client-max-body-size ConfigMap
@@ -3303,6 +3309,12 @@ spec:
                             is set in the proxy-buffers ConfigMap key.
                           type: string
                       type: object
+                    busy-buffers-size:
+                      description: Sets the size of the buffers used for reading a
+                        response from the upstream server when the proxy_buffering
+                        is enabled. The default is set in the proxy-busy-buffers-size
+                        ConfigMap key.'
+                      type: string
                     client-max-body-size:
                       description: Sets the maximum allowed size of the client request
                         body. The default is set in the client-max-body-size ConfigMap

docs/crd/k8s.nginx.org_virtualserverroutes.md

@@ -168,6 +168,7 @@ The `.spec` object supports the following fields:
 | `upstreams[].buffers` | `object` | Configures the buffers used for reading a response from the upstream server for a single connection. |
 | `upstreams[].buffers.number` | `integer` | Configures the number of buffers. The default is set in the proxy-buffers ConfigMap key. |
 | `upstreams[].buffers.size` | `string` | Configures the size of a buffer. The default is set in the proxy-buffers ConfigMap key. |
+| `upstreams[].busy-buffers-size` | `string` | Sets the size of the buffers used for reading a response from the upstream server when the proxy_buffering is enabled. The default is set in the proxy-busy-buffers-size ConfigMap key.' |
 | `upstreams[].client-max-body-size` | `string` | Sets the maximum allowed size of the client request body. The default is set in the client-max-body-size ConfigMap key. |
 | `upstreams[].connect-timeout` | `string` | The timeout for establishing a connection with an upstream server. The default is specified in the proxy-connect-timeout ConfigMap key. |
 | `upstreams[].fail-timeout` | `string` | The time during which the specified number of unsuccessful attempts to communicate with an upstream server should happen to consider the server unavailable. The default is set in the fail-timeout ConfigMap key. |

docs/crd/k8s.nginx.org_virtualservers.md

@@ -203,6 +203,7 @@ The `.spec` object supports the following fields:
 | `upstreams[].buffers` | `object` | Configures the buffers used for reading a response from the upstream server for a single connection. |
 | `upstreams[].buffers.number` | `integer` | Configures the number of buffers. The default is set in the proxy-buffers ConfigMap key. |
 | `upstreams[].buffers.size` | `string` | Configures the size of a buffer. The default is set in the proxy-buffers ConfigMap key. |
+| `upstreams[].busy-buffers-size` | `string` | Sets the size of the buffers used for reading a response from the upstream server when the proxy_buffering is enabled. The default is set in the proxy-busy-buffers-size ConfigMap key.' |
 | `upstreams[].client-max-body-size` | `string` | Sets the maximum allowed size of the client request body. The default is set in the client-max-body-size ConfigMap key. |
 | `upstreams[].connect-timeout` | `string` | The timeout for establishing a connection with an upstream server. The default is specified in the proxy-connect-timeout ConfigMap key. |
 | `upstreams[].fail-timeout` | `string` | The time during which the specified number of unsuccessful attempts to communicate with an upstream server should happen to consider the server unavailable. The default is set in the fail-timeout ConfigMap key. |

internal/configs/annotations.go

@@ -6,6 +6,7 @@ import (
 	"slices"
 
 	nl "github.com/nginx/kubernetes-ingress/internal/logger"
+	"github.com/nginx/kubernetes-ingress/internal/validation"
 )
 
 // JWTKeyAnnotation is the annotation where the Secret with a JWK is specified.
@@ -74,6 +75,7 @@ var minionInheritanceList = map[string]bool{
 	"nginx.org/proxy-buffering":          true,
 	"nginx.org/proxy-buffers":            true,
 	"nginx.org/proxy-buffer-size":        true,
+	"nginx.org/proxy-busy-buffers-size":  true,
 	"nginx.org/proxy-max-temp-file-size": true,
 	"nginx.org/upstream-zone-size":       true,
 	"nginx.org/location-snippets":        true,
@@ -108,7 +110,8 @@ var allowedAnnotationKeys = []string{
 	"ingress.kubernetes.io/ssl-redirect",
 }
 
-func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool, hasAppProtect bool, hasAppProtectDos bool, enableInternalRoutes bool) ConfigParams {
+// nolint: gocyclo
+func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool, hasAppProtect bool, hasAppProtectDos bool, enableInternalRoutes bool, enableDirectiveAutoadjust bool) ConfigParams {
 	l := nl.LoggerFromContext(baseCfgParams.Context)
 	cfgParams := *baseCfgParams
 
@@ -296,12 +299,48 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
 		}
 	}
 
+	// Proxy Buffers uses number + size format, like "8 4k".
 	if proxyBuffers, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffers"]; exists {
-		cfgParams.ProxyBuffers = proxyBuffers
+		proxyBufferUnits, err := validation.NewNumberSizeConfig(proxyBuffers)
+		if err != nil {
+			nl.Errorf(l, "error parsing nginx.org/proxy-buffers: %s", err)
+		} else {
+			cfgParams.ProxyBuffers = proxyBufferUnits
+		}
 	}
 
+	// Proxy Buffer Size uses only size format, like "4k".
 	if proxyBufferSize, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffer-size"]; exists {
-		cfgParams.ProxyBufferSize = proxyBufferSize
+		proxyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBufferSize)
+		if err != nil {
+			nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
+		} else {
+			cfgParams.ProxyBufferSize = proxyBufferSizeUnit
+		}
+	}
+
+	// Proxy Busy Buffers Size uses only size format, like "8k".
+	if proxyBusyBuffersSize, exists := ingEx.Ingress.Annotations["nginx.org/proxy-busy-buffers-size"]; exists {
+		proxyBusyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBusyBuffersSize)
+		if err != nil {
+			nl.Errorf(l, "error parsing nginx.org/proxy-busy-buffers-size: %s", err)
+		} else {
+			cfgParams.ProxyBusyBuffersSize = proxyBusyBufferSizeUnit
+		}
+	}
+
+	balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
+	if err != nil {
+		nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
+	}
+	cfgParams.ProxyBuffers = balancedProxyBuffers
+	cfgParams.ProxyBufferSize = balancedProxyBufferSize
+	cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
+
+	if len(modifications) > 0 {
+		for _, modification := range modifications {
+			nl.Infof(l, "Changes made to proxy values: %s", modification)
+		}
 	}
 
 	if upstreamZoneSize, exists := ingEx.Ingress.Annotations["nginx.org/upstream-zone-size"]; exists {

internal/configs/config_params.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/nginx/kubernetes-ingress/internal/configs/version2"
 	"github.com/nginx/kubernetes-ingress/internal/nginx"
+	"github.com/nginx/kubernetes-ingress/internal/validation"
 )
 
 // ConfigParams holds NGINX configuration parameters that affect the main NGINX config
@@ -69,8 +70,9 @@ type ConfigParams struct {
 	MainAppProtectDosLogFormatEscaping     string
 	MainAppProtectDosArbFqdn               string
 	ProxyBuffering                         bool
-	ProxyBuffers                           string
-	ProxyBufferSize                        string
+	ProxyBuffers                           validation.NumberSizeConfig
+	ProxyBufferSize                        validation.SizeWithUnit
+	ProxyBusyBuffersSize                   validation.SizeWithUnit
 	ProxyConnectTimeout                    string
 	ProxyHideHeaders                       []string
 	ProxyMaxTempFileSize                   string

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, eventLog record.EventRecorder) (*ConfigParams, bool) {
+func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasAppProtectDos bool, hasTLSPassthrough bool, enableDirectiveAutoadjust bool, eventLog record.EventRecorder) (*ConfigParams, bool) {
 	l := nl.LoggerFromContext(ctx)
 	cfgParams := NewDefaultConfigParams(ctx, nginxPlus)
 	configOk := true
@@ -335,12 +335,53 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
 	}
 
 	if proxyBuffers, exists := cfgm.Data["proxy-buffers"]; exists {
-		cfgParams.ProxyBuffers = proxyBuffers
+		proxyBuffersData, err := validation.NewNumberSizeConfig(proxyBuffers)
+		if err != nil {
+			wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-buffers': %w", cfgm.GetNamespace(), cfgm.GetName(), err)
+
+			nl.Errorf(l, "%s", wrappedError.Error())
+			eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, wrappedError.Error())
+			configOk = false
+		} else {
+			cfgParams.ProxyBuffers = proxyBuffersData
+		}
 	}
 
 	if proxyBufferSize, exists := cfgm.Data["proxy-buffer-size"]; exists {
-		cfgParams.ProxyBufferSize = proxyBufferSize
+		proxyBufferSizeData, err := validation.NewSizeWithUnit(proxyBufferSize)
+		if err != nil {
+			nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
+		} else {
+			cfgParams.ProxyBufferSize = proxyBufferSizeData
+		}
+		// normalizedProxyBufferSize := validation.NormalizeBufferSize(proxyBufferSize)
+	}
+
+	// Proxy Busy Buffers Size uses only size format, like "8k".
+	if proxyBusyBuffersSize, exists := cfgm.Data["proxy-busy-buffers-size"]; exists {
+		proxyBusyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBusyBuffersSize)
+		if err != nil {
+			nl.Errorf(l, "error parsing nginx.org/proxy-busy-buffers-size: %s", err)
+		} else {
+			cfgParams.ProxyBusyBuffersSize = proxyBusyBufferSizeUnit
+		}
+	}
+
+	balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
+	if err != nil {
+		nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
 	}
+	cfgParams.ProxyBuffers = balancedProxyBuffers
+	cfgParams.ProxyBufferSize = balancedProxyBufferSize
+	cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
+
+	if len(modifications) > 0 {
+		for _, modification := range modifications {
+			nl.Infof(l, "Changes made to proxy values: %s", modification)
+		}
+	}
+
+	// Normalise the three proxy buffer values across each other.
 
 	if proxyMaxTempFileSize, exists := cfgm.Data["proxy-max-temp-file-size"]; exists {
 		cfgParams.ProxyMaxTempFileSize = proxyMaxTempFileSize
@@ -408,7 +449,7 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
 		}
 	}
 
-	_, err := parseConfigMapZoneSync(l, cfgm, cfgParams, eventLog, nginxPlus)
+	_, err = parseConfigMapZoneSync(l, cfgm, cfgParams, eventLog, nginxPlus)
 	if err != nil {
 		configOk = false
 	}