Add use-cluster-ip annotation for ingress resources (#4862)

* add use cluter ip annotation * Update docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md Co-authored-by: Alan Dooley <[email protected]> Signed-off-by: Jim Ryan <[email protected]> * annotation validation * remove duplicate test * small docs fix * go ingress tests * use fqdn to support cross namespace ingresses --------- Signed-off-by: Jim Ryan <[email protected]> Co-authored-by: Alan Dooley <[email protected]>
nginx · Jan 5, 2024 · 38a03fa · 38a03fa
1 parent 99c9212
commit 38a03fa
Show file tree

Hide file tree

Showing 10 changed files with 325 additions and 10 deletions.
diff --git a/...tent/configuration/ingress-resources/advanced-configuration-with-annotations.md b/...tent/configuration/ingress-resources/advanced-configuration-with-annotations.md
@@ -172,6 +172,7 @@ The table below summarizes the available annotations.
 |``nginx.com/health-checks-mandatory`` | N/A | Configures active health checks as mandatory. | ``False`` | [Support for Active Health Checks](https://github.com/nginxinc/kubernetes-ingress/tree/v3.4.0/examples/ingress-resources/health-checks). |
 |``nginx.com/health-checks-mandatory-queue`` | N/A | When active health checks are mandatory, creates a queue where incoming requests are temporarily stored while NGINX Plus is checking the health of the endpoints after a configuration reload. | ``0`` | [Support for Active Health Checks](https://github.com/nginxinc/kubernetes-ingress/tree/v3.4.0/examples/ingress-resources/health-checks). |
 |``nginx.com/slow-start`` | N/A | Sets the upstream server [slow-start period](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#server-slow-start). By default, slow-start is activated after a server becomes [available](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/#passive-health-checks) or [healthy](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/#active-health-checks). To enable slow-start for newly-added servers, configure [mandatory active health checks](https://github.com/nginxinc/kubernetes-ingress/tree/v3.4.0/examples/ingress-resources/health-checks). | ``"0s"`` |  |
+|``nginx.org/use-cluster-ip`` | N/A | Enables using the Cluster IP and port of the service instead of the default behavior of using the IP and port of the pods. When this field is enabled, the fields that configure NGINX behavior related to multiple upstream servers (like ``lb-method`` and ``next-upstream``) will have no effect, as NGINX Ingress Controller will configure NGINX with only one upstream server that will match the service Cluster IP.   | ``False`` |  |
 {{% /table %}}
 
 ### Snippets and Custom Templates

diff --git a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md
@@ -349,7 +349,7 @@ tls:
 |``name`` | The name of the upstream. Must be a valid DNS label as defined in RFC 1035. For example, ``hello`` and ``upstream-123`` are valid. The name must be unique among all upstreams of the resource. | ``string`` | Yes |
 |``service`` | The name of a [service](https://kubernetes.io/docs/concepts/services-networking/service/). The service must belong to the same namespace as the resource. If the service doesn't exist, NGINX will assume the service has zero endpoints and return a ``502`` response for requests for this upstream. For NGINX Plus only, services of type [ExternalName](https://kubernetes.io/docs/concepts/services-networking/service/#externalname) are also supported (check the [prerequisites](https://github.com/nginxinc/kubernetes-ingress/tree/v3.4.0/examples/ingress-resources/externalname-services#prerequisites) ). | ``string`` | Yes |
 |``subselector`` | Selects the pods within the service using label keys and values. By default, all pods of the service are selected. Note: the specified labels are expected to be present in the pods when they are created. If the pod labels are updated, the Ingress Controller will not see that change until the number of the pods is changed. | ``map[string]string`` | No |
-|``use-cluster-ip`` | Enables using the Cluster IP and port of the service instead of the default behavior of using the IP and port of the pods. When this field is enabled, the fields that configure NGINX behavior related to multiple upstream servers (like ``lb-method`` and ``next-upstream``) will have no effect, as the Ingress Controller will configure NGINX with only one upstream server that will match the service Cluster IP. | ``boolean`` | No |
+|``use-cluster-ip`` | Enables using the Cluster IP and port of the service instead of the default behavior of using the IP and port of the pods. When this field is enabled, the fields that configure NGINX behavior related to multiple upstream servers (like ``lb-method`` and ``next-upstream``) will have no effect, as NGINX Ingress Controller will configure NGINX with only one upstream server that will match the service Cluster IP. | ``boolean`` | No |
 |``port`` | The port of the service. If the service doesn't define that port, NGINX will assume the service has zero endpoints and return a ``502`` response for requests for this upstream. The port must fall into the range ``1..65535``. | ``uint16`` | Yes |
 |``lb-method`` | The load [balancing method](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method). To use the round-robin method, specify ``round_robin``. The default is specified in the ``lb-method`` ConfigMap key. | ``string`` | No |
 |``fail-timeout`` | The time during which the specified number of unsuccessful attempts to communicate with an upstream server should happen to consider the server unavailable. See the [fail_timeout](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#fail_timeout) parameter of the server directive. The default is set in the ``fail-timeout`` ConfigMap key. | ``string`` | No |

diff --git a/docs/content/tutorials/nginx-ingress-istio.md b/docs/content/tutorials/nginx-ingress-istio.md
@@ -214,7 +214,7 @@ We can see in the above output that our curl request is sent and received by NGI
 
 By default, for NGINX Ingress Controller, we populate the upstream server addresses with the endpoint IPs of the pods.
 
-When using the new `use-cluster-ip` feature, we will no populate the upstream with the `service` IP address, instead of the endpoint IP addresses.
+When using the new `use-cluster-ip` feature, we will now populate the upstream with the `service` IP address, instead of the endpoint IP addresses.
 
 In the 1.11 release, NGINX Ingress controller will only send one host header, depending on how you configure Ingress. By default NGINX Ingress Controller will send `proxy_set_header $host`. If Ingress has been configured with `action.proxy.requestHeaders` this ensures that only one set of headers will be sent to the upstream server. In summary, by setting `action.proxy.requestHeaders` in the `VirtualServer` CRD, NGINX Ingress will only send the specified headers that have been defined.
 

diff --git a/examples/ingress-resources/mergeable-ingress-types/cafe.yaml b/examples/ingress-resources/mergeable-ingress-types/cafe.yaml
@@ -55,7 +55,6 @@ apiVersion: v1
 kind: Service
 metadata:
   name: tea-svc
-  labels:
 spec:
   ports:
   - port: 80

diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go
@@ -13,6 +13,9 @@ const BasicAuthSecretAnnotation = "nginx.org/basic-auth-secret" // #nosec G101
 // PathRegexAnnotation is the annotation where the regex location (path) modifier is specified.
 const PathRegexAnnotation = "nginx.org/path-regex"
 
+// UseClusterIPAnnotation is the annotation where the use-cluster-ip boolean is specified.
+const UseClusterIPAnnotation = "nginx.org/use-cluster-ip"
+
 // AppProtectPolicyAnnotation is where the NGINX App Protect policy is specified
 const AppProtectPolicyAnnotation = "appprotect.f5.com/app-protect-policy"
 
@@ -37,6 +40,7 @@ var masterBlacklist = map[string]bool{
 	"nginx.com/health-checks":                 true,
 	"nginx.com/health-checks-mandatory":       true,
 	"nginx.com/health-checks-mandatory-queue": true,
+	UseClusterIPAnnotation:                    true,
 }
 
 var minionBlacklist = map[string]bool{
@@ -401,6 +405,14 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
 			glog.Errorf("Ingress %s/%s: Invalid value nginx.org/path-regex: got %q. Allowed values: 'case_sensitive', 'case_insensitive', 'exact'", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), pathRegex)
 		}
 	}
+
+	if useClusterIP, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, UseClusterIPAnnotation, ingEx.Ingress); exists {
+		if err != nil {
+			glog.Error(err)
+		} else {
+			cfgParams.UseClusterIP = useClusterIP
+		}
+	}
 	return cfgParams
 }
 

diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go
@@ -83,6 +83,7 @@ type ConfigParams struct {
 	SlowStart                              string
 	SSLRedirect                            bool
 	UpstreamZoneSize                       string
+	UseClusterIP                           bool
 	VariablesHashBucketSize                uint64
 	VariablesHashMaxSize                   uint64
 

diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go
@@ -511,21 +511,34 @@ func createUpstream(ingEx *IngressEx, name string, backend *networking.IngressBa
 			endps = []string{}
 		}
 
-		for _, endp := range endps {
+		if cfg.UseClusterIP {
+			fqdn := fmt.Sprintf("%s.%s.svc.cluster.local:%d", backend.Service.Name, ingEx.Ingress.Namespace, backend.Service.Port.Number)
 			upsServers = append(upsServers, version1.UpstreamServer{
-				Address:     endp,
+				Address:     fqdn,
 				MaxFails:    cfg.MaxFails,
 				MaxConns:    cfg.MaxConns,
 				FailTimeout: cfg.FailTimeout,
 				SlowStart:   cfg.SlowStart,
 				Resolve:     isExternalNameSvc,
 			})
-		}
-		if len(upsServers) > 0 {
-			sort.Slice(upsServers, func(i, j int) bool {
-				return upsServers[i].Address < upsServers[j].Address
-			})
 			ups.UpstreamServers = upsServers
+		} else {
+			for _, endp := range endps {
+				upsServers = append(upsServers, version1.UpstreamServer{
+					Address:     endp,
+					MaxFails:    cfg.MaxFails,
+					MaxConns:    cfg.MaxConns,
+					FailTimeout: cfg.FailTimeout,
+					SlowStart:   cfg.SlowStart,
+					Resolve:     isExternalNameSvc,
+				})
+			}
+			if len(upsServers) > 0 {
+				sort.Slice(upsServers, func(i, j int) bool {
+					return upsServers[i].Address < upsServers[j].Address
+				})
+				ups.UpstreamServers = upsServers
+			}
 		}
 	}
 

diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go
@@ -663,6 +663,253 @@ func TestGenerateNginxCfgForMergeableIngressesForBasicAuth(t *testing.T) {
 	}
 }
 
+func TestGenerateNginxCfgForMergeableIngressesWithUseClusterIP(t *testing.T) {
+	t.Parallel()
+	mergeableIngresses := createMergeableCafeIngress()
+	mergeableIngresses.Minions[0].Ingress.Annotations["nginx.org/use-cluster-ip"] = "true"
+
+	isPlus := false
+
+	expected := createExpectedConfigForMergeableCafeIngressWithUseClusterIP()
+	configParams := NewDefaultConfigParams(isPlus)
+
+	result, warnings := generateNginxCfgForMergeableIngresses(NginxCfgParams{
+		mergeableIngs:        mergeableIngresses,
+		apResources:          nil,
+		dosResource:          nil,
+		baseCfgParams:        configParams,
+		isPlus:               isPlus,
+		isResolverConfigured: false,
+		staticParams:         &StaticConfigParams{},
+		isWildcardEnabled:    false,
+	})
+
+	if diff := cmp.Diff(expected, result); diff != "" {
+		t.Errorf("generateNginxCfgForMergeableIngresses() returned unexpected result (-want +got):\n%s", diff)
+	}
+	if len(warnings) != 0 {
+		t.Errorf("generateNginxCfgForMergeableIngresses() returned warnings: %v", warnings)
+	}
+}
+
+func createExpectedConfigForMergeableCafeIngressWithUseClusterIP() version1.IngressNginxConfig {
+	upstreamZoneSize := "256k"
+	coffeeUpstream := version1.Upstream{
+		Name:             "default-cafe-ingress-coffee-minion-cafe.example.com-coffee-svc-80",
+		LBMethod:         "random two least_conn",
+		UpstreamZoneSize: upstreamZoneSize,
+		UpstreamServers: []version1.UpstreamServer{
+			{
+				Address:     "coffee-svc.default.svc.cluster.local:80",
+				MaxFails:    1,
+				MaxConns:    0,
+				FailTimeout: "10s",
+			},
+		},
+	}
+	teaUpstream := version1.Upstream{
+		Name:             "default-cafe-ingress-tea-minion-cafe.example.com-tea-svc-80",
+		LBMethod:         "random two least_conn",
+		UpstreamZoneSize: upstreamZoneSize,
+		UpstreamServers: []version1.UpstreamServer{
+			{
+				Address:     "10.0.0.2:80",
+				MaxFails:    1,
+				MaxConns:    0,
+				FailTimeout: "10s",
+			},
+		},
+	}
+	expected := version1.IngressNginxConfig{
+		Upstreams: []version1.Upstream{
+			coffeeUpstream,
+			teaUpstream,
+		},
+		Servers: []version1.Server{
+			{
+				Name:         "cafe.example.com",
+				ServerTokens: "on",
+				Locations: []version1.Location{
+					{
+						Path:                "/coffee",
+						ServiceName:         "coffee-svc",
+						Upstream:            coffeeUpstream,
+						ProxyConnectTimeout: "60s",
+						ProxyReadTimeout:    "60s",
+						ProxySendTimeout:    "60s",
+						ClientMaxBodySize:   "1m",
+						ProxyBuffering:      true,
+						MinionIngress: &version1.Ingress{
+							Name:      "cafe-ingress-coffee-minion",
+							Namespace: "default",
+							Annotations: map[string]string{
+								"kubernetes.io/ingress.class":      "nginx",
+								"nginx.org/mergeable-ingress-type": "minion",
+								"nginx.org/use-cluster-ip":         "true",
+							},
+						},
+						ProxySSLName: "coffee-svc.default.svc",
+					},
+					{
+						Path:                "/tea",
+						ServiceName:         "tea-svc",
+						Upstream:            teaUpstream,
+						ProxyConnectTimeout: "60s",
+						ProxyReadTimeout:    "60s",
+						ProxySendTimeout:    "60s",
+						ClientMaxBodySize:   "1m",
+						ProxyBuffering:      true,
+						MinionIngress: &version1.Ingress{
+							Name:      "cafe-ingress-tea-minion",
+							Namespace: "default",
+							Annotations: map[string]string{
+								"kubernetes.io/ingress.class":      "nginx",
+								"nginx.org/mergeable-ingress-type": "minion",
+							},
+						},
+						ProxySSLName: "tea-svc.default.svc",
+					},
+				},
+				SSL:               true,
+				SSLCertificate:    "/etc/nginx/secrets/default-cafe-secret",
+				SSLCertificateKey: "/etc/nginx/secrets/default-cafe-secret",
+				StatusZone:        "cafe.example.com",
+				HSTSMaxAge:        2592000,
+				Ports:             []int{80},
+				SSLPorts:          []int{443},
+				SSLRedirect:       true,
+				HealthChecks:      make(map[string]version1.HealthCheck),
+			},
+		},
+		Ingress: version1.Ingress{
+			Name:      "cafe-ingress-master",
+			Namespace: "default",
+			Annotations: map[string]string{
+				"kubernetes.io/ingress.class":      "nginx",
+				"nginx.org/mergeable-ingress-type": "master",
+			},
+		},
+	}
+
+	return expected
+}
+
+func createExpectedConfigForCafeIngressWithUseClusterIP() version1.IngressNginxConfig {
+	upstreamZoneSize := "256k"
+
+	coffeeUpstream := version1.Upstream{
+		Name:             "default-cafe-ingress-cafe.example.com-coffee-svc-80",
+		LBMethod:         "random two least_conn",
+		UpstreamZoneSize: upstreamZoneSize,
+		UpstreamServers: []version1.UpstreamServer{
+			{
+				Address:     "coffee-svc.default.svc.cluster.local:80",
+				MaxFails:    1,
+				MaxConns:    0,
+				FailTimeout: "10s",
+			},
+		},
+	}
+
+	teaUpstream := version1.Upstream{
+		Name:             "default-cafe-ingress-cafe.example.com-tea-svc-80",
+		LBMethod:         "random two least_conn",
+		UpstreamZoneSize: upstreamZoneSize,
+		UpstreamServers: []version1.UpstreamServer{
+			{
+				Address:     "tea-svc.default.svc.cluster.local:80",
+				MaxFails:    1,
+				MaxConns:    0,
+				FailTimeout: "10s",
+			},
+		},
+	}
+
+	expected := version1.IngressNginxConfig{
+		Upstreams: []version1.Upstream{
+			coffeeUpstream,
+			teaUpstream,
+		},
+		Servers: []version1.Server{
+			{
+				Name:         "cafe.example.com",
+				ServerTokens: "on",
+				Locations: []version1.Location{
+					{
+						Path:                "/coffee",
+						ServiceName:         "coffee-svc",
+						Upstream:            coffeeUpstream,
+						ProxyConnectTimeout: "60s",
+						ProxyReadTimeout:    "60s",
+						ProxySendTimeout:    "60s",
+						ClientMaxBodySize:   "1m",
+						ProxyBuffering:      true,
+						ProxySSLName:        "coffee-svc.default.svc",
+					},
+					{
+						Path:                "/tea",
+						ServiceName:         "tea-svc",
+						Upstream:            teaUpstream,
+						ProxyConnectTimeout: "60s",
+						ProxyReadTimeout:    "60s",
+						ProxySendTimeout:    "60s",
+						ClientMaxBodySize:   "1m",
+						ProxyBuffering:      true,
+						ProxySSLName:        "tea-svc.default.svc",
+					},
+				},
+				SSL:               true,
+				SSLCertificate:    "/etc/nginx/secrets/default-cafe-secret",
+				SSLCertificateKey: "/etc/nginx/secrets/default-cafe-secret",
+				StatusZone:        "cafe.example.com",
+				HSTSMaxAge:        2592000,
+				Ports:             []int{80},
+				SSLPorts:          []int{443},
+				SSLRedirect:       true,
+				HealthChecks:      make(map[string]version1.HealthCheck),
+			},
+		},
+		Ingress: version1.Ingress{
+			Name:      "cafe-ingress",
+			Namespace: "default",
+			Annotations: map[string]string{
+				"kubernetes.io/ingress.class": "nginx",
+				"nginx.org/use-cluster-ip":    "true",
+			},
+		},
+	}
+	return expected
+}
+
+func TestGenerateNginxCfgWithUseClusterIP(t *testing.T) {
+	t.Parallel()
+	cafeIngressEx := createCafeIngressEx()
+	cafeIngressEx.Ingress.Annotations["nginx.org/use-cluster-ip"] = "true"
+	isPlus := false
+	configParams := NewDefaultConfigParams(isPlus)
+
+	expected := createExpectedConfigForCafeIngressWithUseClusterIP()
+
+	result, warnings := generateNginxCfg(NginxCfgParams{
+		staticParams:         &StaticConfigParams{},
+		ingEx:                &cafeIngressEx,
+		apResources:          nil,
+		dosResource:          nil,
+		isMinion:             false,
+		isPlus:               false,
+		baseCfgParams:        configParams,
+		isResolverConfigured: false,
+		isWildcardEnabled:    false,
+	})
+
+	if diff := cmp.Diff(expected, result); diff != "" {
+		t.Errorf("generateNginxCfg() returned unexpected result (-want +got):\n%s", diff)
+	}
+	if len(warnings) != 0 {
+		t.Errorf("generateNginxCfg() returned warnings: %v", warnings)
+	}
+}
+
 func createMergeableCafeIngress() *MergeableIngresses {
 	master := networking.Ingress{
 		ObjectMeta: meta_v1.ObjectMeta{

diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go
@@ -69,6 +69,7 @@ const (
 	rewritesAnnotation                    = "nginx.org/rewrites"
 	stickyCookieServicesAnnotation        = "nginx.com/sticky-cookie-services"
 	pathRegexAnnotation                   = "nginx.org/path-regex"
+	useClusterIPAnnotation                = "nginx.org/use-cluster-ip"
 )
 
 const (
@@ -332,6 +333,9 @@ var (
 		pathRegexAnnotation: {
 			validatePathRegex,
 		},
+		useClusterIPAnnotation: {
+			validateBoolAnnotation,
+		},
 	}
 	annotationNames = sortedAnnotationNames(annotationValidations)
 )