Set GatewayClass status for ignored GatewayClasses (#804)

Problem: It is expected that any GatewayClass that references our controller should have its status set properly, even if not used by our controller.

Solution: Both the provisioner and controller will now add Accepted status False and ObservedGeneration to any GatewayClass that references our controller but is not configured to be used by our controller.

cmd/gateway/commands.go

@@ -193,6 +193,7 @@ func createProvisionerModeCommand() *cobra.Command {
 			return provisioner.StartManager(provisioner.Config{
 				Logger:           logger,
 				GatewayClassName: gatewayClassName.value,
+				GatewayCtlrName:  gatewayCtlrName.value,
 			})
 		},
 	}

conformance/Makefile

@@ -1,7 +1,7 @@
 NKG_TAG = edge
 NKG_PREFIX = nginx-kubernetes-gateway
 GATEWAY_CLASS = nginx
-SUPPORTED_FEATURES = HTTPRoute,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect
+SUPPORTED_FEATURES = HTTPRoute,HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,GatewayClassObservedGenerationBump
 KIND_KUBE_CONFIG=$${HOME}/.kube/kind/config
 TAG = latest
 PREFIX = conformance-test-runner

docs/gateway-api-compatibility.md

@@ -43,7 +43,10 @@ Fields:
     * `parametersRef` - not supported.
     * `description` - supported.
 * `status`
-    * `conditions` - partially supported.
+    * `conditions` - supported (Condition/Status/Reason):
+      * `Accepted/True/Accepted`
+      * `Accepted/False/InvalidParameters`
+      * `Accepted/False/GatewayClassConflict`: Custom reason for when the GatewayClass references this controller, but a different GatewayClass name is provided to the controller via the command-line argument.
 
 ### Gateway
 

internal/manager/manager.go

@@ -80,9 +80,7 @@ func Start(cfg config.Config) error {
 		{
 			objectType: &gatewayv1beta1.GatewayClass{},
 			options: []controller.Option{
-				controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(
-					types.NamespacedName{Name: cfg.GatewayClassName},
-				)),
+				controller.WithK8sPredicate(predicate.GatewayClassPredicate{ControllerName: cfg.GatewayCtlrName}),
 			},
 		},
 		{

internal/manager/predicate/gatewayclass.go

@@ -0,0 +1,47 @@
+package predicate
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+)
+
+// GatewayClassPredicate implements a predicate function based on the controllerName of a GatewayClass.
+// This predicate will skip events for GatewayClasses that don't reference this controller.
+type GatewayClassPredicate struct {
+	predicate.Funcs
+	ControllerName string
+}
+
+// Create implements default CreateEvent filter for validating a GatewayClass controllerName.
+func (gcp GatewayClassPredicate) Create(e event.CreateEvent) bool {
+	if e.Object == nil {
+		return false
+	}
+
+	gc, ok := e.Object.(*v1beta1.GatewayClass)
+	if !ok {
+		return false
+	}
+
+	return string(gc.Spec.ControllerName) == gcp.ControllerName
+}
+
+// Update implements default UpdateEvent filter for validating a GatewayClass controllerName.
+func (gcp GatewayClassPredicate) Update(e event.UpdateEvent) bool {
+	if e.ObjectOld != nil {
+		gcOld, ok := e.ObjectOld.(*v1beta1.GatewayClass)
+		if ok && string(gcOld.Spec.ControllerName) == gcp.ControllerName {
+			return true
+		}
+	}
+
+	if e.ObjectNew != nil {
+		gcNew, ok := e.ObjectNew.(*v1beta1.GatewayClass)
+		if ok && string(gcNew.Spec.ControllerName) == gcp.ControllerName {
+			return true
+		}
+	}
+
+	return false
+}

internal/manager/predicate/gatewayclass_test.go

@@ -0,0 +1,34 @@
+package predicate
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+)
+
+func TestGatewayClassPredicate(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	p := GatewayClassPredicate{ControllerName: "nginx-ctlr"}
+
+	gc := &v1beta1.GatewayClass{
+		Spec: v1beta1.GatewayClassSpec{
+			ControllerName: "nginx-ctlr",
+		},
+	}
+
+	g.Expect(p.Create(event.CreateEvent{Object: gc})).To(BeTrue())
+	g.Expect(p.Update(event.UpdateEvent{ObjectNew: gc})).To(BeTrue())
+
+	gc2 := &v1beta1.GatewayClass{
+		Spec: v1beta1.GatewayClassSpec{
+			ControllerName: "unknown",
+		},
+	}
+	g.Expect(p.Create(event.CreateEvent{Object: gc2})).To(BeFalse())
+	g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc, ObjectNew: gc2})).To(BeTrue())
+	g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc2, ObjectNew: gc})).To(BeTrue())
+	g.Expect(p.Update(event.UpdateEvent{ObjectOld: gc2, ObjectNew: gc2})).To(BeFalse())
+}

internal/manager/predicate/service_test.go

@@ -3,6 +3,7 @@ package predicate
 import (
 	"testing"
 
+	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -224,34 +225,24 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) {
 	p := ServicePortsChangedPredicate{}
 
 	for _, tc := range testcases {
-		update := p.Update(event.UpdateEvent{
-			ObjectOld: tc.objectOld,
-			ObjectNew: tc.objectNew,
-		})
+		t.Run(tc.msg, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+			update := p.Update(event.UpdateEvent{
+				ObjectOld: tc.objectOld,
+				ObjectNew: tc.objectNew,
+			})
 
-		if update != tc.expUpdate {
-			t.Errorf(
-				"ServicePortsChangedPredicate.Update() mismatch for %q; got %t, expected %t",
-				tc.msg,
-				update,
-				tc.expUpdate,
-			)
-		}
+			g.Expect(update).To(Equal(tc.expUpdate))
+		})
 	}
 }
 
 func TestServicePortsChangedPredicate(t *testing.T) {
-	p := ServicePortsChangedPredicate{}
-
-	if !p.Delete(event.DeleteEvent{Object: &v1.Service{}}) {
-		t.Errorf("ServicePortsChangedPredicate.Delete() returned false; expected true")
-	}
+	g := NewGomegaWithT(t)
 
-	if !p.Create(event.CreateEvent{Object: &v1.Service{}}) {
-		t.Errorf("ServicePortsChangedPredicate.Create() returned false; expected true")
-	}
+	p := ServicePortsChangedPredicate{}
 
-	if !p.Generic(event.GenericEvent{Object: &v1.Service{}}) {
-		t.Errorf("ServicePortsChangedPredicate.Generic() returned false; expected true")
-	}
+	g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeTrue())
+	g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeTrue())
+	g.Expect(p.Generic(event.GenericEvent{Object: &v1.Service{}})).To(BeTrue())
 }

internal/provisioner/handler.go

@@ -53,17 +53,28 @@ func newEventHandler(
 	}
 }
 
-func (h *eventHandler) ensureGatewayClassAccepted(ctx context.Context) {
-	gc, exist := h.store.gatewayClasses[types.NamespacedName{Name: h.gcName}]
-	if !exist {
-		panic(fmt.Errorf("GatewayClass %s must exist", h.gcName))
+func (h *eventHandler) setGatewayClassStatuses(ctx context.Context) {
+	statuses := status.Statuses{
+		GatewayClassStatuses: make(status.GatewayClassStatuses),
 	}
 
-	statuses := status.Statuses{
-		GatewayClassStatus: &status.GatewayClassStatus{
-			Conditions:         conditions.NewDefaultGatewayClassConditions(),
+	var gcExists bool
+	for nsname, gc := range h.store.gatewayClasses {
+		var conds []conditions.Condition
+		if gc.Name == h.gcName {
+			gcExists = true
+			conds = conditions.NewDefaultGatewayClassConditions()
+		} else {
+			conds = []conditions.Condition{conditions.NewGatewayClassConflict()}
+		}
+
+		statuses.GatewayClassStatuses[nsname] = status.GatewayClassStatus{
+			Conditions:         conds,
 			ObservedGeneration: gc.Generation,
-		},
+		}
+	}
+	if !gcExists {
+		panic(fmt.Errorf("GatewayClass %s must exist", h.gcName))
 	}
 
 	h.statusUpdater.Update(ctx, statuses)
@@ -133,7 +144,7 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) {
 
 func (h *eventHandler) HandleEventBatch(ctx context.Context, batch events.EventBatch) {
 	h.store.update(batch)
-	h.ensureGatewayClassAccepted(ctx)
+	h.setGatewayClassStatuses(ctx)
 	h.ensureDeploymentsMatchGateways(ctx)
 }
 

internal/provisioner/handler_test.go

@@ -19,6 +19,7 @@ import (
 	embeddedfiles "github.com/nginxinc/nginx-kubernetes-gateway"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes"
 )
@@ -272,6 +273,46 @@ var _ = Describe("handler", func() {
 				Expect(deps.Items).To(HaveLen(0))
 			})
 		})
+
+		When("upserting GatewayClass that is not set in command-line argument", func() {
+			It("should set the proper status if this controller is referenced", func() {
+				gc := &v1beta1.GatewayClass{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "unknown-gc",
+					},
+					Spec: v1beta1.GatewayClassSpec{
+						ControllerName: "test.example.com",
+					},
+				}
+				err := k8sclient.Create(context.Background(), gc)
+				Expect(err).ShouldNot(HaveOccurred())
+
+				batch := []interface{}{
+					&events.UpsertEvent{
+						Resource: gc,
+					},
+				}
+
+				handler.HandleEventBatch(context.Background(), batch)
+
+				unknownGC := &v1beta1.GatewayClass{}
+				err = k8sclient.Get(context.Background(), client.ObjectKeyFromObject(gc), unknownGC)
+				Expect(err).ShouldNot(HaveOccurred())
+
+				expectedConditions := []metav1.Condition{
+					{
+						Type:               string(v1beta1.GatewayClassConditionStatusAccepted),
+						Status:             metav1.ConditionFalse,
+						ObservedGeneration: 0,
+						LastTransitionTime: fakeClockTime,
+						Reason:             string(conditions.GatewayClassReasonGatewayClassConflict),
+						Message:            string(conditions.GatewayClassMessageGatewayClassConflict),
+					},
+				}
+
+				Expect(unknownGC.Status.Conditions).To(Equal(expectedConditions))
+			})
+		})
 	})
 
 	Describe("Edge cases", func() {
@@ -392,20 +433,20 @@ var _ = Describe("handler", func() {
 				Expect(handle).Should(Panic())
 			})
 		})
-	})
 
-	When("upserting Gateway with broken static Deployment YAML", func() {
-		It("it should panic", func() {
-			handler = newEventHandler(
-				gcName,
-				statusUpdater,
-				k8sclient,
-				zap.New(),
-				[]byte("broken YAML"),
-			)
+		When("upserting Gateway with broken static Deployment YAML", func() {
+			It("it should panic", func() {
+				handler = newEventHandler(
+					gcName,
+					statusUpdater,
+					k8sclient,
+					zap.New(),
+					[]byte("broken YAML"),
+				)
 
-			itShouldUpsertGatewayClass()
-			itShouldPanicWhenUpsertingGateway(types.NamespacedName{Namespace: "test-ns", Name: "test-gw"})
+				itShouldUpsertGatewayClass()
+				itShouldPanicWhenUpsertingGateway(types.NamespacedName{Namespace: "test-ns", Name: "test-gw"})
+			})
 		})
 	})
 })

internal/provisioner/manager.go

@@ -7,7 +7,6 @@ import (
 	v1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	ctlr "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -17,14 +16,15 @@ import (
 	embeddedfiles "github.com/nginxinc/nginx-kubernetes-gateway"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/controller"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
-	"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/filter"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 )
 
 // Config is configuration for the provisioner mode.
 type Config struct {
 	Logger           logr.Logger
 	GatewayClassName string
+	GatewayCtlrName  string
 }
 
 // StartManager starts a Manager for the provisioner mode, which provisions
@@ -60,9 +60,7 @@ func StartManager(cfg Config) error {
 		{
 			objectType: &gatewayv1beta1.GatewayClass{},
 			options: []controller.Option{
-				controller.WithNamespacedNameFilter(
-					filter.CreateSingleResourceFilter(types.NamespacedName{Name: cfg.GatewayClassName}),
-				),
+				controller.WithK8sPredicate(predicate.GatewayClassPredicate{ControllerName: cfg.GatewayCtlrName}),
 			},
 		},
 		{

internal/state/conditions/conditions.go

@@ -8,6 +8,15 @@ import (
 )
 
 const (
+	// GatewayClassReasonGatewayClassConflict indicates there are multiple GatewayClass resources
+	// that reference this controller, and we ignored the resource in question and picked the
+	// GatewayClass that is referenced in the command-line argument.
+	// This reason is used with GatewayClassConditionAccepted (false).
+	GatewayClassReasonGatewayClassConflict v1beta1.GatewayClassConditionReason = "GatewayClassConflict"
+
+	// GatewayClassMessageGatewayClassConflict is a message that describes GatewayClassReasonGatewayClassConflict.
+	GatewayClassMessageGatewayClassConflict = "The resource is ignored due to a conflicting GatewayClass resource"
+
 	// ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener
 	// is invalid or not supported.
 	ListenerReasonUnsupportedValue v1beta1.ListenerConditionReason = "UnsupportedValue"
@@ -427,6 +436,17 @@ func NewDefaultGatewayClassConditions() []Condition {
 	}
 }
 
+// NewGatewayClassConflict returns a Condition that indicates that the GatewayClass is not accepted
+// due to a conflict with another GatewayClass.
+func NewGatewayClassConflict() Condition {
+	return Condition{
+		Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(GatewayClassReasonGatewayClassConflict),
+		Message: GatewayClassMessageGatewayClassConflict,
+	}
+}
+
 // NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters.
 func NewGatewayClassInvalidParameters(msg string) Condition {
 	return Condition{