Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions internal/controller/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"net"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -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{
Expand Down
22 changes: 19 additions & 3 deletions internal/controller/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
},
}

Expand Down Expand Up @@ -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
Expand All @@ -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"))
})
})

Expand Down
13 changes: 12 additions & 1 deletion internal/controller/provisioner/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -572,6 +573,8 @@ func buildNginxService(
},
}

setSvcExternalIPs(svc, addresses)

setIPFamily(nProxyCfg, svc)

setSvcLoadBalancerSettings(serviceCfg, &svc.Spec)
Expand All @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions internal/controller/provisioner/objects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ func TestBuildNginxResourceObjects(t *testing.T) {
Port: 9999,
},
},
Addresses: []gatewayv1.GatewaySpecAddress{
{
Type: helpers.GetPointer(gatewayv1.IPAddressType),
Value: "192.0.0.2",
},
},
},
}

Expand Down Expand Up @@ -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)
Expand Down
46 changes: 30 additions & 16 deletions internal/controller/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
13 changes: 8 additions & 5 deletions internal/controller/state/graph/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
77 changes: 53 additions & 24 deletions internal/controller/state/graph/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions internal/controller/status/prepare_requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package status

import (
"fmt"
"net"
"reflect"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading