From dc4168d79b343c9029011ce4dc2b103d6a16a021 Mon Sep 17 00:00:00 2001 From: sugaf1204 Date: Fri, 4 Jul 2025 02:12:54 +0900 Subject: [PATCH 1/3] Annotations: Add upstream server max connections support - Introduced `upstream-server-max-conns` annotation to configure the maximum number of simultaneous active connections to the proxied server. - Implemented validation for the new annotation to ensure it accepts only unsigned integers. - Updated the Ingress struct and related parsing logic to accommodate the new annotation. - Added tests to verify the functionality and validation of the new annotation. - Updated NGINX configuration to utilize the max connections setting. --- internal/ingress/annotations/annotations.go | 3 + internal/ingress/annotations/parser/main.go | 21 ++ .../ingress/annotations/parser/validators.go | 6 + .../annotations/upstreamserver/main.go | 82 ++++++++ .../annotations/upstreamserver/main_test.go | 179 ++++++++++++++++++ internal/ingress/defaults/main.go | 4 + pkg/apis/ingress/types.go | 2 + .../etc/nginx/lua/test/util/nodemap_test.lua | 35 +++- rootfs/etc/nginx/lua/util.lua | 5 + 9 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 internal/ingress/annotations/upstreamserver/main.go create mode 100644 internal/ingress/annotations/upstreamserver/main_test.go diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index e10cc9be17..7bea74e281 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -63,6 +63,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/sslpassthrough" "k8s.io/ingress-nginx/internal/ingress/annotations/streamsnippet" "k8s.io/ingress-nginx/internal/ingress/annotations/upstreamhashby" + "k8s.io/ingress-nginx/internal/ingress/annotations/upstreamserver" "k8s.io/ingress-nginx/internal/ingress/annotations/upstreamvhost" "k8s.io/ingress-nginx/internal/ingress/annotations/xforwardedprefix" "k8s.io/ingress-nginx/internal/ingress/errors" @@ -114,6 +115,7 @@ type Ingress struct { Logs log.Config ModSecurity modsecurity.Config Mirror mirror.Config + UpstreamServer upstreamserver.Config StreamSnippet string Allowlist ipallowlist.SourceRange } @@ -164,6 +166,7 @@ func NewAnnotationFactory(cfg resolver.Resolver) map[string]parser.IngressAnnota "BackendProtocol": backendprotocol.NewParser(cfg), "ModSecurity": modsecurity.NewParser(cfg), "Mirror": mirror.NewParser(cfg), + "UpstreamServer": upstreamserver.NewParser(cfg), "StreamSnippet": streamsnippet.NewParser(cfg), } } diff --git a/internal/ingress/annotations/parser/main.go b/internal/ingress/annotations/parser/main.go index 3137afbfd0..0965996edf 100644 --- a/internal/ingress/annotations/parser/main.go +++ b/internal/ingress/annotations/parser/main.go @@ -139,6 +139,18 @@ func (a ingAnnotations) parseInt(name string) (int, error) { return 0, errors.ErrMissingAnnotations } +func (a ingAnnotations) parseUint(name string) (uint, error) { + val, ok := a[name] + if ok { + i, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return 0, errors.NewInvalidAnnotationContent(name, val) + } + return uint(i), nil + } + return 0, errors.ErrMissingAnnotations +} + func (a ingAnnotations) parseFloat32(name string) (float32, error) { val, ok := a[name] if ok { @@ -179,6 +191,15 @@ func GetIntAnnotation(name string, ing *networking.Ingress, fields AnnotationFie return ingAnnotations(ing.GetAnnotations()).parseInt(v) } +// GetUintAnnotation extracts an uint from an Ingress annotation +func GetUintAnnotation(name string, ing *networking.Ingress, fields AnnotationFields) (uint, error) { + v, err := checkAnnotation(name, ing, fields) + if err != nil { + return 0, err + } + return ingAnnotations(ing.GetAnnotations()).parseUint(v) +} + // GetFloatAnnotation extracts a float32 from an Ingress annotation func GetFloatAnnotation(name string, ing *networking.Ingress, fields AnnotationFields) (float32, error) { v, err := checkAnnotation(name, ing, fields) diff --git a/internal/ingress/annotations/parser/validators.go b/internal/ingress/annotations/parser/validators.go index 3c724a3110..6e3dfbfa8e 100644 --- a/internal/ingress/annotations/parser/validators.go +++ b/internal/ingress/annotations/parser/validators.go @@ -168,6 +168,12 @@ func ValidateInt(value string) error { return err } +// ValidateUint validates if the specified value is an unsigned integer +func ValidateUint(value string) error { + _, err := strconv.ParseUint(value, 10, 64) + return err +} + // ValidateCIDRs validates if the specified value is an array of IPs and CIDRs func ValidateCIDRs(value string) error { _, err := net.ParseCIDRs(value) diff --git a/internal/ingress/annotations/upstreamserver/main.go b/internal/ingress/annotations/upstreamserver/main.go new file mode 100644 index 0000000000..0683033abb --- /dev/null +++ b/internal/ingress/annotations/upstreamserver/main.go @@ -0,0 +1,82 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upstreamserver + +import ( + networking "k8s.io/api/networking/v1" + + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/errors" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +const ( + upstreamServerMaxConnsAnnotation = "upstream-server-max-conns" +) + +var upstreamServerAnnotations = parser.Annotation{ + Group: "backend", + Annotations: parser.AnnotationFields{ + upstreamServerMaxConnsAnnotation: { + Validator: parser.ValidateUint, + Scope: parser.AnnotationScopeLocation, + Risk: parser.AnnotationRiskLow, + Documentation: `This annotation allows setting the maximum number of simultaneous active connections to the proxied server. Default value is 0, which means no limit.`, + }, + }, +} + +// Config contains the upstream server configuration +type Config struct { + MaxConns uint `json:"maxConns,omitempty"` +} + +type upstreamServer struct { + r resolver.Resolver + annotationConfig parser.Annotation +} + +// NewParser creates a new serviceUpstream annotation parser +func NewParser(r resolver.Resolver) parser.IngressAnnotation { + return upstreamServer{ + r: r, + annotationConfig: upstreamServerAnnotations, + } +} + +func (s upstreamServer) Parse(ing *networking.Ingress) (interface{}, error) { + defBackend := s.r.GetDefaultBackend() + + val, err := parser.GetUintAnnotation(upstreamServerMaxConnsAnnotation, ing, s.annotationConfig.Annotations) + // A missing annotation is not a problem, just use the default + if err == errors.ErrMissingAnnotations { + return &Config{MaxConns: defBackend.UpstreamServerMaxConns}, nil + } else if err != nil { + return &Config{MaxConns: 0}, err + } + + return &Config{MaxConns: val}, nil +} + +func (s upstreamServer) GetDocumentation() parser.AnnotationFields { + return s.annotationConfig.Annotations +} + +func (s upstreamServer) Validate(anns map[string]string) error { + maxrisk := parser.StringRiskToRisk(s.r.GetSecurityConfiguration().AnnotationsRiskLevel) + return parser.CheckAnnotationRisk(anns, maxrisk, upstreamServerAnnotations.Annotations) +} diff --git a/internal/ingress/annotations/upstreamserver/main_test.go b/internal/ingress/annotations/upstreamserver/main_test.go new file mode 100644 index 0000000000..b18201d285 --- /dev/null +++ b/internal/ingress/annotations/upstreamserver/main_test.go @@ -0,0 +1,179 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upstreamserver + +import ( + "fmt" + "testing" + + api "k8s.io/api/core/v1" + networking "k8s.io/api/networking/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/defaults" + "k8s.io/ingress-nginx/internal/ingress/errors" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +func buildIngress() *networking.Ingress { + defaultBackend := networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "default-backend", + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + } + + return &networking.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: networking.IngressSpec{ + DefaultBackend: &networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "default-backend", + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + Rules: []networking.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/foo", + Backend: defaultBackend, + }, + }, + }, + }, + }, + }, + }, + } +} + +func TestIngressAnnotationUpstreamServerMaxConnsSetPositive(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "100" + ing.SetAnnotations(data) + + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + config, ok := val.(*Config) + if !ok { + t.Errorf("expected a *Config type") + } + + if config.MaxConns != 100 { + t.Errorf("expected annotation value to be 100, got %d", config.MaxConns) + } +} + +func TestIngressAnnotationUpstreamServerMaxConnsSetNegative(t *testing.T) { + ing := buildIngress() + + // Test with explicitly set to negative value + expectedErr := errors.New(fmt.Sprintf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation)) + data := map[string]string{} + data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "-1" + ing.SetAnnotations(data) + + _, err := NewParser(&resolver.Mock{}).Parse(ing) + if err == nil || err.Error() != expectedErr.Error() { + t.Errorf("expected error: %v, got %v", expectedErr, err) + } +} + +func TestIngressAnnotationUpstreamServerMaxConnsSetString(t *testing.T) { + ing := buildIngress() + + // Test with explicitly set to negative value + expectedErr := errors.New(fmt.Sprintf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation)) + data := map[string]string{} + data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "uhi" + ing.SetAnnotations(data) + + _, err := NewParser(&resolver.Mock{}).Parse(ing) + if err == nil || err.Error() != expectedErr.Error() { + t.Errorf("expected error: %v, got %v", expectedErr, err) + } +} + +type mockBackend struct { + resolver.Mock +} + +// GetDefaultBackend returns the backend that must be used as default +func (m mockBackend) GetDefaultBackend() defaults.Backend { + return defaults.Backend{ + UpstreamServerMaxConns: 0, + } +} + +// Test that when we have a default configuration set on the Backend that is used +// when we don't have the annotation +func TestParseAnnotationsWithDefaultConfig(t *testing.T) { + ing := buildIngress() + + val, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + config, ok := val.(*Config) + + if !ok { + t.Errorf("expected a *Config type") + } + + if config.MaxConns != 0 { + t.Errorf("expected annotation value to be 0, got %d", config.MaxConns) + } +} + +// Test that the annotation will disable the service upstream when enabled +// in the default configuration +func TestParseAnnotationsOverridesDefaultConfig(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "0" + ing.SetAnnotations(data) + + val, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + config, ok := val.(*Config) + + if !ok { + t.Errorf("expected a *Config type") + } + + if config.MaxConns != 0 { + t.Errorf("expected annotation value to be 0, got %d", config.MaxConns) + } +} diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 4ee3d8e525..8c19f0f673 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -189,6 +189,10 @@ type Backend struct { // AllowedResponseHeaders allows to define allow response headers for custom header annotation AllowedResponseHeaders []string `json:"global-allowed-response-headers"` + + // Sets the maximum number of simultaneous active connections to the proxied server. + // Default value is 0, which means no limit. + UpstreamServerMaxConns uint `json:"upstream-server-max-conns"` } type SecurityConfiguration struct { diff --git a/pkg/apis/ingress/types.go b/pkg/apis/ingress/types.go index ccdd49fe92..3f24ae09e4 100644 --- a/pkg/apis/ingress/types.go +++ b/pkg/apis/ingress/types.go @@ -179,6 +179,8 @@ type Endpoint struct { Address string `json:"address"` // Port number of the TCP port Port string `json:"port"` + // MaxConns maximum number of connections to the endpoint + MaxConns int `json:"maxConns,omitempty"` // Target returns a reference to the object providing the endpoint Target *apiv1.ObjectReference `json:"target,omitempty"` } diff --git a/rootfs/etc/nginx/lua/test/util/nodemap_test.lua b/rootfs/etc/nginx/lua/test/util/nodemap_test.lua index f012bb7eef..ebb54bb145 100644 --- a/rootfs/etc/nginx/lua/test/util/nodemap_test.lua +++ b/rootfs/etc/nginx/lua/test/util/nodemap_test.lua @@ -5,7 +5,7 @@ local function get_test_backend_single() return { name = "access-router-production-web-80", endpoints = { - { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 } + { address = "10.184.7.40", port = "8080", maxConns = 0, maxFails = 0, failTimeout = 0 } } } end @@ -14,8 +14,18 @@ local function get_test_backend_multi() return { name = "access-router-production-web-80", endpoints = { - { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 }, - { address = "10.184.7.41", port = "8080", maxFails = 0, failTimeout = 0 } + { address = "10.184.7.40", port = "8080", maxConns = 0, maxFails = 0, failTimeout = 0 }, + { address = "10.184.7.41", port = "8080", maxConns = 0, maxFails = 0, failTimeout = 0 } + } + } +end + +local function get_test_backend_parameterized() + return { + name = "access-router-production-web-80", + endpoints = { + { address = "10.184.7.40", port = "8080", maxConns = 100, maxFails = 0, failTimeout = 0 }, + { address = "10.184.7.41", port = "8080", maxConns = 100, maxFails = 0, failTimeout = 0 } } } end @@ -30,12 +40,17 @@ describe("Node Map", function() local test_backend_single = get_test_backend_single() local test_backend_multi = get_test_backend_multi() + local test_backend_parameterized = get_test_backend_parameterized() local test_salt = test_backend_single.name local test_nodes_single = util.get_nodes(test_backend_single.endpoints) local test_nodes_multi = util.get_nodes(test_backend_multi.endpoints) + local test_nodes_parameterized = util.get_nodes(test_backend_parameterized.endpoints) local test_endpoint1 = test_backend_multi.endpoints[1].address .. ":" .. test_backend_multi.endpoints[1].port local test_endpoint2 = test_backend_multi.endpoints[2].address .. ":" .. test_backend_multi.endpoints[2].port + local test_endpoint_parameterized1 = test_backend_parameterized.endpoints[1].address .. ":" .. test_backend_parameterized.endpoints[1].port .. " max_conns=" .. test_backend_parameterized.endpoints[1].maxConns + local test_endpoint_parameterized2 = test_backend_parameterized.endpoints[2].address .. ":" .. test_backend_parameterized.endpoints[2].port .. " max_conns=" .. test_backend_parameterized.endpoints[2].maxConns local test_nodes_ignore = get_test_nodes_ignore(test_endpoint1) + local test_nodes_ignore_parameterized = get_test_nodes_ignore(test_endpoint_parameterized1) describe("new()", function() context("when no salt has been provided", function() @@ -163,5 +178,19 @@ describe("Node Map", function() assert.not_equal(test_hash_key, nil) end) end) + + context("when an parameterized endpoint has been excluded", function() + it("random_except() does not return it", function() + local nodemap_instance = nodemap:new(test_nodes_parameterized, test_salt) + local expected_endpoint = test_endpoint_parameterized2 + local actual_endpoint + local test_hash_key + + actual_endpoint, test_hash_key = nodemap_instance:random_except(test_nodes_ignore_parameterized) + + assert.equal(actual_endpoint, expected_endpoint) + assert.not_equal(test_hash_key, nil) + end) + end) end) end) diff --git a/rootfs/etc/nginx/lua/util.lua b/rootfs/etc/nginx/lua/util.lua index 1e4cd7c017..758076c714 100644 --- a/rootfs/etc/nginx/lua/util.lua +++ b/rootfs/etc/nginx/lua/util.lua @@ -19,6 +19,11 @@ function _M.get_nodes(endpoints) for _, endpoint in pairs(endpoints) do local endpoint_string = endpoint.address .. ":" .. endpoint.port + + if endpoint.maxConns and endpoint.maxConns > 0 then + endpoint_string = endpoint_string .. " max_conns=" .. endpoint.maxConns + end + nodes[endpoint_string] = weight end From f1cc54c99d43255b11f40bce23f52db20b66a7b7 Mon Sep 17 00:00:00 2001 From: sugaf1204 Date: Fri, 4 Jul 2025 04:02:22 +0900 Subject: [PATCH 2/3] update docs for max_conns annotations --- .../nginx-configuration/annotations-risk.md | 1 + .../nginx-configuration/annotations.md | 22 ++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/user-guide/nginx-configuration/annotations-risk.md b/docs/user-guide/nginx-configuration/annotations-risk.md index aff9357b88..de56dd59dc 100755 --- a/docs/user-guide/nginx-configuration/annotations-risk.md +++ b/docs/user-guide/nginx-configuration/annotations-risk.md @@ -135,6 +135,7 @@ | UpstreamHashBy | upstream-hash-by | High | location | | UpstreamHashBy | upstream-hash-by-subset | Low | location | | UpstreamHashBy | upstream-hash-by-subset-size | Low | location | +| UpstreamServer | upstream-server-max-conns | Low | location | | UpstreamVhost | upstream-vhost | Low | location | | UsePortInRedirects | use-port-in-redirects | Low | location | | XForwardedPrefix | x-forwarded-prefix | Medium | location | diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 82ad076626..2b071965b8 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -132,6 +132,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/mirror-request-body](#mirror)|string| |[nginx.ingress.kubernetes.io/mirror-target](#mirror)|string| |[nginx.ingress.kubernetes.io/mirror-host](#mirror)|string| +|[nginx.ingress.kubernetes.io/upstream-server-max-conns](#upstream-server-max-conns)|number| ### Canary @@ -418,7 +419,7 @@ CORS can be controlled with the following annotations: ### HTTP2 Push Preload. -Enables automatic conversion of preload links specified in the “Link” response header fields into push requests. +Enables automatic conversion of preload links specified in the "Link" response header fields into push requests. !!! example @@ -986,3 +987,22 @@ metadata: proxy_pass 127.0.0.1:80; } ``` + +### Upstream Server Max Connections + +The annotation `nginx.ingress.kubernetes.io/upstream-server-max-conns` allows you to set [max_conns](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#server) parameter. The default value is `0`, which means no limit is applied. + +This can be useful to limit the number of concurrent connections to a backend pod, helping to prevent overloading the backend. + +**Example usage:** + +```yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/upstream-server-max-conns: "100" +spec: + # ... +``` + From 96c7d07e7141b195bb779c2be45baa0d6aaef98f Mon Sep 17 00:00:00 2001 From: sugaf1204 Date: Fri, 4 Jul 2025 04:10:38 +0900 Subject: [PATCH 3/3] fix to pass ci-lint --- internal/ingress/annotations/upstreamserver/main_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/ingress/annotations/upstreamserver/main_test.go b/internal/ingress/annotations/upstreamserver/main_test.go index b18201d285..377a5de4ff 100644 --- a/internal/ingress/annotations/upstreamserver/main_test.go +++ b/internal/ingress/annotations/upstreamserver/main_test.go @@ -25,7 +25,6 @@ import ( meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/defaults" - "k8s.io/ingress-nginx/internal/ingress/errors" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -97,7 +96,7 @@ func TestIngressAnnotationUpstreamServerMaxConnsSetNegative(t *testing.T) { ing := buildIngress() // Test with explicitly set to negative value - expectedErr := errors.New(fmt.Sprintf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation)) + expectedErr := fmt.Errorf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation) data := map[string]string{} data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "-1" ing.SetAnnotations(data) @@ -112,7 +111,7 @@ func TestIngressAnnotationUpstreamServerMaxConnsSetString(t *testing.T) { ing := buildIngress() // Test with explicitly set to negative value - expectedErr := errors.New(fmt.Sprintf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation)) + expectedErr := fmt.Errorf("annotation nginx.ingress.kubernetes.io/%s contains invalid value", upstreamServerMaxConnsAnnotation) data := map[string]string{} data[parser.GetAnnotationWithPrefix(upstreamServerMaxConnsAnnotation)] = "uhi" ing.SetAnnotations(data)