Support IP and Hostname trusted addresses (#2619)

Problem: Our initial implementation of rewriting client IP only supported CIDRs for the trusted addresses field.

Solution: Add support for IP addresses and hostnames

apis/v1alpha1/nginxproxy_types.go

@@ -27,20 +27,6 @@ type NginxProxyList struct {
 	Items           []NginxProxy `json:"items"`
 }
 
-// IPFamilyType specifies the IP family to be used by NGINX.
-//
-// +kubebuilder:validation:Enum=dual;ipv4;ipv6
-type IPFamilyType string
-
-const (
-	// Dual specifies that NGINX will use both IPv4 and IPv6.
-	Dual IPFamilyType = "dual"
-	// IPv4 specifies that NGINX will use only IPv4.
-	IPv4 IPFamilyType = "ipv4"
-	// IPv6 specifies that NGINX will use only IPv6.
-	IPv6 IPFamilyType = "ipv6"
-)
-
 // NginxProxySpec defines the desired state of the NginxProxy.
 type NginxProxySpec struct {
 	// IPFamily specifies the IP family to be used by the NGINX.
@@ -154,11 +140,11 @@ type RewriteClientIP struct {
 	// If no addresses are provided, NGINX will not rewrite the client IP information.
 	// Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from
 	// This field is required if mode is set.
-	// +kubebuilder:validation:MaxItems=16
-	// +listType=map
-	// +listMapKey=type
 	//
 	// +optional
+	// +listType=map
+	// +listMapKey=type
+	// +kubebuilder:validation:MaxItems=16
 	TrustedAddresses []Address `json:"trustedAddresses,omitempty"`
 }
 
@@ -179,27 +165,40 @@ const (
 	RewriteClientIPModeXForwardedFor RewriteClientIPModeType = "XForwardedFor"
 )
 
+// IPFamilyType specifies the IP family to be used by NGINX.
+//
+// +kubebuilder:validation:Enum=dual;ipv4;ipv6
+type IPFamilyType string
+
+const (
+	// Dual specifies that NGINX will use both IPv4 and IPv6.
+	Dual IPFamilyType = "dual"
+	// IPv4 specifies that NGINX will use only IPv4.
+	IPv4 IPFamilyType = "ipv4"
+	// IPv6 specifies that NGINX will use only IPv6.
+	IPv6 IPFamilyType = "ipv6"
+)
+
 // Address is a struct that specifies address type and value.
 type Address struct {
 	// Type specifies the type of address.
-	// Default is "cidr" which specifies that the address is a CIDR block.
-	//
-	// +optional
-	// +kubebuilder:default:=cidr
-	Type AddressType `json:"type,omitempty"`
+	Type AddressType `json:"type"`
 
 	// Value specifies the address value.
-	//
-	// +optional
-	Value string `json:"value,omitempty"`
+	Value string `json:"value"`
 }
 
 // AddressType specifies the type of address.
-// +kubebuilder:validation:Enum=cidr
+// +kubebuilder:validation:Enum=CIDR;IPAddress;Hostname
 type AddressType string
 
 const (
-	// AddressTypeCIDR specifies that the address is a CIDR block.
-	// kubebuilder:validation:Pattern=`^[\.a-zA-Z0-9:]*(\/([0-9]?[0-9]?[0-9]))$`
-	AddressTypeCIDR AddressType = "cidr"
+	// CIDRAddressType specifies that the address is a CIDR block.
+	CIDRAddressType AddressType = "CIDR"
+
+	// IPAddressType specifies that the address is an IP address.
+	IPAddressType AddressType = "IPAddress"
+
+	// HostnameAddressType specifies that the address is a Hostname.
+	HostnameAddressType AddressType = "Hostname"
 )

charts/nginx-gateway-fabric/values.schema.json

@@ -73,6 +73,47 @@
               "required": [],
               "type": "string"
             },
+            "rewriteClientIP": {
+              "description": "RewriteClientIP defines configuration for rewriting the client IP to the original client's IP.",
+              "properties": {
+                "mode": {
+                  "enum": [
+                    "ProxyProtocol",
+                    "XForwardedFor"
+                  ],
+                  "required": [],
+                  "type": "string"
+                },
+                "setIPRecursively": {
+                  "required": [],
+                  "type": "boolean"
+                },
+                "trustedAddresses": {
+                  "items": {
+                    "properties": {
+                      "type": {
+                        "enum": [
+                          "CIDR",
+                          "IPAddress",
+                          "Hostname"
+                        ],
+                        "required": [],
+                        "type": "string"
+                      },
+                      "value": {
+                        "required": [],
+                        "type": "string"
+                      }
+                    },
+                    "required": []
+                  },
+                  "required": [],
+                  "type": "array"
+                }
+              },
+              "required": [],
+              "type": "object"
+            },
             "telemetry": {
               "description": "Telemetry specifies the OpenTelemetry configuration.",
               "properties": {

charts/nginx-gateway-fabric/values.yaml

@@ -142,6 +142,29 @@ nginx:
   #       - ipv4
   #       - ipv6
   #       - dual
+  #   rewriteClientIP:
+  #     type: object
+  #     description: RewriteClientIP defines configuration for rewriting the client IP to the original client's IP.
+  #     properties:
+  #       mode:
+  #         type: string
+  #         enum:
+  #           - ProxyProtocol
+  #           - XForwardedFor
+  #       setIPRecursively:
+  #         type: boolean
+  #       trustedAddresses:
+  #         type: array
+  #         items:
+  #           properties:
+  #             type:
+  #               type: string
+  #               enum:
+  #                 - CIDR
+  #                 - IPAddress
+  #                 - Hostname
+  #             value:
+  #               type: string
   #   telemetry:
   #     type: object
   #     description: Telemetry specifies the OpenTelemetry configuration.

config/crd/bases/gateway.nginx.org_nginxproxies.yaml

@@ -105,16 +105,18 @@ spec:
                         and value.
                       properties:
                         type:
-                          default: cidr
-                          description: |-
-                            Type specifies the type of address.
-                            Default is "cidr" which specifies that the address is a CIDR block.
+                          description: Type specifies the type of address.
                           enum:
-                          - cidr
+                          - CIDR
+                          - IPAddress
+                          - Hostname
                           type: string
                         value:
                           description: Value specifies the address value.
                           type: string
+                      required:
+                      - type
+                      - value
                       type: object
                     maxItems: 16
                     type: array

deploy/crds.yaml

@@ -690,16 +690,18 @@ spec:
                         and value.
                       properties:
                         type:
-                          default: cidr
-                          description: |-
-                            Type specifies the type of address.
-                            Default is "cidr" which specifies that the address is a CIDR block.
+                          description: Type specifies the type of address.
                           enum:
-                          - cidr
+                          - CIDR
+                          - IPAddress
+                          - Hostname
                           type: string
                         value:
                           description: Value specifies the address value.
                           type: string
+                      required:
+                      - type
+                      - value
                       type: object
                     maxItems: 16
                     type: array

internal/mode/static/state/dataplane/configuration_test.go

@@ -2201,7 +2201,7 @@ func TestBuildConfiguration(t *testing.T) {
 								SetIPRecursively: helpers.GetPointer(true),
 								TrustedAddresses: []ngfAPI.Address{
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "1.1.1.1/32",
 									},
 								},
@@ -3651,7 +3651,7 @@ func TestBuildRewriteIPSettings(t *testing.T) {
 								Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol),
 								TrustedAddresses: []ngfAPI.Address{
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "10.9.9.4/32",
 									},
 								},
@@ -3678,7 +3678,7 @@ func TestBuildRewriteIPSettings(t *testing.T) {
 								Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor),
 								TrustedAddresses: []ngfAPI.Address{
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "76.89.90.11/24",
 									},
 								},
@@ -3705,19 +3705,19 @@ func TestBuildRewriteIPSettings(t *testing.T) {
 								Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeXForwardedFor),
 								TrustedAddresses: []ngfAPI.Address{
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "5.5.5.5/12",
 									},
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "1.1.1.1/26",
 									},
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "2.2.2.2/32",
 									},
 									{
-										Type:  ngfAPI.AddressTypeCIDR,
+										Type:  ngfAPI.CIDRAddressType,
 										Value: "3.3.3.3/24",
 									},
 								},

internal/mode/static/state/graph/nginxproxy.go

@@ -172,23 +172,33 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList {
 		}
 
 		for _, addr := range rewriteClientIP.TrustedAddresses {
+			valuePath := trustedAddressesPath.Child("value")
+
 			switch addr.Type {
-			case ngfAPI.AddressTypeCIDR:
-				if err := k8svalidation.IsValidCIDR(trustedAddressesPath, addr.Value); err != nil {
-					allErrs = append(
-						allErrs,
-						field.Invalid(trustedAddressesPath.Child(addr.Value),
-							addr,
-							err.ToAggregate().Error(),
-						),
-					)
+			case ngfAPI.CIDRAddressType:
+				if err := k8svalidation.IsValidCIDR(valuePath, addr.Value); err != nil {
+					allErrs = append(allErrs, err...)
+				}
+			case ngfAPI.IPAddressType:
+				if err := k8svalidation.IsValidIP(valuePath, addr.Value); err != nil {
+					allErrs = append(allErrs, err...)
+				}
+			case ngfAPI.HostnameAddressType:
+				if errs := k8svalidation.IsDNS1123Subdomain(addr.Value); len(errs) > 0 {
+					for _, e := range errs {
+						allErrs = append(allErrs, field.Invalid(valuePath, addr.Value, e))
+					}
 				}
 			default:
 				allErrs = append(
 					allErrs,
 					field.NotSupported(trustedAddressesPath.Child("type"),
 						addr.Type,
-						[]string{string(ngfAPI.AddressTypeCIDR)},
+						[]string{
+							string(ngfAPI.CIDRAddressType),
+							string(ngfAPI.IPAddressType),
+							string(ngfAPI.HostnameAddressType),
+						},
 					),
 				)
 			}