Conformance tests for static Gateway addresses

CHANGELOG.md

@@ -31,6 +31,24 @@
 - [v0.1.0-rc2](#v010-rc2)
 - [v0.1.0-rc1](#v010-rc1)
 
+# Unreleased
+
+## Other Changes
+
+- The condition reason `GatewayReasonUnsupportedAddress` for `Accepted` now ONLY
+  applies when an address type is provided for a `Gateway` which it does not
+  support.
+  (#2412 @shaneutt)
+- The condition reason `GatewayReasonAddressNotAssigned` for `Programmed` now
+  ONLY applies to problems with dynamic address allocation.
+  (#2412 @shaneutt)
+- The condition reason `GatewayReasonAddressNotUsable` for `Programmed` has been
+  added to deal with situations where a static address has been provided for a
+  Gateway which is of a supported type, and is syntatically valid, but for some
+  reason it can not be used for this Gateway (e.g. the address is already in use
+  on the network).
+  (#2412 @shaneutt)
+
 # v0.8.1
 
 This is a patch release that includes small bug fixes and a new conformance test

apis/v1beta1/gateway_types.go

@@ -510,7 +510,7 @@ type GatewayAddress struct {
 	Value string `json:"value"`
 }
 
-// GatewayStatusAddress describes an address that is bound to a Gateway.
+// GatewayStatusAddress describes a network address that is bound to a Gateway.
 //
 // +kubebuilder:validation:XValidation:message="Hostname value must only contain valid characters (matching ^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$)",rule="self.type == 'Hostname' ? self.value.matches(r\"\"\"^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"\"\"): true"
 type GatewayStatusAddress struct {
@@ -532,10 +532,15 @@ type GatewayStatusAddress struct {
 
 // GatewayStatus defines the observed state of Gateway.
 type GatewayStatus struct {
-	// Addresses lists the IP addresses that have actually been
-	// bound to the Gateway. These addresses may differ from the
-	// addresses in the Spec, e.g. if the Gateway automatically
-	// assigns an address from a reserved pool.
+	// Addresses lists the network addresses that have been bound to the
+	// Gateway.
+	//
+	// This list may differ from the addresses provided in the spec under some
+	// conditions:
+	//
+	//   * no addresses are specified, all addresses are dynamically assigned
+	//   * a combination of specified and dynamic addresses are assigned
+	//   * a specified address was unusable (e.g. already in use)
 	//
 	// +optional
 	// <gateway:validateIPAddress>
@@ -625,11 +630,34 @@ const (
 	// resources are available.
 	GatewayReasonNoResources GatewayConditionReason = "NoResources"
 
-	// This reason is used with the "Programmed" condition when none of the requested
-	// addresses have been assigned to the Gateway. This reason can be used to
-	// express a range of circumstances, including (but not limited to) IPAM
-	// address exhaustion, address not yet allocated, or a named address not being found.
+	// This reason is used with the "Programmed" condition when the underlying
+	// implementation and network have yet to dynamically assign addresses for a
+	// Gateway.
+	//
+	// Some example situations where this reason can be used:
+	//
+	//   * IPAM address exhaustion
+	//   * Address not yet allocated
+	//
+	// When this reason is used the implementation SHOULD provide a clear
+	// message explaining the underlying problem, ideally with some hints as to
+	// what actions can be taken that might resolve the problem.
 	GatewayReasonAddressNotAssigned GatewayConditionReason = "AddressNotAssigned"
+
+	// This reason is used with the "Programmed" condition when the underlying
+	// implementation (and possibly, network) are unable to use an address that
+	// was provided in the Gateway specification.
+	//
+	// Some example situations where this reason can be used:
+	//
+	//   * a named address not being found
+	//   * a provided static address can't be used
+	//   * the address is already in use
+	//
+	// When this reason is used the implementation SHOULD provide prescriptive
+	// information on which address is causing the problem and how to resolve it
+	// in the condition message.
+	GatewayReasonAddressNotUsable GatewayConditionReason = "AddressNotUsable"
 )
 
 const (
@@ -675,12 +703,9 @@ const (
 	// the Gateway.
 	GatewayReasonPending GatewayConditionReason = "Pending"
 
-	// This reason is used with the "Accepted" condition when the Gateway could not be configured
-	// because the requested address is not supported. This reason could be used in a number of
-	// instances, including:
-	//
-	// * The address is already in use.
-	// * The type of address is not supported by the implementation.
+	// This reason is used with the "Accepted" condition to indicate that the
+	// Gateway could not be accepted because an address that was provided is a
+	// type which is not supported by the implementation.
 	GatewayReasonUnsupportedAddress GatewayConditionReason = "UnsupportedAddress"
 )
 

conformance/tests/gateway-static-addresses.go

@@ -0,0 +1,177 @@
+/*
+Copyright 2023 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 tests
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, GatewayStaticAddresses)
+}
+
+// GatewayStaticAddresses tests the implementation's support of deploying
+// Gateway resources with static addresses, or in other words addresses
+// provided via the specification rather than relying on the underlying
+// implementation/network to dynamically assign the Gateway an address.
+//
+// Running this test against your own implementation is currently a little bit
+// messy, as at the time of writing we didn't have great ways to provide the
+// test suite with things like known good, or known bad addresses to run the
+// test with (as we obviously can't determine that for the implementation).
+//
+// As such, if you're trying to enable this test for yourself and you're getting
+// confused about how to provide addresses, you'll actually do that in the
+// conformance test suite BEFORE you even set up and run your tests. Make sure
+// you populate the following test suite fields:
+//
+//   - suite.UsableNetworkAddresses
+//   - suite.UnusableNetworkAddresses
+//
+// With appropriate network addresses for your network environment.
+var GatewayStaticAddresses = suite.ConformanceTest{
+	ShortName:   "GatewayStaticAddresses",
+	Description: "A Gateway in the gateway-conformance-infra namespace should be able to use previously determined addresses.",
+	Features: []suite.SupportedFeature{
+		suite.SupportGateway,
+		suite.SupportGatewayStaticAddresses,
+	},
+	Manifests: []string{
+		"tests/gateway-static-addresses.yaml",
+	},
+	Test: func(t *testing.T, s *suite.ConformanceTestSuite) {
+		gwNN := types.NamespacedName{
+			Name:      "gateway-static-addresses",
+			Namespace: "gateway-conformance-infra",
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+		defer cancel()
+
+		t.Logf("waiting for namespace %s and Gateway %s to be ready for testing", gwNN.Namespace, gwNN.Name)
+		kubernetes.GatewayMustHaveLatestConditions(t, s.Client, s.TimeoutConfig, gwNN)
+
+		t.Logf("retrieving Gateway %s/%s and noting the provided addresses", gwNN.Namespace, gwNN.Name)
+		currentGW := &v1beta1.Gateway{}
+		err := s.Client.Get(ctx, gwNN, currentGW)
+		require.NoError(t, err, "error getting Gateway: %v", err)
+		require.Len(t, currentGW.Spec.Addresses, 3, "expected 3 addresses on the Gateway, one invalid, one usable and one unusable. somehow got %d", len(currentGW.Spec.Addresses))
+		invalidAddress := currentGW.Spec.Addresses[0]
+		unusableAddress := currentGW.Spec.Addresses[1]
+		usableAddress := currentGW.Spec.Addresses[2]
+
+		t.Logf("verifying that the Gateway %s/%s is NOT accepted due to an address type that the implementation doesn't support", gwNN.Namespace, gwNN.Name)
+		kubernetes.GatewayMustHaveCondition(t, s.Client, s.TimeoutConfig, gwNN, metav1.Condition{
+			Type:   string(v1beta1.GatewayConditionAccepted),
+			Status: metav1.ConditionFalse,
+			Reason: string(v1beta1.GatewayReasonUnsupportedAddress),
+		})
+
+		t.Logf("patching Gateway %s/%s to remove the invalid address %s", gwNN.Namespace, gwNN.Name, invalidAddress.Value)
+		updatedGW := currentGW.DeepCopy()
+		updatedGW.Spec.Addresses = filterAddr(currentGW.Spec.Addresses, invalidAddress)
+		err = s.Client.Patch(ctx, updatedGW, client.MergeFrom(currentGW))
+		require.NoError(t, err, "failed to patch Gateway: %v", err)
+		kubernetes.GatewayMustHaveLatestConditions(t, s.Client, s.TimeoutConfig, gwNN)
+
+		t.Logf("verifying that the Gateway %s/%s is now accepted, but is not programmed due to an address that can't be used", gwNN.Namespace, gwNN.Name)
+		err = s.Client.Get(ctx, gwNN, currentGW)
+		require.NoError(t, err, "error getting Gateway: %v", err)
+		kubernetes.GatewayMustHaveCondition(t, s.Client, s.TimeoutConfig, gwNN, metav1.Condition{
+			Type:   string(v1beta1.GatewayConditionAccepted),
+			Status: metav1.ConditionTrue,
+			Reason: string(v1beta1.GatewayReasonAccepted),
+		})
+		kubernetes.GatewayMustHaveCondition(t, s.Client, s.TimeoutConfig, gwNN, metav1.Condition{
+			Type:   string(v1beta1.GatewayConditionProgrammed),
+			Status: metav1.ConditionFalse,
+			Reason: string(v1beta1.GatewayReasonAddressNotUsable),
+		})
+
+		t.Logf("patching Gateway %s/%s to remove the unusable address %s", gwNN.Namespace, gwNN.Name, unusableAddress.Value)
+		updatedGW = currentGW.DeepCopy()
+		updatedGW.Spec.Addresses = filterAddr(currentGW.Spec.Addresses, unusableAddress)
+		err = s.Client.Patch(ctx, updatedGW, client.MergeFrom(currentGW))
+		require.NoError(t, err, "failed to patch Gateway: %v", err)
+		kubernetes.GatewayMustHaveLatestConditions(t, s.Client, s.TimeoutConfig, gwNN)
+
+		t.Logf("verifying that the Gateway %s/%s is accepted and programmed with the usable static address %s assigned", gwNN.Namespace, gwNN.Name, usableAddress.Value)
+		err = s.Client.Get(ctx, gwNN, currentGW)
+		require.NoError(t, err, "error getting Gateway: %v", err)
+		kubernetes.GatewayMustHaveCondition(t, s.Client, s.TimeoutConfig, gwNN, metav1.Condition{
+			Type:   string(v1beta1.GatewayConditionAccepted),
+			Status: metav1.ConditionTrue,
+			Reason: string(v1beta1.GatewayReasonAccepted),
+		})
+		kubernetes.GatewayMustHaveCondition(t, s.Client, s.TimeoutConfig, gwNN, metav1.Condition{
+			Type:   string(v1beta1.GatewayConditionProgrammed),
+			Status: metav1.ConditionTrue,
+			Reason: string(v1beta1.GatewayReasonProgrammed),
+		})
+		kubernetes.GatewayStatusMustHaveListeners(t, s.Client, s.TimeoutConfig, gwNN, finalExpectedListenerState)
+		require.Len(t, currentGW.Spec.Addresses, 1, "expected only 1 address left specified on Gateway")
+		require.Len(t, currentGW.Status.Addresses, 1, "one usable address was provided, so it should be the one reflected in status")
+		require.Equal(t, usableAddress.Type, currentGW.Status.Addresses[0].Type, "expected address type to match the usable address")
+		require.Equal(t, usableAddress.Value, currentGW.Status.Addresses[0].Value, "expected usable address to be assigned")
+	},
+}
+
+// -----------------------------------------------------------------------------
+// Private Helper Functions
+// -----------------------------------------------------------------------------
+
+func filterAddr(addrs []v1beta1.GatewayAddress, filter v1beta1.GatewayAddress) (newAddrs []v1beta1.GatewayAddress) {
+	for _, addr := range addrs {
+		if addr.Value != filter.Value {
+			newAddrs = append(newAddrs, addr)
+		}
+	}
+	return
+}
+
+var finalExpectedListenerState = []v1beta1.ListenerStatus{
+	{
+		Name: v1beta1.SectionName("http"),
+		SupportedKinds: []v1beta1.RouteGroupKind{{
+			Group: (*v1beta1.Group)(&v1beta1.GroupVersion.Group),
+			Kind:  v1beta1.Kind("HTTPRoute"),
+		}},
+		Conditions: []metav1.Condition{
+			{
+				Type:   string(v1beta1.ListenerConditionAccepted),
+				Status: metav1.ConditionTrue,
+				Reason: "", // any reason
+			},
+			{
+				Type:   string(v1beta1.ListenerConditionResolvedRefs),
+				Status: metav1.ConditionTrue,
+				Reason: "", // any reason
+			},
+		},
+		AttachedRoutes: 0,
+	},
+}