diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index b2d9a7f4a3..4312187d44 100644 --- a/internal/state/change_processor_test.go +++ b/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"}, diff --git a/internal/state/conditions/conditions.go b/internal/state/conditions/conditions.go index 85a6e04bdc..e70ca2d250 100644 --- a/internal/state/conditions/conditions.go +++ b/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", + }, + } +} diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 56ce57b425..4c448bc252 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/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{}, diff --git a/internal/state/graph/gatewayclass.go b/internal/state/graph/gatewayclass.go index d5a7841f52..8413f3ae3f 100644 --- a/internal/state/graph/gatewayclass.go +++ b/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 diff --git a/internal/state/graph/gatewayclass_test.go b/internal/state/graph/gatewayclass_test.go index b60bf66401..0fa8dedee0 100644 --- a/internal/state/graph/gatewayclass_test.go +++ b/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)) + }) } } diff --git a/internal/state/graph/graph.go b/internal/state/graph/graph.go index c1ee3001ab..bed5138477 100644 --- a/internal/state/graph/graph.go +++ b/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) diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index 2041ebaaf3..67bd2883e0 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -156,24 +156,22 @@ func TestBuildGraph(t *testing.T) { svc := &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "foo"}} - store := ClusterStore{ - GatewayClass: &v1beta1.GatewayClass{ - Spec: v1beta1.GatewayClassSpec{ - ControllerName: controllerName, + createStoreWithGatewayClass := func(gc *v1beta1.GatewayClass) ClusterStore { + return ClusterStore{ + GatewayClass: gc, + Gateways: map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "gateway-1"}: gw1, + {Namespace: "test", Name: "gateway-2"}: gw2, }, - }, - Gateways: map[types.NamespacedName]*v1beta1.Gateway{ - {Namespace: "test", Name: "gateway-1"}: gw1, - {Namespace: "test", Name: "gateway-2"}: gw2, - }, - HTTPRoutes: map[types.NamespacedName]*v1beta1.HTTPRoute{ - {Namespace: "test", Name: "hr-1"}: hr1, - {Namespace: "test", Name: "hr-2"}: hr2, - {Namespace: "test", Name: "hr-3"}: hr3, - }, - Services: map[types.NamespacedName]*v1.Service{ - {Namespace: "test", Name: "foo"}: svc, - }, + HTTPRoutes: map[types.NamespacedName]*v1beta1.HTTPRoute{ + {Namespace: "test", Name: "hr-1"}: hr1, + {Namespace: "test", Name: "hr-2"}: hr2, + {Namespace: "test", Name: "hr-3"}: hr3, + }, + Services: map[types.NamespacedName]*v1.Service{ + {Namespace: "test", Name: "foo"}: svc, + }, + } } routeHR1 := &Route{ @@ -210,49 +208,89 @@ func TestBuildGraph(t *testing.T) { panic("unexpected secret request") }) - expected := &Graph{ - GatewayClass: &GatewayClass{ - Source: store.GatewayClass, - Valid: true, - }, - Gateway: &Gateway{ - Source: gw1, - Listeners: map[string]*Listener{ - "listener-80-1": { - Source: gw1.Spec.Listeners[0], - Valid: true, - Routes: map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr-1"}: routeHR1, - }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, - }, - "listener-443-1": { - Source: gw1.Spec.Listeners[1], - Valid: true, - Routes: map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr-3"}: routeHR3, + createExpectedGraphWithGatewayClass := func(gc *v1beta1.GatewayClass) *Graph { + return &Graph{ + GatewayClass: &GatewayClass{ + Source: gc, + Valid: true, + }, + Gateway: &Gateway{ + Source: gw1, + Listeners: map[string]*Listener{ + "listener-80-1": { + Source: gw1.Spec.Listeners[0], + Valid: true, + Routes: map[types.NamespacedName]*Route{ + {Namespace: "test", Name: "hr-1"}: routeHR1, + }, + AcceptedHostnames: map[string]struct{}{ + "foo.example.com": {}, + }, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, + "listener-443-1": { + Source: gw1.Spec.Listeners[1], + Valid: true, + Routes: map[types.NamespacedName]*Route{ + {Namespace: "test", Name: "hr-3"}: routeHR3, + }, + AcceptedHostnames: map[string]struct{}{ + "foo.example.com": {}, + }, + SecretPath: secretPath, }, - SecretPath: secretPath, }, }, + IgnoredGateways: map[types.NamespacedName]*v1beta1.Gateway{ + {Namespace: "test", Name: "gateway-2"}: gw2, + }, + Routes: map[types.NamespacedName]*Route{ + {Namespace: "test", Name: "hr-1"}: routeHR1, + {Namespace: "test", Name: "hr-3"}: routeHR3, + }, + } + } + + normalGC := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ControllerName: controllerName, }, - IgnoredGateways: map[types.NamespacedName]*v1beta1.Gateway{ - {Namespace: "test", Name: "gateway-2"}: gw2, + } + differentControllerGC := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ControllerName: "different-controller", }, - Routes: map[types.NamespacedName]*Route{ - {Namespace: "test", Name: "hr-1"}: routeHR1, - {Namespace: "test", Name: "hr-3"}: routeHR3, + } + + tests := []struct { + store ClusterStore + expected *Graph + name string + }{ + { + store: createStoreWithGatewayClass(normalGC), + expected: createExpectedGraphWithGatewayClass(normalGC), + name: "normal case", + }, + { + store: createStoreWithGatewayClass(differentControllerGC), + expected: &Graph{}, + name: "gatewayclass belongs to a different controller", }, } - g := NewGomegaWithT(t) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewGomegaWithT(t) + + result := BuildGraph( + test.store, + controllerName, + gcName, + secretMemoryMgr, + validation.Validators{HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}}, + ) - result := BuildGraph(store, controllerName, gcName, secretMemoryMgr, - validation.Validators{HTTPFieldsValidator: &validationfakes.FakeHTTPFieldsValidator{}}) - g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } } diff --git a/internal/state/statuses.go b/internal/state/statuses.go index 466b100077..7ce0f9ae81 100644 --- a/internal/state/statuses.go +++ b/internal/state/statuses.go @@ -65,14 +65,10 @@ type ParentStatus struct { Conditions []conditions.Condition } -// GatewayClassStatus holds status-related infortmation about the GatewayClass resource. +// GatewayClassStatus holds status-related information about the GatewayClass resource. type GatewayClassStatus struct { - // ErrorMsg describe the error when the resource is invalid. - ErrorMsg string - // ObservedGeneration is the generation of the resource that was processed. + Conditions []conditions.Condition ObservedGeneration int64 - // Valid shows if the resource is valid. - Valid bool } // buildStatuses builds statuses from a Graph. @@ -83,9 +79,17 @@ func buildStatuses(graph *graph.Graph) Statuses { } if graph.GatewayClass != nil { + defaultConds := conditions.NewDefaultGatewayClassConditions() + + conds := make([]conditions.Condition, 0, len(graph.GatewayClass.Conditions)+len(defaultConds)) + + // We add default conds first, so that any additional conditions will override them, which is + // ensured by DeduplicateConditions. + conds = append(conds, defaultConds...) + conds = append(conds, graph.GatewayClass.Conditions...) + statuses.GatewayClassStatus = &GatewayClassStatus{ - Valid: graph.GatewayClass.Valid, - ErrorMsg: graph.GatewayClass.ErrorMsg, + Conditions: conditions.DeduplicateConditions(conds), ObservedGeneration: graph.GatewayClass.Source.Generation, } } diff --git a/internal/state/statuses_test.go b/internal/state/statuses_test.go index 543e482b2a..644407dc24 100644 --- a/internal/state/statuses_test.go +++ b/internal/state/statuses_test.go @@ -116,8 +116,8 @@ func TestBuildStatuses(t *testing.T) { }, expected: Statuses{ GatewayClassStatus: &GatewayClassStatus{ - Valid: true, ObservedGeneration: 1, + Conditions: conditions.NewDefaultGatewayClassConditions(), }, GatewayStatus: &GatewayStatus{ NsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, @@ -210,8 +210,10 @@ func TestBuildStatuses(t *testing.T) { Source: &v1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{Generation: 1}, }, - Valid: false, - ErrorMsg: "error", + Valid: false, + Conditions: []conditions.Condition{ + conditions.NewGatewayClassInvalidParameters("error"), + }, }, Gateway: &graph.Gateway{ Source: gw, @@ -224,9 +226,10 @@ func TestBuildStatuses(t *testing.T) { }, expected: Statuses{ GatewayClassStatus: &GatewayClassStatus{ - Valid: false, - ErrorMsg: "error", ObservedGeneration: 1, + Conditions: []conditions.Condition{ + conditions.NewGatewayClassInvalidParameters("error"), + }, }, GatewayStatus: &GatewayStatus{ NsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, @@ -281,8 +284,8 @@ func TestBuildStatuses(t *testing.T) { }, expected: Statuses{ GatewayClassStatus: &GatewayClassStatus{ - Valid: true, ObservedGeneration: 1, + Conditions: conditions.NewDefaultGatewayClassConditions(), }, GatewayStatus: nil, IgnoredGatewayStatuses: map[types.NamespacedName]IgnoredGatewayStatus{}, diff --git a/internal/status/gatewayclass.go b/internal/status/gatewayclass.go index 1901198356..ce2f0a1e83 100644 --- a/internal/status/gatewayclass.go +++ b/internal/status/gatewayclass.go @@ -1,8 +1,6 @@ package status import ( - "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -11,29 +9,7 @@ import ( // prepareGatewayClassStatus prepares the status for the GatewayClass resource. func prepareGatewayClassStatus(status state.GatewayClassStatus, transitionTime metav1.Time) v1beta1.GatewayClassStatus { - var ( - condStatus metav1.ConditionStatus - msg string - ) - - if status.Valid { - condStatus = metav1.ConditionTrue - msg = "GatewayClass has been accepted" - } else { - condStatus = metav1.ConditionFalse - msg = fmt.Sprintf("GatewayClass has been rejected: %s", status.ErrorMsg) - } - - cond := metav1.Condition{ - Type: string(v1beta1.GatewayClassConditionStatusAccepted), - Status: condStatus, - ObservedGeneration: status.ObservedGeneration, - LastTransitionTime: transitionTime, - Reason: string(v1beta1.GatewayClassReasonAccepted), - Message: msg, - } - return v1beta1.GatewayClassStatus{ - Conditions: []metav1.Condition{cond}, + Conditions: convertConditions(status.Conditions, status.ObservedGeneration, transitionTime), } } diff --git a/internal/status/gatewayclass_test.go b/internal/status/gatewayclass_test.go index f57a99d33f..5987727e2a 100644 --- a/internal/status/gatewayclass_test.go +++ b/internal/status/gatewayclass_test.go @@ -4,66 +4,27 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" + "github.com/nginxinc/nginx-kubernetes-gateway/internal/helpers" "github.com/nginxinc/nginx-kubernetes-gateway/internal/state" ) func TestPrepareGatewayClassStatus(t *testing.T) { transitionTime := metav1.NewTime(time.Now()) - tests := []struct { - msg string - expected v1beta1.GatewayClassStatus - status state.GatewayClassStatus - }{ - { - status: state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: 1, - }, - expected: v1beta1.GatewayClassStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1beta1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionTrue, - ObservedGeneration: 1, - LastTransitionTime: transitionTime, - Reason: string(v1beta1.GatewayClassReasonAccepted), - Message: "GatewayClass has been accepted", - }, - }, - }, - msg: "valid GatewayClass", - }, - { - status: state.GatewayClassStatus{ - Valid: false, - ErrorMsg: "error", - ObservedGeneration: 2, - }, - expected: v1beta1.GatewayClassStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1beta1.GatewayClassConditionStatusAccepted), - Status: metav1.ConditionFalse, - ObservedGeneration: 2, - LastTransitionTime: transitionTime, - Reason: string(v1beta1.GatewayClassReasonAccepted), - Message: "GatewayClass has been rejected: error", - }, - }, - }, - msg: "invalid GatewayClass", - }, + status := state.GatewayClassStatus{ + ObservedGeneration: 1, + Conditions: CreateTestConditions(), } - - for _, test := range tests { - result := prepareGatewayClassStatus(test.status, transitionTime) - if diff := cmp.Diff(test.expected, result); diff != "" { - t.Errorf("prepareGatewayClassStatus() '%s' mismatch (-want +got):\n%s", test.msg, diff) - } + expected := v1beta1.GatewayClassStatus{ + Conditions: CreateExpectedAPIConditions(1, transitionTime), } + + g := NewGomegaWithT(t) + + result := prepareGatewayClassStatus(status, transitionTime) + g.Expect(helpers.Diff(expected, result)).To(BeEmpty()) } diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index 94ab50bc47..3a43f72c87 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -59,22 +59,21 @@ var _ = Describe("Updater", func() { }) Describe("Process status updates", Ordered, func() { + type generations struct { + gatewayClass int64 + gateways int64 + } + var ( gc *v1beta1.GatewayClass gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute - createStatuses = func(valid bool, generation int64) state.Statuses { - var gcErrorMsg string - if !valid { - gcErrorMsg = "error" - } - + createStatuses = func(gens generations) state.Statuses { return state.Statuses{ GatewayClassStatus: &state.GatewayClassStatus{ - Valid: valid, - ErrorMsg: gcErrorMsg, - ObservedGeneration: generation, + ObservedGeneration: gens.gatewayClass, + Conditions: status.CreateTestConditions(), }, GatewayStatus: &state.GatewayStatus{ NsName: types.NamespacedName{Namespace: "test", Name: "gateway"}, @@ -84,11 +83,11 @@ var _ = Describe("Updater", func() { Conditions: status.CreateTestConditions(), }, }, - ObservedGeneration: generation, + ObservedGeneration: gens.gateways, }, IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ {Namespace: "test", Name: "ignored-gateway"}: { - ObservedGeneration: generation, + ObservedGeneration: gens.gateways, }, }, HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ @@ -104,12 +103,7 @@ var _ = Describe("Updater", func() { } } - createExpectedGc = func( - status metav1.ConditionStatus, - generation int64, - reason string, - msg string, - ) *v1beta1.GatewayClass { + createExpectedGCWithGeneration = func(generation int64) *v1beta1.GatewayClass { return &v1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ Name: gcName, @@ -119,21 +113,12 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.networking.k8s.io/v1beta1", }, Status: v1beta1.GatewayClassStatus{ - Conditions: []metav1.Condition{ - { - Type: string(v1beta1.GatewayClassConditionStatusAccepted), - Status: status, - ObservedGeneration: generation, - LastTransitionTime: fakeClockTime, - Reason: reason, - Message: msg, - }, - }, + Conditions: status.CreateExpectedAPIConditions(generation, fakeClockTime), }, } } - createExpectedGw = func(generation int64) *v1beta1.Gateway { + createExpectedGwWithGeneration = func(generation int64) *v1beta1.Gateway { return &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -264,17 +249,15 @@ var _ = Describe("Updater", func() { }) It("should update statuses", func() { - updater.Update(context.Background(), createStatuses(true, 1)) + updater.Update(context.Background(), createStatuses(generations{ + gatewayClass: 1, + gateways: 1, + })) }) It("should have the updated status of GatewayClass in the API server", func() { latestGc := &v1beta1.GatewayClass{} - expectedGc := createExpectedGc( - metav1.ConditionTrue, - 1, - string(v1beta1.GatewayClassConditionStatusAccepted), - "GatewayClass has been accepted", - ) + expectedGc := createExpectedGCWithGeneration(1) err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) Expect(err).Should(Not(HaveOccurred())) @@ -286,7 +269,7 @@ var _ = Describe("Updater", func() { It("should have the updated status of Gateway in the API server", func() { latestGw := &v1beta1.Gateway{} - expectedGw := createExpectedGw(1) + expectedGw := createExpectedGwWithGeneration(1) err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "gateway"}, latestGw) Expect(err).Should(Not(HaveOccurred())) @@ -327,18 +310,16 @@ var _ = Describe("Updater", func() { It("should update statuses with canceled context - function normally returns", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() - updater.Update(ctx, createStatuses(false, 2)) + updater.Update(ctx, createStatuses(generations{ + gatewayClass: 2, + gateways: 2, + })) }) When("updating with canceled context", func() { It("should have the updated status of GatewayClass in the API server", func() { latestGc := &v1beta1.GatewayClass{} - expectedGc := createExpectedGc( - metav1.ConditionFalse, - 2, - string(v1beta1.GatewayClassConditionStatusAccepted), - "GatewayClass has been rejected: error", - ) + expectedGc := createExpectedGCWithGeneration(2) err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) Expect(err).Should(Not(HaveOccurred())) @@ -350,7 +331,7 @@ var _ = Describe("Updater", func() { It("should have the updated status of Gateway in the API server", func() { latestGw := &v1beta1.Gateway{} - expectedGw := createExpectedGw(2) + expectedGw := createExpectedGwWithGeneration(2) err := client.Get( context.Background(),