Add support for gateway addresses field (#3896)

Problem: Users would like a way to specify addresses for all incoming traffic to use through the GatewaySpecAddresses field.

Solution: Implement the GatewaySpecAddresses field by adding the validated addresses to the externalIP field on the NGINX service.

Testing: GatewayStaticAddresses conformance test passes. Manually verified service externalIP field is set correctly. Manually verified conditions on Gateway.

Author: bjee19

internal/controller/handler.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"strings"
 	"sync"
 	"time"
@@ -508,6 +509,14 @@ func getGatewayAddresses(
 		addresses = append(addresses, gwSvc.Spec.ClusterIP)
 	}
 
+	for _, address := range gateway.Source.Spec.Addresses {
+		if address.Type != nil &&
+			*address.Type == gatewayv1.IPAddressType &&
+			net.ParseIP(address.Value) != nil {
+			addresses = append(addresses, address.Value)
+		}
+	}
+
 	gwAddresses := make([]gatewayv1.GatewayStatusAddress, 0, len(addresses)+len(hostnames))
 	for _, addr := range addresses {
 		statusAddr := gatewayv1.GatewayStatusAddress{

internal/controller/handler_test.go

@@ -543,6 +543,18 @@ var _ = Describe("getGatewayAddresses", func() {
 					Name:      "gateway",
 					Namespace: "test",
 				},
+				Spec: gatewayv1.GatewaySpec{
+					Addresses: []gatewayv1.GatewaySpecAddress{
+						{
+							Type:  helpers.GetPointer(gatewayv1.IPAddressType),
+							Value: "192.0.2.1",
+						},
+						{
+							Type:  helpers.GetPointer(gatewayv1.IPAddressType),
+							Value: "192.0.2.3",
+						},
+					},
+				},
 			},
 		}
 
@@ -580,9 +592,11 @@ var _ = Describe("getGatewayAddresses", func() {
 
 		addrs, err = getGatewayAddresses(context.Background(), fakeClient, &svc, gateway, "nginx")
 		Expect(err).ToNot(HaveOccurred())
-		Expect(addrs).To(HaveLen(2))
+		Expect(addrs).To(HaveLen(4))
 		Expect(addrs[0].Value).To(Equal("34.35.36.37"))
-		Expect(addrs[1].Value).To(Equal("myhost"))
+		Expect(addrs[1].Value).To(Equal("192.0.2.1"))
+		Expect(addrs[2].Value).To(Equal("192.0.2.3"))
+		Expect(addrs[3].Value).To(Equal("myhost"))
 
 		Expect(fakeClient.Delete(context.Background(), &svc)).To(Succeed())
 		// Create ClusterIP Service
@@ -601,8 +615,10 @@ var _ = Describe("getGatewayAddresses", func() {
 
 		addrs, err = getGatewayAddresses(context.Background(), fakeClient, &svc, gateway, "nginx")
 		Expect(err).ToNot(HaveOccurred())
-		Expect(addrs).To(HaveLen(1))
+		Expect(addrs).To(HaveLen(3))
 		Expect(addrs[0].Value).To(Equal("12.13.14.15"))
+		Expect(addrs[1].Value).To(Equal("192.0.2.1"))
+		Expect(addrs[2].Value).To(Equal("192.0.2.3"))
 	})
 })
 

internal/controller/provisioner/objects.go

@@ -167,7 +167,7 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
 		Annotations: maps.Clone(objectMeta.Annotations),
 	}
 
-	service, err := buildNginxService(serviceObjectMeta, nProxyCfg, ports, selectorLabels)
+	service, err := buildNginxService(serviceObjectMeta, nProxyCfg, ports, selectorLabels, gateway.Spec.Addresses)
 	if err != nil {
 		errs = append(errs, err)
 	}
@@ -517,6 +517,7 @@ func buildNginxService(
 	nProxyCfg *graph.EffectiveNginxProxy,
 	ports map[int32]struct{},
 	selectorLabels map[string]string,
+	addresses []gatewayv1.GatewaySpecAddress,
 ) (*corev1.Service, error) {
 	var serviceCfg ngfAPIv1alpha2.ServiceSpec
 	if nProxyCfg != nil && nProxyCfg.Kubernetes != nil && nProxyCfg.Kubernetes.Service != nil {
@@ -572,6 +573,8 @@ func buildNginxService(
 		},
 	}
 
+	setSvcExternalIPs(svc, addresses)
+
 	setIPFamily(nProxyCfg, svc)
 
 	setSvcLoadBalancerSettings(serviceCfg, &svc.Spec)
@@ -586,6 +589,14 @@ func buildNginxService(
 	return svc, nil
 }
 
+func setSvcExternalIPs(svc *corev1.Service, addresses []gatewayv1.GatewaySpecAddress) {
+	for _, address := range addresses {
+		if address.Type != nil && *address.Type == gatewayv1.IPAddressType {
+			svc.Spec.ExternalIPs = append(svc.Spec.ExternalIPs, address.Value)
+		}
+	}
+}
+
 func setIPFamily(nProxyCfg *graph.EffectiveNginxProxy, svc *corev1.Service) {
 	if nProxyCfg != nil && nProxyCfg.IPFamily != nil && *nProxyCfg.IPFamily != ngfAPIv1alpha2.Dual {
 		svc.Spec.IPFamilyPolicy = helpers.GetPointer(corev1.IPFamilyPolicySingleStack)

internal/controller/provisioner/objects_test.go

@@ -81,6 +81,12 @@ func TestBuildNginxResourceObjects(t *testing.T) {
 					Port: 9999,
 				},
 			},
+			Addresses: []gatewayv1.GatewaySpecAddress{
+				{
+					Type:  helpers.GetPointer(gatewayv1.IPAddressType),
+					Value: "192.0.0.2",
+				},
+			},
 		},
 	}
 
@@ -185,6 +191,7 @@ func TestBuildNginxResourceObjects(t *testing.T) {
 			TargetPort: intstr.FromInt(9999),
 		},
 	}))
+	g.Expect(svc.Spec.ExternalIPs).To(Equal([]string{"192.0.0.2"}))
 
 	depObj := objects[5]
 	dep, ok := depObj.(*appsv1.Deployment)

internal/controller/state/conditions/conditions.go

@@ -840,22 +840,36 @@ func NewGatewayInvalid(msg string) []Condition {
 	}
 }
 
-// NewGatewayUnsupportedValue returns Conditions that indicate that a field of the Gateway has an unsupported value.
-// Unsupported means that the value is not supported by the implementation or invalid.
-func NewGatewayUnsupportedValue(msg string) []Condition {
-	return []Condition{
-		{
-			Type:    string(v1.GatewayConditionAccepted),
-			Status:  metav1.ConditionFalse,
-			Reason:  string(GatewayReasonUnsupportedValue),
-			Message: msg,
-		},
-		{
-			Type:    string(v1.GatewayConditionProgrammed),
-			Status:  metav1.ConditionFalse,
-			Reason:  string(GatewayReasonUnsupportedValue),
-			Message: msg,
-		},
+// NewGatewayUnsupportedAddress returns a Condition that indicates the Gateway is not accepted because it
+// contains an address type that is not supported.
+func NewGatewayUnsupportedAddress(msg string) Condition {
+	return Condition{
+		Type:    string(v1.GatewayConditionAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1.GatewayReasonUnsupportedAddress),
+		Message: msg,
+	}
+}
+
+// NewGatewayUnusableAddress returns a Condition that indicates the Gateway is not programmed because it
+// contains an address type that can't be used.
+func NewGatewayUnusableAddress(msg string) Condition {
+	return Condition{
+		Type:    string(v1.GatewayConditionProgrammed),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1.GatewayReasonAddressNotUsable),
+		Message: msg,
+	}
+}
+
+// NewGatewayAddressNotAssigned returns a Condition that indicates the Gateway is not programmed because it
+// has not assigned an address for the Gateway.
+func NewGatewayAddressNotAssigned(msg string) Condition {
+	return Condition{
+		Type:    string(v1.GatewayConditionProgrammed),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1.GatewayReasonAddressNotAssigned),
+		Message: msg,
 	}
 }
 

internal/controller/state/graph/gateway.go

@@ -179,11 +179,14 @@ func validateGateway(gw *v1.Gateway, gc *GatewayClass, npCfg *NginxProxy) ([]con
 		conds = append(conds, conditions.NewGatewayInvalid("GatewayClass is invalid")...)
 	}
 
-	if len(gw.Spec.Addresses) > 0 {
-		path := field.NewPath("spec", "addresses")
-		valErr := field.Forbidden(path, "addresses are not supported")
-
-		conds = append(conds, conditions.NewGatewayUnsupportedValue(valErr.Error())...)
+	// Set the unaccepted conditions here, because those make the gateway invalid. We set the unprogrammed conditions
+	// elsewhere, because those do not make the gateway invalid.
+	for _, address := range gw.Spec.Addresses {
+		if address.Type == nil {
+			conds = append(conds, conditions.NewGatewayUnsupportedAddress("AddressType must be specified"))
+		} else if *address.Type != v1.IPAddressType {
+			conds = append(conds, conditions.NewGatewayUnsupportedAddress("Only AddressType IPAddress is supported"))
+		}
 	}
 
 	// we evaluate validity before validating parametersRef because an invalid parametersRef/NginxProxy does not

internal/controller/state/graph/gateway_test.go

@@ -1120,30 +1120,6 @@ func TestBuildGateway(t *testing.T) {
 			},
 			name: "port/protocol collisions",
 		},
-		{
-			gateway: createGateway(
-				gatewayCfg{
-					name:      "gateway1",
-					listeners: []v1.Listener{foo80Listener1, foo443HTTPSListener1},
-					addresses: []v1.GatewaySpecAddress{{}},
-				},
-			),
-			gatewayClass: validGC,
-			expected: map[types.NamespacedName]*Gateway{
-				{Namespace: "test", Name: "gateway1"}: {
-					Source: getLastCreatedGateway(),
-					DeploymentName: types.NamespacedName{
-						Namespace: "test",
-						Name:      controller.CreateNginxResourceName("gateway1", gcName),
-					},
-					Valid: false,
-					Conditions: conditions.NewGatewayUnsupportedValue("spec." +
-						"addresses: Forbidden: addresses are not supported",
-					),
-				},
-			},
-			name: "gateway addresses are not supported",
-		},
 		{
 			gateway:  nil,
 			expected: nil,
@@ -1484,6 +1460,59 @@ func TestBuildGateway(t *testing.T) {
 			},
 			name: "invalid gatewayclass and invalid NginxProxy",
 		},
+		{
+			name: "invalid gateway; gateway addresses type unspecified",
+			gateway: createGateway(gatewayCfg{
+				name:      "gateway-addr-unspecified",
+				listeners: []v1.Listener{foo80Listener1},
+				addresses: []v1.GatewaySpecAddress{
+					{
+						Value: "198.0.0.1",
+					},
+				},
+			}),
+			gatewayClass: validGC,
+			expected: map[types.NamespacedName]*Gateway{
+				{Namespace: "test", Name: "gateway-addr-unspecified"}: {
+					Source: getLastCreatedGateway(),
+					DeploymentName: types.NamespacedName{
+						Namespace: "test",
+						Name:      controller.CreateNginxResourceName("gateway-addr-unspecified", gcName),
+					},
+					Valid: false,
+					Conditions: []conditions.Condition{
+						conditions.NewGatewayUnsupportedAddress("AddressType must be specified"),
+					},
+				},
+			},
+		},
+		{
+			name: "invalid gateway; gateway addresses type unsupported",
+			gateway: createGateway(gatewayCfg{
+				name:      "gateway-addr-unsupported",
+				listeners: []v1.Listener{foo80Listener1},
+				addresses: []v1.GatewaySpecAddress{
+					{
+						Type:  helpers.GetPointer(v1.HostnameAddressType),
+						Value: "example.com",
+					},
+				},
+			}),
+			gatewayClass: validGC,
+			expected: map[types.NamespacedName]*Gateway{
+				{Namespace: "test", Name: "gateway-addr-unsupported"}: {
+					Source: getLastCreatedGateway(),
+					DeploymentName: types.NamespacedName{
+						Namespace: "test",
+						Name:      controller.CreateNginxResourceName("gateway-addr-unsupported", gcName),
+					},
+					Valid: false,
+					Conditions: []conditions.Condition{
+						conditions.NewGatewayUnsupportedAddress("Only AddressType IPAddress is supported"),
+					},
+				},
+			},
+		},
 	}
 
 	secretResolver := newSecretResolver(

internal/controller/status/prepare_requests.go

@@ -2,6 +2,8 @@ package status
 
 import (
 	"fmt"
+	"net"
+	"reflect"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -17,6 +19,10 @@ import (
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"
 )
 
+// unusableGatewayIPAddress 198.51.100.0 is a publicly reserved IP address specifically for documentation.
+// This is needed to give the conformance tests an example valid ip unusable address.
+const unusableGatewayIPAddress = "198.51.100.0"
+
 // PrepareRouteRequests prepares status UpdateRequests for the given Routes.
 func PrepareRouteRequests(
 	l4routes map[graph.L4RouteKey]*graph.L4Route,
@@ -329,6 +335,20 @@ func prepareGatewayRequest(
 		)
 	}
 
+	// Set the unprogrammed conditions here, because those do not make the gateway invalid.
+	// We set the unaccepted conditions elsewhere, because those do make the gateway invalid.
+	for _, address := range gateway.Source.Spec.Addresses {
+		if address.Value == "" {
+			gwConds = append(gwConds, conditions.NewGatewayAddressNotAssigned("Dynamically assigned addresses for the "+
+				"Gateway addresses field are not supported, value must be specified"))
+		} else {
+			ip := net.ParseIP(address.Value)
+			if ip == nil || reflect.DeepEqual(ip, net.ParseIP(unusableGatewayIPAddress)) {
+				gwConds = append(gwConds, conditions.NewGatewayUnusableAddress("Invalid IP address"))
+			}
+		}
+	}
+
 	apiGwConds := conditions.ConvertConditions(
 		conditions.DeduplicateConditions(gwConds),
 		gateway.Source.Generation,