Skip to content

Commit f5255a4

Browse files
AlexFenlonAlexFenlon
authored
Fix Proxy Buffer Config Adjustments (#8226)
1 parent 43da93f commit f5255a4

17 files changed

+603
-874
lines changed

‎internal/configs/annotations.go‎

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -299,47 +299,36 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
299299
}
300300
}
301301

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

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

322-
// Proxy Busy Buffers Size uses only size format, like "8k".
312+
// proxyBusyBuffersSize gets validated in k8s/validation.go in annotationValidations
323313
if proxyBusyBuffersSize, exists := ingEx.Ingress.Annotations["nginx.org/proxy-busy-buffers-size"]; exists {
324-
proxyBusyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBusyBuffersSize)
325-
if err != nil {
326-
nl.Errorf(l, "error parsing nginx.org/proxy-busy-buffers-size: %s", err)
327-
} else {
328-
cfgParams.ProxyBusyBuffersSize = proxyBusyBufferSizeUnit
329-
}
314+
cfgParams.ProxyBusyBuffersSize = proxyBusyBuffersSize
330315
}
331316

332-
balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
333-
if err != nil {
334-
nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
335-
}
336-
cfgParams.ProxyBuffers = balancedProxyBuffers
337-
cfgParams.ProxyBufferSize = balancedProxyBufferSize
338-
cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
317+
// Only run balance validation if auto-adjust is enabled
318+
if enableDirectiveAutoadjust {
319+
balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
320+
if err != nil {
321+
nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
322+
} else {
323+
cfgParams.ProxyBuffers = balancedProxyBuffers
324+
cfgParams.ProxyBufferSize = balancedProxyBufferSize
325+
cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
339326

340-
if len(modifications) > 0 {
341-
for _, modification := range modifications {
342-
nl.Infof(l, "Changes made to proxy values: %s", modification)
327+
if len(modifications) > 0 {
328+
for _, modification := range modifications {
329+
nl.Infof(l, "Changes made to proxy values: %s", modification)
330+
}
331+
}
343332
}
344333
}
345334

‎internal/configs/config_params.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55

66
"github.com/nginx/kubernetes-ingress/internal/configs/version2"
77
"github.com/nginx/kubernetes-ingress/internal/nginx"
8-
"github.com/nginx/kubernetes-ingress/internal/validation"
98
)
109

1110
// ConfigParams holds NGINX configuration parameters that affect the main NGINX config
@@ -70,9 +69,9 @@ type ConfigParams struct {
7069
MainAppProtectDosLogFormatEscaping string
7170
MainAppProtectDosArbFqdn string
7271
ProxyBuffering bool
73-
ProxyBuffers validation.NumberSizeConfig
74-
ProxyBufferSize validation.SizeWithUnit
75-
ProxyBusyBuffersSize validation.SizeWithUnit
72+
ProxyBuffers string
73+
ProxyBufferSize string
74+
ProxyBusyBuffersSize string
7675
ProxyConnectTimeout string
7776
ProxyHideHeaders []string
7877
ProxyMaxTempFileSize string

‎internal/configs/configmaps.go‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -335,48 +335,56 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
335335
}
336336

337337
if proxyBuffers, exists := cfgm.Data["proxy-buffers"]; exists {
338-
proxyBuffersData, err := validation.NewNumberSizeConfig(proxyBuffers)
339-
if err != nil {
338+
if parsedProxyBuffers, err := ParseProxyBuffersSpec(proxyBuffers); err != nil {
340339
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-buffers': %w", cfgm.GetNamespace(), cfgm.GetName(), err)
341340

342341
nl.Errorf(l, "%s", wrappedError.Error())
343342
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, wrappedError.Error())
344343
configOk = false
345344
} else {
346-
cfgParams.ProxyBuffers = proxyBuffersData
345+
cfgParams.ProxyBuffers = parsedProxyBuffers
347346
}
348347
}
349348

350349
if proxyBufferSize, exists := cfgm.Data["proxy-buffer-size"]; exists {
351-
proxyBufferSizeData, err := validation.NewSizeWithUnit(proxyBufferSize)
352-
if err != nil {
353-
nl.Errorf(l, "error parsing nginx.org/proxy-buffer-size: %s", err)
350+
if parsedProxyBufferSize, err := ParseSize(proxyBufferSize); err != nil {
351+
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-buffer-size': %w", cfgm.GetNamespace(), cfgm.GetName(), err)
352+
353+
nl.Errorf(l, "%s", wrappedError.Error())
354+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, wrappedError.Error())
355+
configOk = false
354356
} else {
355-
cfgParams.ProxyBufferSize = proxyBufferSizeData
357+
cfgParams.ProxyBufferSize = parsedProxyBufferSize
356358
}
357359
}
358360

359-
// Proxy Busy Buffers Size uses only size format, like "8k".
360361
if proxyBusyBuffersSize, exists := cfgm.Data["proxy-busy-buffers-size"]; exists {
361-
proxyBusyBufferSizeUnit, err := validation.NewSizeWithUnit(proxyBusyBuffersSize)
362-
if err != nil {
363-
nl.Errorf(l, "error parsing nginx.org/proxy-busy-buffers-size: %s", err)
362+
if parsedProxyBusyBuffersSize, err := ParseSize(proxyBusyBuffersSize); err != nil {
363+
wrappedError := fmt.Errorf("ConfigMap %s/%s: invalid value for 'proxy-busy-buffers-size': %w", cfgm.GetNamespace(), cfgm.GetName(), err)
364+
365+
nl.Errorf(l, "%s", wrappedError.Error())
366+
eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, wrappedError.Error())
367+
configOk = false
364368
} else {
365-
cfgParams.ProxyBusyBuffersSize = proxyBusyBufferSizeUnit
369+
cfgParams.ProxyBusyBuffersSize = parsedProxyBusyBuffersSize
366370
}
367371
}
368372

369-
balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
370-
if err != nil {
371-
nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
372-
}
373-
cfgParams.ProxyBuffers = balancedProxyBuffers
374-
cfgParams.ProxyBufferSize = balancedProxyBufferSize
375-
cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
373+
// Only run balance validation if auto-adjust is enabled
374+
if enableDirectiveAutoadjust {
375+
balancedProxyBuffers, balancedProxyBufferSize, balancedProxyBusyBufferSize, modifications, err := validation.BalanceProxyValues(cfgParams.ProxyBuffers, cfgParams.ProxyBufferSize, cfgParams.ProxyBusyBuffersSize, enableDirectiveAutoadjust)
376+
if err != nil {
377+
nl.Errorf(l, "error reconciling proxy_buffers, proxy_buffer_size, and proxy_busy_buffers_size values: %s", err.Error())
378+
} else {
379+
cfgParams.ProxyBuffers = balancedProxyBuffers
380+
cfgParams.ProxyBufferSize = balancedProxyBufferSize
381+
cfgParams.ProxyBusyBuffersSize = balancedProxyBusyBufferSize
376382

377-
if len(modifications) > 0 {
378-
for _, modification := range modifications {
379-
nl.Infof(l, "Changes made to proxy values: %s", modification)
383+
if len(modifications) > 0 {
384+
for _, modification := range modifications {
385+
nl.Infof(l, "Changes made to proxy values: %s", modification)
386+
}
387+
}
380388
}
381389
}
382390

@@ -446,7 +454,7 @@ func ParseConfigMap(ctx context.Context, cfgm *v1.ConfigMap, nginxPlus bool, has
446454
}
447455
}
448456

449-
_, err = parseConfigMapZoneSync(l, cfgm, cfgParams, eventLog, nginxPlus)
457+
_, err := parseConfigMapZoneSync(l, cfgm, cfgParams, eventLog, nginxPlus)
450458
if err != nil {
451459
configOk = false
452460
}

0 commit comments

Comments
 (0)