Implement NKG-specific validation for GatewayClass (#495)

Fixes #474

internal/state/change_processor_test.go

@@ -403,8 +403,8 @@ var _ = Describe("ChangeProcessor", func() {
 
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gc.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -512,8 +512,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gc.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -622,8 +622,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gc.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -731,8 +731,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gcUpdated.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -839,8 +839,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gcUpdated.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -940,8 +940,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gcUpdated.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-1"},
@@ -1061,8 +1061,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gcUpdated.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-2"},
@@ -1125,8 +1125,8 @@ var _ = Describe("ChangeProcessor", func() {
 					}
 					expectedStatuses := state.Statuses{
 						GatewayClassStatus: &state.GatewayClassStatus{
-							Valid:              true,
 							ObservedGeneration: gcUpdated.Generation,
+							Conditions:         conditions.NewDefaultGatewayClassConditions(),
 						},
 						GatewayStatus: &state.GatewayStatus{
 							NsName:             types.NamespacedName{Namespace: "test", Name: "gateway-2"},

internal/state/conditions/conditions.go

@@ -263,3 +263,25 @@ func NewRouteBackendRefUnsupportedValue(msg string) Condition {
 		Message: msg,
 	}
 }
+
+// NewGatewayClassInvalidParameters returns a Condition that indicates that the GatewayClass has invalid parameters.
+func NewGatewayClassInvalidParameters(msg string) Condition {
+	return Condition{
+		Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(v1beta1.GatewayClassReasonInvalidParameters),
+		Message: msg,
+	}
+}
+
+// NewDefaultGatewayClassConditions returns the default Conditions that must be present in the status of a GatewayClass.
+func NewDefaultGatewayClassConditions() []Condition {
+	return []Condition{
+		{
+			Type:    string(v1beta1.GatewayClassConditionStatusAccepted),
+			Status:  metav1.ConditionTrue,
+			Reason:  string(v1beta1.GatewayClassReasonAccepted),
+			Message: "GatewayClass is accepted",
+		},
+	}
+}

internal/state/dataplane/configuration_test.go

@@ -748,9 +748,8 @@ func TestBuildConfiguration(t *testing.T) {
 		{
 			graph: &graph.Graph{
 				GatewayClass: &graph.GatewayClass{
-					Source:   &v1beta1.GatewayClass{},
-					Valid:    false,
-					ErrorMsg: "error",
+					Source: &v1beta1.GatewayClass{},
+					Valid:  false,
 				},
 				Gateway: &graph.Gateway{
 					Source: &v1beta1.Gateway{},

internal/state/graph/gatewayclass.go

@@ -1,43 +1,54 @@
 package graph
 
 import (
-	"fmt"
-
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions"
 )
 
 // GatewayClass represents the GatewayClass resource.
 type GatewayClass struct {
 	// Source is the source resource.
 	Source *v1beta1.GatewayClass
-	// ErrorMsg explains the error when the resource is invalid.
-	ErrorMsg string
+	// Conditions include Conditions for the GatewayClass.
+	Conditions []conditions.Condition
 	// Valid shows whether the GatewayClass is valid.
 	Valid bool
 }
 
-func buildGatewayClass(gc *v1beta1.GatewayClass, controllerName string) *GatewayClass {
+func gatewayClassBelongsToController(gc *v1beta1.GatewayClass, controllerName string) bool {
+	// if GatewayClass doesn't exist, we assume it belongs to the controller
+	if gc == nil {
+		return true
+	}
+
+	return string(gc.Spec.ControllerName) == controllerName
+}
+
+func buildGatewayClass(gc *v1beta1.GatewayClass) *GatewayClass {
 	if gc == nil {
 		return nil
 	}
 
-	var errorMsg string
+	var conds []conditions.Condition
 
-	err := validateGatewayClass(gc, controllerName)
-	if err != nil {
-		errorMsg = err.Error()
+	valErr := validateGatewayClass(gc)
+	if valErr != nil {
+		conds = append(conds, conditions.NewGatewayClassInvalidParameters(valErr.Error()))
 	}
 
 	return &GatewayClass{
-		Source:   gc,
-		Valid:    err == nil,
-		ErrorMsg: errorMsg,
+		Source:     gc,
+		Valid:      valErr == nil,
+		Conditions: conds,
 	}
 }
 
-func validateGatewayClass(gc *v1beta1.GatewayClass, controllerName string) error {
-	if string(gc.Spec.ControllerName) != controllerName {
-		return fmt.Errorf("Spec.ControllerName must be %s got %s", controllerName, gc.Spec.ControllerName)
+func validateGatewayClass(gc *v1beta1.GatewayClass) error {
+	if gc.Spec.ParametersRef != nil {
+		path := field.NewPath("spec").Child("parametersRef")
+		return field.Forbidden(path, "parametersRef is not supported")
 	}
 
 	return nil

internal/state/graph/gatewayclass_test.go

@@ -3,76 +3,111 @@ package graph
 import (
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	. "github.com/onsi/gomega"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/conditions"
 )
 
 func TestBuildGatewayClass(t *testing.T) {
-	const controllerName = "my.controller"
+	validGC := &v1beta1.GatewayClass{}
 
-	validGC := &v1beta1.GatewayClass{
-		Spec: v1beta1.GatewayClassSpec{
-			ControllerName: "my.controller",
-		},
-	}
 	invalidGC := &v1beta1.GatewayClass{
 		Spec: v1beta1.GatewayClassSpec{
-			ControllerName: "wrong.controller",
+			ParametersRef: &v1beta1.ParametersReference{},
 		},
 	}
 
 	tests := []struct {
 		gc       *v1beta1.GatewayClass
 		expected *GatewayClass
-		msg      string
+		name     string
 	}{
-		{
-			gc:       nil,
-			expected: nil,
-			msg:      "no gatewayclass",
-		},
 		{
 			gc: validGC,
 			expected: &GatewayClass{
-				Source:   validGC,
-				Valid:    true,
-				ErrorMsg: "",
+				Source: validGC,
+				Valid:  true,
 			},
-			msg: "valid gatewayclass",
+			name: "valid gatewayclass",
+		},
+		{
+			gc:       nil,
+			expected: nil,
+			name:     "no gatewayclass",
 		},
 		{
 			gc: invalidGC,
 			expected: &GatewayClass{
-				Source:   invalidGC,
-				Valid:    false,
-				ErrorMsg: "Spec.ControllerName must be my.controller got wrong.controller",
+				Source: invalidGC,
+				Valid:  false,
+				Conditions: []conditions.Condition{
+					conditions.NewGatewayClassInvalidParameters("spec.parametersRef: Forbidden: parametersRef is not supported"),
+				},
 			},
-			msg: "invalid gatewayclass",
+			name: "invalid gatewayclass",
 		},
 	}
 
 	for _, test := range tests {
-		result := buildGatewayClass(test.gc, controllerName)
-		if diff := cmp.Diff(test.expected, result); diff != "" {
-			t.Errorf("buildGatewayClass() '%s' mismatch (-want +got):\n%s", test.msg, diff)
-		}
+		t.Run(test.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			result := buildGatewayClass(test.gc)
+			g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty())
+		})
 	}
 }
 
-func TestValidateGatewayClass(t *testing.T) {
-	gc := &v1beta1.GatewayClass{
-		Spec: v1beta1.GatewayClassSpec{
-			ControllerName: "test.controller",
+func TestGatewayClassBelongsToController(t *testing.T) {
+	const controllerName = "my.controller"
+
+	tests := []struct {
+		gc       *v1beta1.GatewayClass
+		name     string
+		expected bool
+	}{
+		{
+			gc: &v1beta1.GatewayClass{
+				Spec: v1beta1.GatewayClassSpec{
+					ControllerName: controllerName,
+				},
+			},
+			expected: true,
+			name:     "normal gatewayclass",
+		},
+		{
+			gc:       nil,
+			expected: true,
+			name:     "no gatewayclass",
+		},
+		{
+			gc: &v1beta1.GatewayClass{
+				Spec: v1beta1.GatewayClassSpec{
+					ControllerName: "some.controller",
+				},
+			},
+			expected: false,
+			name:     "wrong controller name",
+		},
+		{
+			gc: &v1beta1.GatewayClass{
+				Spec: v1beta1.GatewayClassSpec{
+					ControllerName: "",
+				},
+			},
+			expected: false,
+			name:     "empty controller name",
 		},
 	}
 
-	err := validateGatewayClass(gc, "test.controller")
-	if err != nil {
-		t.Errorf("validateGatewayClass() returned unexpected error %v", err)
-	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
 
-	err = validateGatewayClass(gc, "unmatched.controller")
-	if err == nil {
-		t.Errorf("validateGatewayClass() didn't return an error")
+			result := gatewayClassBelongsToController(test.gc, controllerName)
+			g.Expect(result).To(Equal(test.expected))
+		})
 	}
 }

internal/state/graph/graph.go

@@ -39,7 +39,11 @@ func BuildGraph(
 	secretMemoryMgr secrets.SecretDiskMemoryManager,
 	validators validation.Validators,
 ) *Graph {
-	gc := buildGatewayClass(store.GatewayClass, controllerName)
+	if !gatewayClassBelongsToController(store.GatewayClass, controllerName) {
+		return &Graph{}
+	}
+
+	gc := buildGatewayClass(store.GatewayClass)
 
 	processedGws := processGateways(store.Gateways, gcName)