Skip to content

Fix Proxy Buffer Config Adjustments for Configmap #8217

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

Closed
wants to merge 5 commits into from
+632 −555
Closed

Fix Proxy Buffer Config Adjustments for Configmap #8217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 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
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
60 changes: 38 additions & 22 deletions internal/configs/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,45 +301,61 @@ 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 {
proxyBufferUnits, err := validation.NewNumberSizeConfig(proxyBuffers)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffers: %s", err)
if enableDirectiveAutoadjust {
normalizedProxyBuffers, err := validation.NewNumberSizeConfig(proxyBuffers, enableDirectiveAutoadjust)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffers: %s", err)
} else {
cfgParams.ProxyBuffers = normalizedProxyBuffers
}
} else {
cfgParams.ProxyBuffers = proxyBufferUnits
cfgParams.ProxyBuffers = proxyBuffers
}
}

// Proxy Buffer Size uses only size format, like "4k".
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)
if enableDirectiveAutoadjust {
normalizedProxyBufferSize, err := validation.NewSizeWithUnit(proxyBufferSize, enableDirectiveAutoadjust)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
} else {
cfgParams.ProxyBufferSize = normalizedProxyBufferSize
}
} else {
cfgParams.ProxyBufferSize = proxyBufferSizeUnit
cfgParams.ProxyBufferSize = proxyBufferSize
}
}

// 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)
if enableDirectiveAutoadjust {
normalizedProxyBusyBuffersSize, err := validation.NewSizeWithUnit(proxyBusyBuffersSize, enableDirectiveAutoadjust)
if err != nil {
nl.Errorf(l, "error parsing nginx.org/proxy-busy-buffers-size: %s", err)
} else {
cfgParams.ProxyBusyBuffersSize = normalizedProxyBusyBuffersSize
}
} 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
76 changes: 50 additions & 26 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,72 @@ 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 {
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-buffers': %w", cfgm.GetNamespace(), cfgm.GetName(), err)
if enableDirectiveAutoadjust {
proxyBuffersData, err := validation.NewNumberSizeConfig(proxyBuffers, enableDirectiveAutoadjust)
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
nl.Errorf(l, "%s", wrappedError.Error())
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, wrappedError.Error())
configOk = false
} else {
cfgParams.ProxyBuffers = proxyBuffersData
}
} else {
cfgParams.ProxyBuffers = proxyBuffersData
cfgParams.ProxyBuffers = proxyBuffers
}
}

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 enableDirectiveAutoadjust {
proxyBufferSizeData, err := validation.NewSizeWithUnit(proxyBufferSize, enableDirectiveAutoadjust)
if 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
}
} else {
cfgParams.ProxyBufferSize = proxyBufferSizeData
cfgParams.ProxyBufferSize = 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)
if enableDirectiveAutoadjust {
proxyBusyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBusyBuffersSize, enableDirectiveAutoadjust)
if 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
}
} 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 Expand Up @@ -446,7 +470,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