Skip to content

Fix Proxy Buffer Config Adjustments #8226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Sep 5, 2025
Merged

Fix Proxy Buffer Config Adjustments #8226

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9ee4e0c
init
AlexFenlon Sep 2, 2025
5c41283
fix VS, CM and annotations
AlexFenlon Sep 3, 2025
b0fbd1d
fix tests
AlexFenlon Sep 3, 2025
730f26c
fix cyclomatic complexity
AlexFenlon Sep 3, 2025
456ca73
Make vs invalid if user enters invalid config
AlexFenlon Sep 4, 2025
7799888
remove dead code
AlexFenlon Sep 5, 2025
39f8dfe
refactor annotations logic
AlexFenlon Sep 5, 2025
f866840
refactor configmap validation
AlexFenlon Sep 5, 2025
8ded94d
Merge branch 'main' into chore/proxy-buffers-fix
AlexFenlon Sep 5, 2025
3441aeb
fix lint
AlexFenlon Sep 5, 2025
41db8b8
fix typo, make NewNumberSizeConfig private, fix test file to same pac…
AlexFenlon Sep 5, 2025
1193bf0
Merge branch 'main' into chore/proxy-buffers-fix
AlexFenlon Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 20 additions & 31 deletions internal/configs/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,47 +299,36 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
}
}

// Proxy Buffers uses number + size format, like "8 4k".
// proxyBuffers gets validated in k8s/validation.go in annotationValidations
if proxyBuffers, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffers"]; exists {
proxyBufferUnits, err := validation.NewNumberSizeConfig(proxyBuffers)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffers: %s", err)
} else {
cfgParams.ProxyBuffers = proxyBufferUnits
}
cfgParams.ProxyBuffers = proxyBuffers
}

// Proxy Buffer Size uses only size format, like "4k".
// proxyBufferSize gets validated in k8s/validation.go in annotationValidations
if proxyBufferSize, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffer-size"]; exists {
proxyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBufferSize)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
} else {
cfgParams.ProxyBufferSize = proxyBufferSizeUnit
}
cfgParams.ProxyBufferSize = proxyBufferSize
}

// Proxy Busy Buffers Size uses only size format, like "8k".
// proxyBusyBuffersSize gets validated in k8s/validation.go in annotationValidations
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
}
cfgParams.ProxyBusyBuffersSize = proxyBusyBuffersSize
}

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
// Only run balance validation if auto-adjust is enabled
if enableDirectiveAutoadjust {
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())
} else {
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 len(modifications) > 0 {
for _, modification := range modifications {
nl.Infof(l, "Changes made to proxy values: %s", modification)
}
}
}
}

Expand Down
7 changes: 3 additions & 4 deletions internal/configs/config_params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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
Expand Down Expand Up @@ -70,9 +69,9 @@ type ConfigParams struct {
MainAppProtectDosLogFormatEscaping string
MainAppProtectDosArbFqdn string
ProxyBuffering bool
ProxyBuffers validation.NumberSizeConfig
ProxyBufferSize validation.SizeWithUnit
ProxyBusyBuffersSize validation.SizeWithUnit
ProxyBuffers string
ProxyBufferSize string
ProxyBusyBuffersSize string
ProxyConnectTimeout string
ProxyHideHeaders []string
ProxyMaxTempFileSize string
Expand Down
54 changes: 31 additions & 23 deletions internal/configs/configmaps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,48 +335,56 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
}

if proxyBuffers, exists := cfgm.Data["proxy-buffers"]; exists {
proxyBuffersData, err := validation.NewNumberSizeConfig(proxyBuffers)
if err != nil {
if parsedProxyBuffers, err := ParseProxyBuffersSpec(proxyBuffers); 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
cfgParams.ProxyBuffers = parsedProxyBuffers
}
}

if proxyBufferSize, exists := cfgm.Data["proxy-buffer-size"]; exists {
proxyBufferSizeData, err := validation.NewSizeWithUnit(proxyBufferSize)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
if parsedProxyBufferSize, err := ParseSize(proxyBufferSize); err != nil {
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-buffer-size': %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.ProxyBufferSize = proxyBufferSizeData
cfgParams.ProxyBufferSize = parsedProxyBufferSize
}
}

// 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)
if parsedProxyBusyBuffersSize, err := ParseSize(proxyBusyBuffersSize); err != nil {
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-busy-buffers-size': %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.ProxyBusyBuffersSize = proxyBusyBufferSizeUnit
cfgParams.ProxyBusyBuffersSize = parsedProxyBusyBuffersSize
}
}

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
// Only run balance validation if auto-adjust is enabled
if enableDirectiveAutoadjust {
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())
} else {
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 len(modifications) > 0 {
for _, modification := range modifications {
nl.Infof(l, "Changes made to proxy values: %s", modification)
}
}
}
}

Expand Down Expand Up @@ -446,7 +454,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
}
Expand Down
Loading
Loading