diff --git a/.golangci.yml b/.golangci.yml index bdf95b45b..656cb6818 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -35,7 +35,8 @@ linters-settings: govet: enable: - fieldalignment - + lll: + line-length: 120 linters: enable: - asciicheck @@ -49,6 +50,7 @@ linters: - gosimple - govet - ineffassign + - lll - makezero - misspell - nilerr diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index e9b53ae20..d4c666c07 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -12,7 +12,11 @@ import ( ) const ( - domain string = "k8s-gateway.nginx.org" + domain = "k8s-gateway.nginx.org" + gatewayClassNameUsage = `The name of the GatewayClass resource. ` + + `Every NGINX Gateway must have a unique corresponding GatewayClass resource.` + gatewayCtrlNameUsageFmt = `The name of the Gateway controller. ` + + `The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'` ) var ( @@ -25,14 +29,10 @@ var ( gatewayCtlrName = flag.String( "gateway-ctlr-name", "", - fmt.Sprintf("The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'", domain), + fmt.Sprintf(gatewayCtrlNameUsageFmt, domain), ) - gatewayClassName = flag.String( - "gatewayclass", - "", - "The name of the GatewayClass resource. Every NGINX Gateway must have a unique corresponding GatewayClass resource", - ) + gatewayClassName = flag.String("gatewayclass", "", gatewayClassNameUsage) ) func main() { diff --git a/cmd/gateway/setup.go b/cmd/gateway/setup.go index b49015122..d9059ad9d 100644 --- a/cmd/gateway/setup.go +++ b/cmd/gateway/setup.go @@ -15,8 +15,10 @@ import ( ) const ( - errTmpl = "failed validation - flag: '--%s' reason: '%s'\n" - controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` + errTmpl = "failed validation - flag: '--%s' reason: '%s'\n" + // nolint:lll + // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/547122f7f55ac0464685552898c560658fb40073/apis/v1alpha2/shared_types.go#L462 + controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll ) type ( @@ -57,7 +59,6 @@ func GatewayControllerParam(domain string) ValidatorContext { } func validateControllerName(name string) error { - // Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/547122f7f55ac0464685552898c560658fb40073/apis/v1alpha2/shared_types.go#L462 re := regexp.MustCompile(controllerNameRegex) if !re.MatchString(name) { return fmt.Errorf("invalid gateway controller name: %s; expected format is DOMAIN/PATH", name) diff --git a/internal/events/first_eventbatch_preparer.go b/internal/events/first_eventbatch_preparer.go index ad89e729b..bc1a95767 100644 --- a/internal/events/first_eventbatch_preparer.go +++ b/internal/events/first_eventbatch_preparer.go @@ -47,7 +47,11 @@ type FirstEventBatchPreparerImpl struct { // The object must specify its namespace (if any) and name. // For each list from objectLists, FirstEventBatchPreparerImpl will list the resources of the corresponding type from // the reader. -func NewFirstEventBatchPreparerImpl(reader Reader, objects []client.Object, objectLists []client.ObjectList) *FirstEventBatchPreparerImpl { +func NewFirstEventBatchPreparerImpl( + reader Reader, + objects []client.Object, + objectLists []client.ObjectList, +) *FirstEventBatchPreparerImpl { return &FirstEventBatchPreparerImpl{ reader: reader, objects: objects, diff --git a/internal/events/first_eventbatch_preparer_test.go b/internal/events/first_eventbatch_preparer_test.go index 2b81ece0b..02b5aa6d3 100644 --- a/internal/events/first_eventbatch_preparer_test.go +++ b/internal/events/first_eventbatch_preparer_test.go @@ -45,12 +45,14 @@ var _ = Describe("FirstEventBatchPreparer", func() { }) It("should prepare zero events when resources don't exist", func() { - fakeReader.GetCalls(func(ctx context.Context, name types.NamespacedName, object client.Object, opts ...client.GetOption) error { - Expect(name).Should(Equal(types.NamespacedName{Name: gcName})) - Expect(object).Should(BeAssignableToTypeOf(&v1beta1.GatewayClass{})) - - return apierrors.NewNotFound(schema.GroupResource{}, "test") - }) + fakeReader.GetCalls( + func(ctx context.Context, name types.NamespacedName, object client.Object, opts ...client.GetOption) error { + Expect(name).Should(Equal(types.NamespacedName{Name: gcName})) + Expect(object).Should(BeAssignableToTypeOf(&v1beta1.GatewayClass{})) + + return apierrors.NewNotFound(schema.GroupResource{}, "test") + }, + ) fakeReader.ListReturns(nil) batch, err := preparer.Prepare(context.Background()) @@ -62,13 +64,15 @@ var _ = Describe("FirstEventBatchPreparer", func() { It("should prepare one event for each resource type", func() { gatewayClass := v1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: gcName}} - fakeReader.GetCalls(func(ctx context.Context, name types.NamespacedName, object client.Object, opts ...client.GetOption) error { - Expect(name).Should(Equal(types.NamespacedName{Name: gcName})) - Expect(object).Should(BeAssignableToTypeOf(&v1beta1.GatewayClass{})) + fakeReader.GetCalls( + func(ctx context.Context, name types.NamespacedName, object client.Object, opts ...client.GetOption) error { + Expect(name).Should(Equal(types.NamespacedName{Name: gcName})) + Expect(object).Should(BeAssignableToTypeOf(&v1beta1.GatewayClass{})) - reflect.Indirect(reflect.ValueOf(object)).Set(reflect.Indirect(reflect.ValueOf(&gatewayClass))) - return nil - }) + reflect.Indirect(reflect.ValueOf(object)).Set(reflect.Indirect(reflect.ValueOf(&gatewayClass))) + return nil + }, + ) httpRoute := v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "test"}} @@ -101,13 +105,15 @@ var _ = Describe("FirstEventBatchPreparer", func() { Describe("EachListItem cases", func() { BeforeEach(func() { fakeReader.GetReturns(apierrors.NewNotFound(schema.GroupResource{}, "test")) - fakeReader.ListCalls(func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { - httpRoute := v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "test"}} - typedList := list.(*v1beta1.HTTPRouteList) - typedList.Items = append(typedList.Items, httpRoute) - - return nil - }) + fakeReader.ListCalls( + func(ctx context.Context, list client.ObjectList, option ...client.ListOption) error { + httpRoute := v1beta1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Name: "test"}} + typedList := list.(*v1beta1.HTTPRouteList) + typedList.Items = append(typedList.Items, httpRoute) + + return nil + }, + ) }) It("should return error if EachListItem passes a wrong object type", func() { diff --git a/internal/events/handler_test.go b/internal/events/handler_test.go index 7bb554176..f849982c6 100644 --- a/internal/events/handler_test.go +++ b/internal/events/handler_test.go @@ -88,7 +88,8 @@ var _ = Describe("EventHandler", func() { }) Describe("Process the Gateway API resources events", func() { - DescribeTable("A batch with one event", + DescribeTable( + "A batch with one event", func(e interface{}) { fakeConf := state.Configuration{} fakeStatuses := state.Statuses{} @@ -119,17 +120,59 @@ var _ = Describe("EventHandler", func() { // Check that a reconfig happened expectReconfig(fakeConf, fakeCfg, fakeStatuses) }, - Entry("HTTPRoute upsert", &events.UpsertEvent{Resource: &v1beta1.HTTPRoute{}}), - Entry("Gateway upsert", &events.UpsertEvent{Resource: &v1beta1.Gateway{}}), - Entry("GatewayClass upsert", &events.UpsertEvent{Resource: &v1beta1.GatewayClass{}}), - Entry("Service upsert", &events.UpsertEvent{Resource: &apiv1.Service{}}), - Entry("EndpointSlice upsert", &events.UpsertEvent{Resource: &discoveryV1.EndpointSlice{}}), - - Entry("HTTPRoute delete", &events.DeleteEvent{Type: &v1beta1.HTTPRoute{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "route"}}), - Entry("Gateway delete", &events.DeleteEvent{Type: &v1beta1.Gateway{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "gateway"}}), - Entry("GatewayClass delete", &events.DeleteEvent{Type: &v1beta1.GatewayClass{}, NamespacedName: types.NamespacedName{Name: "class"}}), - Entry("Service delete", &events.DeleteEvent{Type: &apiv1.Service{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "service"}}), - Entry("EndpointSlice deleted", &events.DeleteEvent{Type: &discoveryV1.EndpointSlice{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "endpointslice"}}), + Entry( + "HTTPRoute upsert", + &events.UpsertEvent{Resource: &v1beta1.HTTPRoute{}}, + ), + Entry( + "Gateway upsert", + &events.UpsertEvent{Resource: &v1beta1.Gateway{}}, + ), + Entry( + "GatewayClass upsert", + &events.UpsertEvent{Resource: &v1beta1.GatewayClass{}}, + ), + Entry( + "Service upsert", + &events.UpsertEvent{Resource: &apiv1.Service{}}, + ), + Entry( + "EndpointSlice upsert", + &events.UpsertEvent{Resource: &discoveryV1.EndpointSlice{}}, + ), + + Entry( + "HTTPRoute delete", + &events.DeleteEvent{ + Type: &v1beta1.HTTPRoute{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "route"}, + }, + ), + Entry( + "Gateway delete", + &events.DeleteEvent{ + Type: &v1beta1.Gateway{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "gateway"}, + }, + ), + Entry( + "GatewayClass delete", + &events.DeleteEvent{Type: &v1beta1.GatewayClass{}, NamespacedName: types.NamespacedName{Name: "class"}}, + ), + Entry( + "Service delete", + &events.DeleteEvent{ + Type: &apiv1.Service{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "service"}, + }, + ), + Entry( + "EndpointSlice deleted", + &events.DeleteEvent{ + Type: &discoveryV1.EndpointSlice{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "endpointslice"}, + }, + ), ) }) @@ -192,11 +235,20 @@ var _ = Describe("EventHandler", func() { &events.UpsertEvent{Resource: secret}, } deletes := []interface{}{ - &events.DeleteEvent{Type: &v1beta1.HTTPRoute{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "route"}}, - &events.DeleteEvent{Type: &v1beta1.Gateway{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "gateway"}}, + &events.DeleteEvent{ + Type: &v1beta1.HTTPRoute{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "route"}, + }, + &events.DeleteEvent{ + Type: &v1beta1.Gateway{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "gateway"}, + }, &events.DeleteEvent{Type: &v1beta1.GatewayClass{}, NamespacedName: types.NamespacedName{Name: "class"}}, &events.DeleteEvent{Type: &apiv1.Service{}, NamespacedName: svcNsName}, - &events.DeleteEvent{Type: &discoveryV1.EndpointSlice{}, NamespacedName: types.NamespacedName{Namespace: "test", Name: "endpointslice"}}, + &events.DeleteEvent{ + Type: &discoveryV1.EndpointSlice{}, + NamespacedName: types.NamespacedName{Namespace: "test", Name: "endpointslice"}, + }, &events.DeleteEvent{Type: &apiv1.Secret{}, NamespacedName: secretNsName}, } @@ -219,7 +271,8 @@ var _ = Describe("EventHandler", func() { // 5, not 6, because secret upsert events do not result into CaptureUpsertChange() call Expect(fakeProcessor.CaptureUpsertChangeCallCount()).Should(Equal(5)) for i := 0; i < 5; i++ { - Expect(fakeProcessor.CaptureUpsertChangeArgsForCall(i)).Should(Equal(upserts[i].(*events.UpsertEvent).Resource)) + Expect(fakeProcessor.CaptureUpsertChangeArgsForCall(i)). + Should(Equal(upserts[i].(*events.UpsertEvent).Resource)) } // 5, not 6, because secret delete events do not result into CaptureDeleteChange() call diff --git a/internal/helpers/helpers.go b/internal/helpers/helpers.go index 04ac7a751..48c70017d 100644 --- a/internal/helpers/helpers.go +++ b/internal/helpers/helpers.go @@ -1,3 +1,4 @@ +// Package helpers contains helper functions for unit tests. package helpers import ( @@ -17,42 +18,42 @@ func Diff(x, y interface{}) string { return r } -// GetStringPointer takes a string and returns a pointer to it. Useful in unit tests when initializing structs. +// GetStringPointer takes a string and returns a pointer to it. func GetStringPointer(s string) *string { return &s } -// GetIntPointer takes an int and returns a pointer to it. Useful in unit tests when initializing structs. +// GetIntPointer takes an int and returns a pointer to it. func GetIntPointer(i int) *int { return &i } -// GetInt32Pointer takes an int32 and returns a pointer to it. Useful in unit tests when initializing structs. +// GetInt32Pointer takes an int32 and returns a pointer to it. func GetInt32Pointer(i int32) *int32 { return &i } -// GetHTTPMethodPointer takes an HTTPMethod and returns a pointer to it. Useful in unit tests when initializing structs. +// GetHTTPMethodPointer takes an HTTPMethod and returns a pointer to it. func GetHTTPMethodPointer(m v1beta1.HTTPMethod) *v1beta1.HTTPMethod { return &m } -// GetHeaderMatchTypePointer takes an HeaderMatchType and returns a pointer to it. Useful in unit tests when initializing structs. +// GetHeaderMatchTypePointer takes an HeaderMatchType and returns a pointer to it. func GetHeaderMatchTypePointer(t v1beta1.HeaderMatchType) *v1beta1.HeaderMatchType { return &t } -// GetQueryParamMatchTypePointer takes an QueryParamMatchType and returns a pointer to it. Useful in unit tests when initializing structs. +// GetQueryParamMatchTypePointer takes an QueryParamMatchType and returns a pointer to it. func GetQueryParamMatchTypePointer(t v1beta1.QueryParamMatchType) *v1beta1.QueryParamMatchType { return &t } -// GetTLSModePointer takes a TLSModeType and returns a pointer to it. Useful in unit tests when initializing structs. +// GetTLSModePointer takes a TLSModeType and returns a pointer to it. func GetTLSModePointer(t v1beta1.TLSModeType) *v1beta1.TLSModeType { return &t } -// GetBoolPointer takes a bool and returns a pointer to it. Useful in unit tests when initializing structs. +// GetBoolPointer takes a bool and returns a pointer to it. func GetBoolPointer(b bool) *bool { return &b } diff --git a/internal/manager/controllers_test.go b/internal/manager/controllers_test.go index 4c21e9884..c4282d3ab 100644 --- a/internal/manager/controllers_test.go +++ b/internal/manager/controllers_test.go @@ -86,17 +86,38 @@ func TestRegisterController(t *testing.T) { for _, test := range tests { newReconciler := func(c reconciler.Config) *reconciler.Implementation { if c.Getter != test.fakes.mgr.GetClient() { - t.Errorf("regiterController() created a reconciler config with Getter %p but expected %p for case of %q", c.Getter, test.fakes.mgr.GetClient(), test.msg) + t.Errorf( + "regiterController() created a reconciler config with Getter %p but expected %p for case of %q", + c.Getter, + test.fakes.mgr.GetClient(), + test.msg, + ) } if c.ObjectType != objectType { - t.Errorf("registerController() created a reconciler config with ObjectType %T but expected %T for case of %q", c.ObjectType, objectType, test.msg) + t.Errorf( + "registerController() created a reconciler config with ObjectType %T but expected %T for case of %q", + c.ObjectType, + objectType, + test.msg, + ) } if c.EventCh != eventCh { - t.Errorf("registerController() created a reconciler config with EventCh %v but expected %v for case of %q", c.EventCh, eventCh, test.msg) + t.Errorf( + "registerController() created a reconciler config with EventCh %v but expected %v for case of %q", + c.EventCh, + eventCh, + test.msg, + ) } // comparing functions is not allowed in Go, so we're comparing the pointers + // nolint: lll if reflect.ValueOf(c.NamespacedNameFilter).Pointer() != reflect.ValueOf(namespacedNameFilter).Pointer() { - t.Errorf("registerController() created a reconciler config with NamespacedNameFilter %p but expected %p for case of %q", c.NamespacedNameFilter, namespacedNameFilter, test.msg) + t.Errorf( + "registerController() created a reconciler config with NamespacedNameFilter %p but expected %p for case of %q", + c.NamespacedNameFilter, + namespacedNameFilter, + test.msg, + ) } return reconciler.NewImplementation(c) @@ -114,32 +135,60 @@ func TestRegisterController(t *testing.T) { ) if !errors.Is(err, test.expectedErr) { - t.Errorf("registerController() returned %q but expected %q for case of %q", err, test.expectedErr, test.msg) + t.Errorf( + "registerController() returned %q but expected %q for case of %q", + err, + test.expectedErr, + test.msg, + ) } indexCallCount := test.fakes.indexer.IndexFieldCallCount() if indexCallCount != 1 { - t.Errorf("registerController() called indexer.IndexField() %d times but expected 1 for case of %q", indexCallCount, test.msg) + t.Errorf( + "registerController() called indexer.IndexField() %d times but expected 1 for case of %q", + indexCallCount, + test.msg, + ) } else { _, objType, field, indexFunc := test.fakes.indexer.IndexFieldArgsForCall(0) if objType != objectType { - t.Errorf("registerController() called indexer.IndexField() with object type %T but expected %T for case of %q", objType, objectType, test.msg) + t.Errorf( + "registerController() called indexer.IndexField() with object type %T but expected %T for case %q", + objType, + objectType, + test.msg, + ) } if field != index.KubernetesServiceNameIndexField { - t.Errorf("registerController() called indexer.IndexField() with field %q but expected %q for case of %q", field, index.KubernetesServiceNameIndexField, test.msg) + t.Errorf("registerController() called indexer.IndexField() with field %q but expected %q for case %q", + field, + index.KubernetesServiceNameIndexField, + test.msg, + ) } expectedIndexFunc := fieldIndexes[index.KubernetesServiceNameIndexField] // comparing functions is not allowed in Go, so we're comparing the pointers + // nolint:lll if reflect.ValueOf(indexFunc).Pointer() != reflect.ValueOf(expectedIndexFunc).Pointer() { - t.Errorf("registerController() called indexer.IndexField() with indexFunc %p but expected %p for case of %q", indexFunc, expectedIndexFunc, test.msg) + t.Errorf("registerController() called indexer.IndexField() with indexFunc %p but expected %p for case %q", + indexFunc, + expectedIndexFunc, + test.msg, + ) } } addCallCount := test.fakes.mgr.AddCallCount() if addCallCount != test.expectedMgrAddCallCount { - t.Errorf("registerController() called mgr.Add() %d times but expected %d times for case of %q", addCallCount, test.expectedMgrAddCallCount, test.msg) + t.Errorf( + "registerController() called mgr.Add() %d times but expected %d times for case %q", + addCallCount, + test.expectedMgrAddCallCount, + test.msg, + ) } } } diff --git a/internal/manager/filter/gatewayclass.go b/internal/manager/filter/gatewayclass.go index 4a85a5924..306877b4a 100644 --- a/internal/manager/filter/gatewayclass.go +++ b/internal/manager/filter/gatewayclass.go @@ -13,7 +13,10 @@ import ( func CreateFilterForGatewayClass(gcName string) reconciler.NamespacedNameFilterFunc { return func(nsname types.NamespacedName) (bool, string) { if nsname.Name != gcName { - return false, fmt.Sprintf("GatewayClass is ignored because this controller only supports the GatewayClass %s", gcName) + return false, fmt.Sprintf( + "GatewayClass is ignored because this controller only supports the GatewayClass %s", + gcName, + ) } return true, "" } diff --git a/internal/manager/predicate/service_test.go b/internal/manager/predicate/service_test.go index 4a2493ebc..b7b80dd65 100644 --- a/internal/manager/predicate/service_test.go +++ b/internal/manager/predicate/service_test.go @@ -230,7 +230,12 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) { }) if update != tc.expUpdate { - t.Errorf("ServicePortsChangedPredicate.Update() mismatch for %q; got %t, expected %t", tc.msg, update, tc.expUpdate) + t.Errorf( + "ServicePortsChangedPredicate.Update() mismatch for %q; got %t, expected %t", + tc.msg, + update, + tc.expUpdate, + ) } } } diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 87f38d936..07cc6fffa 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -178,8 +178,9 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, // httpMatch is an internal representation of an HTTPRouteMatch. // This struct is marshaled into a string and stored as a variable in the nginx location block for the route's path. -// The NJS httpmatches module will lookup this variable on the request object and compare the request against the Method, Headers, and QueryParams contained in httpMatch. -// If the request satisfies the httpMatch, the request will be internally redirected to the location RedirectPath by NGINX. +// The NJS httpmatches module will look up this variable on the request object and compare the request against the +// Method, Headers, and QueryParams contained in httpMatch. +// If the request satisfies the httpMatch, NGINX will redirect the request to the location RedirectPath. type httpMatch struct { // Method is the HTTPMethod of the HTTPRouteMatch. Method v1beta1.HTTPMethod `json:"method,omitempty"` @@ -248,8 +249,10 @@ func createQueryParamKeyValString(p v1beta1.HTTPQueryParamMatch) string { } // The name and values are delimited by ":". A name and value can always be recovered using strings.Split(arg, ":"). -// Header names are case-insensitive while header values are case-sensitive (e.g. foo:bar == FOO:bar, but foo:bar != foo:BAR). -// We preserve the case of the name here because NGINX allows us to lookup the header names in a case-insensitive manner. +// Header names are case-insensitive and header values are case-sensitive. +// Ex. foo:bar == FOO:bar, but foo:bar != foo:BAR, +// We preserve the case of the name here because NGINX allows us to look up the header names in a case-insensitive +// manner. func createHeaderKeyValString(h v1beta1.HTTPHeaderMatch) string { return string(h.Name) + ":" + h.Value } diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 490472e69..42e3b3d5f 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -184,7 +184,9 @@ func TestCreateServers(t *testing.T) { }, { Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer("/"), // should generate an "any" httpmatch since other matches exists for / + Value: helpers.GetStringPointer( + "/", // should generate an "any" httpmatch since other matches exists for / + ), }, }, }, diff --git a/internal/nginx/config/upstreams_template.go b/internal/nginx/config/upstreams_template.go index 4365b51b9..c5d74bb17 100644 --- a/internal/nginx/config/upstreams_template.go +++ b/internal/nginx/config/upstreams_template.go @@ -1,6 +1,7 @@ package config -// FIXME(kate-osborn): Add upstream zone size for each upstream. This should be dynamically calculated based on the number of upstreams. +// FIXME(kate-osborn): Add upstream zone size for each upstream. +// This should be dynamically calculated based on the number of upstreams. var upstreamsTemplateText = ` {{ range $u := . }} upstream {{ $u.Name }} { diff --git a/internal/nginx/runtime/manager.go b/internal/nginx/runtime/manager.go index 8608bbad4..ba745aead 100644 --- a/internal/nginx/runtime/manager.go +++ b/internal/nginx/runtime/manager.go @@ -32,8 +32,9 @@ func NewManagerImpl() *ManagerImpl { func (m *ManagerImpl) Reload(ctx context.Context) error { // FIXME(pleshakov): Before reload attempt, make sure NGINX is running. - // If the gateway container starts before NGINX container (which is possible), then it is possible that a reload can be attempted - // when NGINX is not running yet. Make sure to prevent this case, so we don't get an error. + // If the gateway container starts before NGINX container (which is possible), + // then it is possible that a reload can be attempted when NGINX is not running yet. + // Make sure to prevent this case, so we don't get an error. // We find the main NGINX PID on every reload because it will change if the NGINX container is restarted. pid, err := findMainProcess(os.ReadFile) diff --git a/internal/state/backend_refs.go b/internal/state/backend_refs.go index abbb80d65..2c2df5d95 100644 --- a/internal/state/backend_refs.go +++ b/internal/state/backend_refs.go @@ -97,7 +97,11 @@ func validateBackendRef(ref v1beta1.BackendRef, routeNs string) error { } if ref.Namespace != nil && string(*ref.Namespace) != routeNs { - return fmt.Errorf("cross-namespace routing is not permitted; namespace %s does not match the HTTPRoute namespace %s", *ref.Namespace, routeNs) + return fmt.Errorf( + "cross-namespace routing is not permitted; namespace %s does not match the HTTPRoute namespace %s", + *ref.Namespace, + routeNs, + ) } if ref.Port == nil { diff --git a/internal/state/change_processor.go b/internal/state/change_processor.go index c026c1275..fdb4d3ec0 100644 --- a/internal/state/change_processor.go +++ b/internal/state/change_processor.go @@ -18,8 +18,8 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ChangeProcessor -// ChangeProcessor processes the changes to resources producing the internal representation of the Gateway configuration. -// ChangeProcessor only supports one GatewayClass resource. +// ChangeProcessor processes the changes to resources producing the internal representation +// of the Gateway configuration. It only supports one GatewayClass resource. type ChangeProcessor interface { // CaptureUpsertChange captures an upsert change to a resource. // It panics if the resource is of unsupported type or if the passed Gateway is different from the one this diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 3cdb4649e..5aaffbbb5 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -28,7 +28,12 @@ const ( certificatePath = "path/to/cert" ) -func createRoute(name string, gateway string, hostname string, backendRefs ...v1beta1.HTTPBackendRef) *v1beta1.HTTPRoute { +func createRoute( + name string, + gateway string, + hostname string, + backendRefs ...v1beta1.HTTPBackendRef, +) *v1beta1.HTTPRoute { return &v1beta1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -39,14 +44,18 @@ func createRoute(name string, gateway string, hostname string, backendRefs ...v1 CommonRouteSpec: v1beta1.CommonRouteSpec{ ParentRefs: []v1beta1.ParentReference{ { - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: v1beta1.ObjectName(gateway), - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-80-1")), + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Name: v1beta1.ObjectName(gateway), + SectionName: (*v1beta1.SectionName)( + helpers.GetStringPointer("listener-80-1"), + ), }, { - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Name: v1beta1.ObjectName(gateway), - SectionName: (*v1beta1.SectionName)(helpers.GetStringPointer("listener-443-1")), + Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), + Name: v1beta1.ObjectName(gateway), + SectionName: (*v1beta1.SectionName)( + helpers.GetStringPointer("listener-443-1"), + ), }, }, }, @@ -114,7 +123,10 @@ func createGatewayWithTLSListener(name string) *v1beta1.Gateway { return gw } -func createRouteWithMultipleRules(name, gateway, hostname string, rules []v1beta1.HTTPRouteRule) *v1beta1.HTTPRoute { +func createRouteWithMultipleRules( + name, gateway, hostname string, + rules []v1beta1.HTTPRouteRule, +) *v1beta1.HTTPRoute { hr := createRoute(name, gateway, hostname) hr.Spec.Rules = rules @@ -134,7 +146,11 @@ func createHTTPRule(path string, backendRefs ...v1beta1.HTTPBackendRef) v1beta1. } } -func createBackendRef(kind *v1beta1.Kind, name v1beta1.ObjectName, namespace *v1beta1.Namespace) v1beta1.HTTPBackendRef { +func createBackendRef( + kind *v1beta1.Kind, + name v1beta1.ObjectName, + namespace *v1beta1.Namespace, +) v1beta1.HTTPBackendRef { return v1beta1.HTTPBackendRef{ BackendRef: v1beta1.BackendRef{ BackendObjectReference: v1beta1.BackendObjectReference{ @@ -213,7 +229,7 @@ var _ = Describe("ChangeProcessor", func() { gw2 = createGatewayWithTLSListener("gateway-2") }) When("no upsert has occurred", func() { - It("should return empty configuration and statuses", func() { + It("returns empty configuration and statuses", func() { changed, conf, statuses := processor.Process(context.TODO()) Expect(changed).To(BeFalse()) Expect(conf).To(BeZero()) @@ -222,13 +238,52 @@ var _ = Describe("ChangeProcessor", func() { }) When("GatewayClass doesn't exist", func() { When("Gateways don't exist", func() { - It("should return empty configuration and updated statuses after upserting the first HTTPRoute", func() { - processor.CaptureUpsertChange(hr1) + When("the first HTTPRoute is upserted", func() { + It("returns empty configuration and statuses", func() { + processor.CaptureUpsertChange(hr1) + + expectedConf := state.Configuration{} + expectedStatuses := state.Statuses{ + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + } + + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) + }) + }) + When("the first Gateway is upserted", func() { + It("returns empty configuration and updated statuses", func() { + processor.CaptureUpsertChange(gw1) expectedConf := state.Configuration{} expectedStatuses := state.Statuses{ + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: false, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: false, + AttachedRoutes: 1, + }, + }, + }, IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: false}, + "listener-443-1": {Attached: false}, + }, + }, + }, } changed, conf, statuses := processor.Process(context.TODO()) @@ -237,21 +292,72 @@ var _ = Describe("ChangeProcessor", func() { Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) }) }) + }) + When("the GatewayClass is upserted", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureUpsertChange(gc) - It("should return empty configuration and updated statuses after upserting the first Gateway", func() { - processor.CaptureUpsertChange(gw1) + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1, + }, + }, + }, + }, + }, + }, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1, + }, + }, + }, + }, + }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, + }, + }, + BackendGroups: []state.BackendGroup{ + hr1Group, + }, + } - expectedConf := state.Configuration{} expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gc.Generation, + }, GatewayStatus: &state.GatewayStatus{ NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, ListenerStatuses: map[string]state.ListenerStatus{ "listener-80-1": { - Valid: false, + Valid: true, AttachedRoutes: 1, }, "listener-443-1": { - Valid: false, + Valid: true, AttachedRoutes: 1, }, }, @@ -261,8 +367,8 @@ var _ = Describe("ChangeProcessor", func() { {Namespace: "test", Name: "hr-1"}: { ObservedGeneration: hr1.Generation, ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: false}, - "listener-443-1": {Attached: false}, + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, }, }, }, @@ -274,757 +380,701 @@ var _ = Describe("ChangeProcessor", func() { Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) }) }) + When("the first HTTPRoute without a generation changed is processed", func() { + It("returns empty configuration and statuses", func() { + hr1UpdatedSameGen := hr1.DeepCopy() + // hr1UpdatedSameGen.Generation has not been changed + processor.CaptureUpsertChange(hr1UpdatedSameGen) - It("should return updated configuration and statuses after the GatewayClass is upserted", func() { - processor.CaptureUpsertChange(gc) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeFalse()) + Expect(conf).To(BeZero()) + Expect(statuses).To(BeZero()) + }) + }) + When("the first HTTPRoute update with a generation changed is processed", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureUpsertChange(hr1Updated) - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1, + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, + }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, + BackendGroups: []state.BackendGroup{ + hr1Group, }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gc.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, - }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gc.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, }, - }, - } + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }, + ) }) + When("the first Gateway update without generation changed is processed", func() { + It("returns empty configuration and statuses", func() { + gwUpdatedSameGen := gw1.DeepCopy() + // gwUpdatedSameGen.Generation has not been changed + processor.CaptureUpsertChange(gwUpdatedSameGen) - It("should return empty configuration and statuses after processing upserting the first HTTPRoute without generation change", func() { - hr1UpdatedSameGen := hr1.DeepCopy() - // hr1UpdatedSameGen.Generation has not been changed - processor.CaptureUpsertChange(hr1UpdatedSameGen) - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeFalse()) + Expect(conf).To(BeZero()) + Expect(statuses).To(BeZero()) + }) }) + When("the first Gateway update with a generation changed is processed", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureUpsertChange(gw1Updated) - It("should return updated configuration and statuses after upserting the first HTTPRoute with generation change", func() { - processor.CaptureUpsertChange(hr1Updated) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, + }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, + BackendGroups: []state.BackendGroup{ + hr1Group, }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gc.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, - }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gc.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, }, - }, - } + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) + When("the GatewayClass update without generation change is processed", func() { + It("returns empty configuration and statuses", func() { + gcUpdatedSameGen := gc.DeepCopy() + // gcUpdatedSameGen.Generation has not been changed + processor.CaptureUpsertChange(gcUpdatedSameGen) - It("should return empty configuration and statuses after processing upserting the first Gateway without generation change", func() { - gwUpdatedSameGen := gw1.DeepCopy() - // gwUpdatedSameGen.Generation has not been changed - processor.CaptureUpsertChange(gwUpdatedSameGen) - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeFalse()) + Expect(conf).To(BeZero()) + Expect(statuses).To(BeZero()) + }) }) + When("the GatewayClass update with generation change is processed", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureUpsertChange(gcUpdated) - It("should return updated configuration and statuses after upserting the first Gateway with generation change", func() { - processor.CaptureUpsertChange(gw1Updated) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, + }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, + BackendGroups: []state.BackendGroup{ + hr1Group, }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gc.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, - }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gcUpdated.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, }, - }, - } + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) + When("no changes are captured", func() { + It("returns empty configuration and statuses", func() { + changed, conf, statuses := processor.Process(context.TODO()) - It("should return empty configuration and statuses after processing upserting the GatewayClass without generation change", func() { - gcUpdatedSameGen := gc.DeepCopy() - // gcUpdatedSameGen.Generation has not been changed - processor.CaptureUpsertChange(gcUpdatedSameGen) - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + Expect(changed).To(BeFalse()) + Expect(conf).To(BeZero()) + Expect(statuses).To(BeZero()) + }) }) - - It("should return updated configuration and statuses after upserting the GatewayClass with generation change", func() { - processor.CaptureUpsertChange(gcUpdated) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + When("the second Gateway is upserted", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureUpsertChange(gw2) + + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, + SSL: &state.SSL{ + CertificatePath: certificatePath, + }, + }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, + BackendGroups: []state.BackendGroup{ + hr1Group, }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gcUpdated.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gcUpdated.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, + }, }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + }, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ + {Namespace: "test", Name: "gateway-2"}: { + ObservedGeneration: gw2.Generation, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) - }) - - It("should return empty configuration and statuses after processing without capturing any changes", func() { - changed, conf, statuses := processor.Process(context.TODO()) + } - Expect(changed).To(BeFalse()) - Expect(conf).To(BeZero()) - Expect(statuses).To(BeZero()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) - - It("should return updated configuration and statuses after the second Gateway is upserted", func() { - processor.CaptureUpsertChange(gw2) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + When("the second HTTPRoute is upserted", func() { + It("returns same configuration and updated statuses", func() { + processor.CaptureUpsertChange(hr2) + + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, + SSLServers: []state.VirtualServer{ + { + Hostname: "foo.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr1Group, + Source: hr1Updated, + }, }, }, }, }, - SSL: &state.SSL{ - CertificatePath: certificatePath, - }, - }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gcUpdated.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, - }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ - {Namespace: "test", Name: "gateway-2"}: { - ObservedGeneration: gw2.Generation, + BackendGroups: []state.BackendGroup{ + hr1Group, }, - }, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, - }, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gcUpdated.Generation, }, - }, - } - - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) - }) - - It("should return same configuration and updated statuses after the second HTTPRoute is upserted", func() { - processor.CaptureUpsertChange(hr2) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, }, - }, - }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "foo.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr1Group, - Source: hr1Updated, - }, - }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, }, }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, - }, - }, - BackendGroups: []state.BackendGroup{ - hr1Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gcUpdated.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-1"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ + {Namespace: "test", Name: "gateway-2"}: { + ObservedGeneration: gw2.Generation, }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, - }, - }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{ - {Namespace: "test", Name: "gateway-2"}: { - ObservedGeneration: gw2.Generation, }, - }, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-1"}: { - ObservedGeneration: hr1Updated.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-1"}: { + ObservedGeneration: hr1Updated.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, - }, - {Namespace: "test", Name: "hr-2"}: { - ObservedGeneration: hr2.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: false}, - "listener-443-1": {Attached: false}, + {Namespace: "test", Name: "hr-2"}: { + ObservedGeneration: hr2.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: false}, + "listener-443-1": {Attached: false}, + }, }, }, - }, - } + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) - - It("should return updated configuration and statuses after deleting the first Gateway", func() { - processor.CaptureDeleteChange(&v1beta1.Gateway{}, types.NamespacedName{Namespace: "test", Name: "gateway-1"}) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{ - { - Hostname: "bar.example.com", - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr2Group, - Source: hr2, + When("the first Gateway is deleted", func() { + It("returns updated configuration and statuses", func() { + processor.CaptureDeleteChange( + &v1beta1.Gateway{}, + types.NamespacedName{Namespace: "test", Name: "gateway-1"}, + ) + + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{ + { + Hostname: "bar.example.com", + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr2Group, + Source: hr2, + }, }, }, }, }, }, - }, - SSLServers: []state.VirtualServer{ - { - Hostname: "bar.example.com", - SSL: &state.SSL{CertificatePath: certificatePath}, - PathRules: []state.PathRule{ - { - Path: "/", - MatchRules: []state.MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: hr2Group, - Source: hr2, + SSLServers: []state.VirtualServer{ + { + Hostname: "bar.example.com", + SSL: &state.SSL{CertificatePath: certificatePath}, + PathRules: []state.PathRule{ + { + Path: "/", + MatchRules: []state.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr2Group, + Source: hr2, + }, }, }, }, }, + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, + }, }, - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, + BackendGroups: []state.BackendGroup{ + hr2Group, }, - }, - BackendGroups: []state.BackendGroup{ - hr2Group, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gcUpdated.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 1, - }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 1, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gcUpdated.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 1, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 1, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ - {Namespace: "test", Name: "hr-2"}: { - ObservedGeneration: hr2.Generation, - ParentStatuses: map[string]state.ParentStatus{ - "listener-80-1": {Attached: true}, - "listener-443-1": {Attached: true}, + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{ + {Namespace: "test", Name: "hr-2"}: { + ObservedGeneration: hr2.Generation, + ParentStatuses: map[string]state.ParentStatus{ + "listener-80-1": {Attached: true}, + "listener-443-1": {Attached: true}, + }, }, }, - }, - } + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) - - It("should return configuration with default ssl server and updated statuses after deleting the second HTTPRoute", func() { - processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, types.NamespacedName{Namespace: "test", Name: "hr-2"}) - - expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{}, - SSLServers: []state.VirtualServer{ - { - Hostname: "~^", - SSL: &state.SSL{CertificatePath: certificatePath}, - }, - }, - } - expectedStatuses := state.Statuses{ - GatewayClassStatus: &state.GatewayClassStatus{ - Valid: true, - ObservedGeneration: gcUpdated.Generation, - }, - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: true, - AttachedRoutes: 0, + When("the second HTTPRoute is deleted", func() { + It("returns configuration with default ssl server and updated statuses", func() { + processor.CaptureDeleteChange( + &v1beta1.HTTPRoute{}, + types.NamespacedName{Namespace: "test", Name: "hr-2"}, + ) + + expectedConf := state.Configuration{ + HTTPServers: []state.VirtualServer{}, + SSLServers: []state.VirtualServer{ + { + Hostname: "~^", + SSL: &state.SSL{CertificatePath: certificatePath}, }, - "listener-443-1": { - Valid: true, - AttachedRoutes: 0, + }, + } + expectedStatuses := state.Statuses{ + GatewayClassStatus: &state.GatewayClassStatus{ + Valid: true, + ObservedGeneration: gcUpdated.Generation, + }, + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: true, + AttachedRoutes: 0, + }, + "listener-443-1": { + Valid: true, + AttachedRoutes: 0, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, - } + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) + When("the GatewayClass is deleted", func() { + It("returns empty configuration and updated statuses", func() { + processor.CaptureDeleteChange( + &v1beta1.GatewayClass{}, + types.NamespacedName{Name: gcName}, + ) - It("should return empty configuration and updated statuses after deleting the GatewayClass", func() { - processor.CaptureDeleteChange(&v1beta1.GatewayClass{}, types.NamespacedName{Name: gcName}) - - expectedConf := state.Configuration{} - expectedStatuses := state.Statuses{ - GatewayStatus: &state.GatewayStatus{ - NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, - ListenerStatuses: map[string]state.ListenerStatus{ - "listener-80-1": { - Valid: false, - AttachedRoutes: 0, - }, - "listener-443-1": { - Valid: false, - AttachedRoutes: 0, + expectedConf := state.Configuration{} + expectedStatuses := state.Statuses{ + GatewayStatus: &state.GatewayStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + ListenerStatuses: map[string]state.ListenerStatus{ + "listener-80-1": { + Valid: false, + AttachedRoutes: 0, + }, + "listener-443-1": { + Valid: false, + AttachedRoutes: 0, + }, }, }, - }, - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, - } + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) + When("the second Gateway is deleted", func() { + It("returns empty configuration and empty statuses", func() { + processor.CaptureDeleteChange( + &v1beta1.Gateway{}, + types.NamespacedName{Namespace: "test", Name: "gateway-2"}, + ) - It("should return empty configuration and empty statuses after deleting the second Gateway", func() { - processor.CaptureDeleteChange(&v1beta1.Gateway{}, types.NamespacedName{Namespace: "test", Name: "gateway-2"}) - - expectedConf := state.Configuration{} - expectedStatuses := state.Statuses{ - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, - } + expectedConf := state.Configuration{} + expectedStatuses := state.Statuses{ + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) + When("the first HTTPRoute is deleted", func() { + It("returns empty configuration and empty statuses", func() { + processor.CaptureDeleteChange( + &v1beta1.HTTPRoute{}, + types.NamespacedName{Namespace: "test", Name: "hr-1"}, + ) - It("should return empty configuration and statuses after deleting the first HTTPRoute", func() { - processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, types.NamespacedName{Namespace: "test", Name: "hr-1"}) - - expectedConf := state.Configuration{} - expectedStatuses := state.Statuses{ - IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, - HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, - } + expectedConf := state.Configuration{} + expectedStatuses := state.Statuses{ + IgnoredGatewayStatuses: map[types.NamespacedName]state.IgnoredGatewayStatus{}, + HTTPRouteStatuses: map[types.NamespacedName]state.HTTPRouteStatus{}, + } - changed, conf, statuses := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) - Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) - Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + changed, conf, statuses := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty()) + Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty()) + }) }) }) Describe("Process services and endpoints", Ordered, func() { @@ -1060,23 +1110,24 @@ var _ = Describe("ChangeProcessor", func() { // backend Refs fooRef := createBackendRef(&kindService, "foo-svc", &testNamespace) - barRef := createBackendRef(&kindService, "bar-svc", nil) // nil namespace should inherit namespace from route - baz1Ref := createBackendRef(&kindService, "baz-svc-v1", &testNamespace) + baz1NilNamespace := createBackendRef(&kindService, "baz-svc-v1", &testNamespace) + barRef := createBackendRef(&kindService, "bar-svc", nil) baz2Ref := createBackendRef(&kindService, "baz-svc-v2", &testNamespace) baz3Ref := createBackendRef(&kindService, "baz-svc-v3", &testNamespace) - invalidRef := createBackendRef(&kindInvalid, "bar-svc", &testNamespace) // invalid kind + invalidKindRef := createBackendRef(&kindInvalid, "bar-svc", &testNamespace) // httproutes hr1 = createRoute("hr1", "gw", "foo.example.com", fooRef) hr2 = createRoute("hr2", "gw", "bar.example.com", barRef) - hr3 = createRoute("hr3", "gw", "bar.2.example.com", barRef) // shares backend ref with hr2 - hrInvalidBackendRef = createRoute("hr-invalid", "gw", "invalid.example.com", invalidRef) + // hr3 shares the same backendRef as hr2 + hr3 = createRoute("hr3", "gw", "bar.2.example.com", barRef) + hrInvalidBackendRef = createRoute("hr-invalid", "gw", "invalid.com", invalidKindRef) hrMultipleRules = createRouteWithMultipleRules( "hr-multiple-rules", "gw", "mutli.example.com", []v1beta1.HTTPRouteRule{ - createHTTPRule("/baz-v1", baz1Ref), + createHTTPRule("/baz-v1", baz1NilNamespace), createHTTPRule("/baz-v2", baz2Ref), createHTTPRule("/baz-v3", baz3Ref), }, @@ -1112,115 +1163,249 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureDeleteChange(obj, nsname) testProcessChangedVal(expChanged) } - - It("add hr1", func() { - testUpsertTriggersChange(hr1, true) + When("hr1 is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1, true) + }) }) - It("add service for hr1", func() { - testUpsertTriggersChange(hr1svc, true) + When("a hr1 service is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1svc, true) + }) }) - It("add endpoint slice for hr1", func() { - testUpsertTriggersChange(hr1slice1, true) + When("an hr1 endpoint slice is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1slice1, true) + }) }) - It("update for service for hr1", func() { - testUpsertTriggersChange(hr1svc, true) + When("an hr1 service is updated", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1svc, true) + }) }) - It("add second endpoint slice for hr1", func() { - testUpsertTriggersChange(hr1slice2, true) + When("another hr1 endpoint slice is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1slice2, true) + }) }) - It("add endpoint slice with missing svc name label", func() { - testUpsertTriggersChange(missingSvcNameSlice, false) + When("an endpoint slice with a missing svc name label is added", func() { + It("should not trigger a change", func() { + testUpsertTriggersChange(missingSvcNameSlice, false) + }) }) - It("delete one endpoint slice for hr1", func() { - testDeleteTriggersChange(hr1slice1, types.NamespacedName{Namespace: hr1slice1.Namespace, Name: hr1slice1.Name}, true) + When("an hr1 endpoint slice is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hr1slice1, + types.NamespacedName{Namespace: hr1slice1.Namespace, Name: hr1slice1.Name}, + true, + ) + }) }) - It("delete second endpoint slice for hr1", func() { - testDeleteTriggersChange(hr1slice2, types.NamespacedName{Namespace: hr1slice2.Namespace, Name: hr1slice2.Name}, true) + When("the second hr1 endpoint slice is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hr1slice2, + types.NamespacedName{Namespace: hr1slice2.Namespace, Name: hr1slice2.Name}, + true, + ) + }) }) - It("recreate second endpoint slice for hr1", func() { - testUpsertTriggersChange(hr1slice2, true) + When("the second hr1 endpoint slice is recreated", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr1slice2, true) + }) }) - It("delete hr1", func() { - testDeleteTriggersChange(hr1, types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, true) + When("hr1 is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hr1, + types.NamespacedName{Namespace: hr1.Namespace, Name: hr1.Name}, + true, + ) + }) }) - It("delete service for hr1", func() { - testDeleteTriggersChange(hr1svc, types.NamespacedName{Namespace: hr1svc.Namespace, Name: hr1svc.Name}, false) + When("hr1 service is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + hr1svc, + types.NamespacedName{Namespace: hr1svc.Namespace, Name: hr1svc.Name}, + false, + ) + }) }) - It("delete second endpoint slice for hr1", func() { - testDeleteTriggersChange(hr1slice2, types.NamespacedName{Namespace: hr1slice2.Namespace, Name: hr1slice2.Name}, false) + When("the second hr1 endpoint slice is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + hr1slice2, + types.NamespacedName{Namespace: hr1slice2.Namespace, Name: hr1slice2.Name}, + false, + ) + }) }) - It("add hr2", func() { - testUpsertTriggersChange(hr2, true) + When("hr2 is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr2, true) + }) }) - It("add hr3 -- a route that shares a backend service with hr2", func() { - testUpsertTriggersChange(hr3, true) + When("a hr3, that shares a backend service with hr2, is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hr3, true) + }) }) - It("add service referenced by both hr2 and hr3", func() { - testUpsertTriggersChange(sharedSvc, true) + When("sharedSvc, a service referenced by both hr2 and hr3, is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(sharedSvc, true) + }) }) - It("delete hr2", func() { - testDeleteTriggersChange(hr2, types.NamespacedName{Namespace: hr2.Namespace, Name: hr2.Name}, true) + When("hr2 is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hr2, + types.NamespacedName{Namespace: hr2.Namespace, Name: hr2.Name}, + true, + ) + }) }) - It("delete shared service", func() { - testDeleteTriggersChange(sharedSvc, types.NamespacedName{Namespace: sharedSvc.Namespace, Name: sharedSvc.Name}, true) + When("sharedSvc is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + sharedSvc, + types.NamespacedName{Namespace: sharedSvc.Namespace, Name: sharedSvc.Name}, + true, + ) + }) }) - It("recreate shared service", func() { - testUpsertTriggersChange(sharedSvc, true) + When("sharedSvc is recreated", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(sharedSvc, true) + }) }) - It("delete hr3", func() { - testDeleteTriggersChange(hr3, types.NamespacedName{Namespace: hr3.Namespace, Name: hr3.Name}, true) + When("hr3 is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hr3, + types.NamespacedName{Namespace: hr3.Namespace, Name: hr3.Name}, + true, + ) + }) }) - It("delete shared service", func() { - testDeleteTriggersChange(sharedSvc, types.NamespacedName{Namespace: sharedSvc.Namespace, Name: sharedSvc.Name}, false) + When("sharedSvc is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + sharedSvc, + types.NamespacedName{Namespace: sharedSvc.Namespace, Name: sharedSvc.Name}, + false, + ) + }) }) - It("add service that is not referenced by any route", func() { - testUpsertTriggersChange(notRefSvc, false) + When("a service that is not referenced by any route is added", func() { + It("should not trigger a change", func() { + testUpsertTriggersChange(notRefSvc, false) + }) }) - It("add route with an invalid backend ref type", func() { - testUpsertTriggersChange(hrInvalidBackendRef, true) + When("a route with an invalid backend ref type is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hrInvalidBackendRef, true) + }) }) - It("add service with a namespace name that matches invalid backend ref (should be ignored)", func() { - testUpsertTriggersChange(invalidSvc, false) + When("a service with a namespace name that matches invalid backend ref is added", func() { + It("should not trigger a change", func() { + testUpsertTriggersChange(invalidSvc, false) + }) }) - It("add endpoint slice that is not owned by a referenced service", func() { - testUpsertTriggersChange(noRefSlice, false) + When("an endpoint slice that is not owned by a referenced service is added", func() { + It("should not trigger a change", func() { + testUpsertTriggersChange(noRefSlice, false) + }) }) - It("delete endpoint slice that is not owned by a referenced service", func() { - testDeleteTriggersChange(noRefSlice, types.NamespacedName{Namespace: noRefSlice.Namespace, Name: noRefSlice.Name}, false) + When("an endpoint slice that is not owned by a referenced service is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + noRefSlice, + types.NamespacedName{Namespace: noRefSlice.Namespace, Name: noRefSlice.Name}, + false, + ) + }) }) - When("processing a route with multiple rules and three unique backend services", func() { - It("adds route", func() { - testUpsertTriggersChange(hrMultipleRules, true) + Context("processing a route with multiple rules and three unique backend services", func() { + When("route is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(hrMultipleRules, true) + }) }) - It("adds one referenced service", func() { - testUpsertTriggersChange(bazSvc1, true) + When("first referenced service is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(bazSvc1, true) + }) }) - It("adds second referenced service", func() { - testUpsertTriggersChange(bazSvc2, true) + When("second referenced service is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(bazSvc2, true) + }) }) - It("deletes first referenced service", func() { - testDeleteTriggersChange(bazSvc1, types.NamespacedName{Namespace: bazSvc1.Namespace, Name: bazSvc1.Name}, true) + When("first referenced service is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + bazSvc1, + types.NamespacedName{Namespace: bazSvc1.Namespace, Name: bazSvc1.Name}, + true, + ) + }) }) - It("recreates first referenced service", func() { - testUpsertTriggersChange(bazSvc1, true) + When("first referenced service is recreated", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(bazSvc1, true) + }) }) - It("adds third referenced service", func() { - testUpsertTriggersChange(bazSvc3, true) + When("third referenced service is added", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(bazSvc3, true) + }) }) - It("updates third referenced service", func() { - testUpsertTriggersChange(bazSvc3, true) + When("third referenced service is updated", func() { + It("should trigger a change", func() { + testUpsertTriggersChange(bazSvc3, true) + }) }) - It("deletes route with multiple services", func() { - testDeleteTriggersChange(hrMultipleRules, types.NamespacedName{Namespace: hrMultipleRules.Namespace, Name: hrMultipleRules.Name}, true) + When("route is deleted", func() { + It("should trigger a change", func() { + testDeleteTriggersChange( + hrMultipleRules, + types.NamespacedName{ + Namespace: hrMultipleRules.Namespace, + Name: hrMultipleRules.Name, + }, + true, + ) + }) }) - It("deletes first service", func() { - testDeleteTriggersChange(bazSvc1, types.NamespacedName{Namespace: bazSvc1.Namespace, Name: bazSvc1.Name}, false) + When("first referenced service is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + bazSvc1, + types.NamespacedName{Namespace: bazSvc1.Namespace, Name: bazSvc1.Name}, + false, + ) + }) }) - It("deletes second service", func() { - testDeleteTriggersChange(bazSvc2, types.NamespacedName{Namespace: bazSvc2.Namespace, Name: bazSvc2.Name}, false) + When("second referenced service is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + bazSvc2, + types.NamespacedName{Namespace: bazSvc2.Namespace, Name: bazSvc2.Name}, + false, + ) + }) }) - It("deletes final service", func() { - testDeleteTriggersChange(bazSvc3, types.NamespacedName{Namespace: bazSvc3.Namespace, Name: bazSvc3.Name}, false) + When("final referenced service is deleted", func() { + It("should not trigger a change", func() { + testDeleteTriggersChange( + bazSvc3, + types.NamespacedName{Namespace: bazSvc3.Namespace, Name: bazSvc3.Name}, + false, + ) + }) }) }) }) @@ -1333,7 +1518,6 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) }) - It("should report not changed after multiple Upserts of the resource with same generation", func() { processor.CaptureUpsertChange(gc) processor.CaptureUpsertChange(gw1) @@ -1342,22 +1526,22 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeFalse()) }) + When("a upsert of updated resources is followed by an upsert of the same generation", func() { + It("should report changed", func() { + // these are changing changes + processor.CaptureUpsertChange(gcUpdated) + processor.CaptureUpsertChange(gw1Updated) + processor.CaptureUpsertChange(hr1Updated) - It("should report changed after upserting updated resources followed by upserting same generations", func() { - // these are changing changes - processor.CaptureUpsertChange(gcUpdated) - processor.CaptureUpsertChange(gw1Updated) - processor.CaptureUpsertChange(hr1Updated) - - // there are non-changing changes - processor.CaptureUpsertChange(gcUpdated) - processor.CaptureUpsertChange(gw1Updated) - processor.CaptureUpsertChange(hr1Updated) + // there are non-changing changes + processor.CaptureUpsertChange(gcUpdated) + processor.CaptureUpsertChange(gw1Updated) + processor.CaptureUpsertChange(hr1Updated) - changed, _, _ := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) + changed, _, _ := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + }) }) - It("should report changed after upserting new resources", func() { // we can't have a second GatewayClass, so we don't add it processor.CaptureUpsertChange(gw2) @@ -1366,19 +1550,20 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) }) + When("resources are deleted followed by upserts with the same generations", func() { + It("should report changed", func() { + // these are changing changes + processor.CaptureDeleteChange(&v1beta1.GatewayClass{}, gcNsName) + processor.CaptureDeleteChange(&v1beta1.Gateway{}, gwNsName) + processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hrNsName) - It("should report changed after deleting resources followed by upserting same generations of new resources", func() { - // these are changing changes - processor.CaptureDeleteChange(&v1beta1.GatewayClass{}, gcNsName) - processor.CaptureDeleteChange(&v1beta1.Gateway{}, gwNsName) - processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hrNsName) - - // these are non-changing changes - processor.CaptureUpsertChange(gw2) - processor.CaptureUpsertChange(hr2) + // these are non-changing changes + processor.CaptureUpsertChange(gw2) + processor.CaptureUpsertChange(hr2) - changed, _, _ := processor.Process(context.TODO()) - Expect(changed).To(BeTrue()) + changed, _, _ := processor.Process(context.TODO()) + Expect(changed).To(BeTrue()) + }) }) It("should report changed after deleting resources", func() { processor.CaptureDeleteChange(&v1beta1.HTTPRoute{}, hr2NsName) @@ -1405,9 +1590,8 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeFalse()) }) - - It("should report changed after upserting related resources followed by upserting unrelated resources", - func() { + When("upserts of related resources are followed by upserts of unrelated resources", func() { + It("should report changed", func() { // these are changing changes fakeRelationshipCapturer.ExistsReturns(true) processor.CaptureUpsertChange(svc) @@ -1421,9 +1605,9 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) }) - - It("should report changed after deleting related resources followed by upserting unrelated resources", - func() { + }) + When("deletes of related resources are followed by upserts of unrelated resources", func() { + It("should report changed", func() { // these are changing changes fakeRelationshipCapturer.ExistsReturns(true) processor.CaptureDeleteChange(&apiv1.Service{}, svcNsName) @@ -1437,6 +1621,7 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) }) + }) }) Describe("Multiple Kubernetes API and Gateway API resource changes", Ordered, func() { It("should report changed after multiple Upserts of new and related resources", func() { @@ -1486,7 +1671,8 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) - }) + }, + ) It("should report changed after upserting changed resources followed by upserting unrelated resources", func() { @@ -1502,8 +1688,10 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) - }) - It("should report changed after upserting related resources followed by upserting unchanged resources", + }, + ) + It( + "should report changed after upserting related resources followed by upserting unchanged resources", func() { // these are changing changes fakeRelationshipCapturer.ExistsReturns(true) @@ -1518,7 +1706,8 @@ var _ = Describe("ChangeProcessor", func() { changed, _, _ := processor.Process(context.TODO()) Expect(changed).To(BeTrue()) - }) + }, + ) }) }) @@ -1548,17 +1737,34 @@ var _ = Describe("ChangeProcessor", func() { } Expect(process).Should(Panic()) }, - Entry("an unsupported resource", &v1alpha2.TCPRoute{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "tcp"}}), - Entry("a wrong gatewayclass", &v1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "wrong-class"}})) + Entry( + "an unsupported resource", + &v1alpha2.TCPRoute{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "tcp"}}, + ), + Entry( + "a wrong gatewayclass", + &v1beta1.GatewayClass{ObjectMeta: metav1.ObjectMeta{Name: "wrong-class"}}, + ), + ) - DescribeTable("CaptureDeleteChange must panic", + DescribeTable( + "CaptureDeleteChange must panic", func(resourceType client.Object, nsname types.NamespacedName) { process := func() { processor.CaptureDeleteChange(resourceType, nsname) } Expect(process).Should(Panic()) }, - Entry("an unsupported resource", &v1alpha2.TCPRoute{}, types.NamespacedName{Namespace: "test", Name: "tcp"}), - Entry("a wrong gatewayclass", &v1beta1.GatewayClass{}, types.NamespacedName{Name: "wrong-class"})) + Entry( + "an unsupported resource", + &v1alpha2.TCPRoute{}, + types.NamespacedName{Namespace: "test", Name: "tcp"}, + ), + Entry( + "a wrong gatewayclass", + &v1beta1.GatewayClass{}, + types.NamespacedName{Name: "wrong-class"}, + ), + ) }) }) diff --git a/internal/state/configuration.go b/internal/state/configuration.go index 0edb5bd04..9061a0968 100644 --- a/internal/state/configuration.go +++ b/internal/state/configuration.go @@ -67,7 +67,8 @@ type Filters struct { // MatchRule represents a routing rule. It corresponds directly to a Match in the HTTPRoute resource. // An HTTPRoute is guaranteed to have at least one rule with one match. -// If no rule or match is specified by the user, the default rule {{path:{ type: "PathPrefix", value: "/"}}} is set by the schema. +// If no rule or match is specified by the user, the default rule {{path:{ type: "PathPrefix", value: "/"}}} +// is set by the schema. type MatchRule struct { // Filters holds the filters for the MatchRule. Filters Filters @@ -337,7 +338,8 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { for _, l := range hpr.listeners { hostname := getListenerHostname(l.Source.Hostname) // generate a 404 ssl server block for listeners with no routes or listeners with wildcard (match-all) routes - // FIXME(kate-osborn): when we support regex hostnames (e.g. *.example.com) we will have to modify this check to catch regex hostnames. + // FIXME(kate-osborn): when we support regex hostnames (e.g. *.example.com) + // we will have to modify this check to catch regex hostnames. if len(l.Routes) == 0 || hostname == wildcardHostname { s := VirtualServer{ Hostname: hostname, diff --git a/internal/state/configuration_test.go b/internal/state/configuration_test.go index 9c5bf37f2..b2577fc2a 100644 --- a/internal/state/configuration_test.go +++ b/internal/state/configuration_test.go @@ -688,8 +688,17 @@ func TestBuildConfiguration(t *testing.T) { SSL: &SSL{CertificatePath: secretPath}, }, }, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{hr3Groups[0], hr3Groups[1], hr4Groups[0], hr4Groups[1], httpsHR3Groups[0], httpsHR3Groups[1], httpsHR4Groups[0], httpsHR4Groups[1]}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{ + hr3Groups[0], + hr3Groups[1], + hr4Groups[0], + hr4Groups[1], + httpsHR3Groups[0], + httpsHR3Groups[1], + httpsHR4Groups[0], + httpsHR4Groups[1], + }, }, msg: "one http and one https listener with two routes with the same hostname with and without collisions", }, @@ -984,7 +993,12 @@ func TestMatchRuleGetMatch(t *testing.T) { for _, tc := range tests { actual := tc.rule.GetMatch() if *actual.Path.Value != tc.expPath { - t.Errorf("MatchRule.GetMatch() returned incorrect match with path: %s, expected path: %s for test case: %q", *actual.Path.Value, tc.expPath, tc.name) + t.Errorf( + "MatchRule.GetMatch() returned incorrect match with path: %s, expected path: %s for test case: %q", + *actual.Path.Value, + tc.expPath, + tc.name, + ) } } } @@ -1018,7 +1032,12 @@ func TestGetListenerHostname(t *testing.T) { for _, test := range tests { result := getListenerHostname(test.hostname) if result != test.expected { - t.Errorf("getListenerHostname() returned %q but expected %q for the case of %q", result, test.expected, test.msg) + t.Errorf( + "getListenerHostname() returned %q but expected %q for the case of %q", + result, + test.expected, + test.msg, + ) } } } diff --git a/internal/state/graph.go b/internal/state/graph.go index 5ffdfdb9f..a507208c9 100644 --- a/internal/state/graph.go +++ b/internal/state/graph.go @@ -103,7 +103,10 @@ func buildGraph( // processGateways determines which Gateway resource the NGINX Gateway will use (the winner) and which Gateway(s) will // be ignored. Note that the function will not take into the account any unrelated Gateway resources - the ones with the // different GatewayClassName field. -func processGateways(gws map[types.NamespacedName]*v1beta1.Gateway, gcName string) (winner *v1beta1.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway) { +func processGateways( + gws map[types.NamespacedName]*v1beta1.Gateway, + gcName string, +) (winner *v1beta1.Gateway, ignoredGateways map[types.NamespacedName]*v1beta1.Gateway) { referencedGws := make([]*v1beta1.Gateway, 0, len(gws)) for _, gw := range gws { diff --git a/internal/state/listener.go b/internal/state/listener.go index 988895f17..ca2c90693 100644 --- a/internal/state/listener.go +++ b/internal/state/listener.go @@ -41,7 +41,10 @@ func (f *listenerConfiguratorFactory) getConfiguratorForListener(l v1beta1.Liste } } -func newListenerConfiguratorFactory(gw *v1beta1.Gateway, secretMemoryMgr SecretDiskMemoryManager) *listenerConfiguratorFactory { +func newListenerConfiguratorFactory( + gw *v1beta1.Gateway, + secretMemoryMgr SecretDiskMemoryManager, +) *listenerConfiguratorFactory { return &listenerConfiguratorFactory{ https: newHTTPSListenerConfigurator(gw, secretMemoryMgr), http: newHTTPListenerConfigurator(), @@ -54,7 +57,10 @@ type httpsListenerConfigurator struct { usedHostnames map[string]*listener } -func newHTTPSListenerConfigurator(gateway *v1beta1.Gateway, secretMemoryMgr SecretDiskMemoryManager) *httpsListenerConfigurator { +func newHTTPSListenerConfigurator( + gateway *v1beta1.Gateway, + secretMemoryMgr SecretDiskMemoryManager, +) *httpsListenerConfigurator { return &httpsListenerConfigurator{ gateway: gateway, secretMemoryMgr: secretMemoryMgr, @@ -156,7 +162,8 @@ func validateHTTPSListener(listener v1beta1.Listener, gwNsname string) bool { // FIXME(kate-osborn): // 1. For now we require that all HTTPS listeners bind to port 443 // 2. Only TLSModeTerminate is supported. - if listener.Port != 443 || listener.TLS == nil || *listener.TLS.Mode != v1beta1.TLSModeTerminate || len(listener.TLS.CertificateRefs) == 0 { + if listener.Port != 443 || listener.TLS == nil || *listener.TLS.Mode != v1beta1.TLSModeTerminate || + len(listener.TLS.CertificateRefs) == 0 { return false } diff --git a/internal/state/listener_test.go b/internal/state/listener_test.go index 2aa0d9eec..9f06fe6c6 100644 --- a/internal/state/listener_test.go +++ b/internal/state/listener_test.go @@ -35,7 +35,12 @@ func TestValidateHTTPListener(t *testing.T) { for _, test := range tests { result := validateHTTPListener(test.l) if result != test.expected { - t.Errorf("validateListener() returned %v but expected %v for the case of %q", result, test.expected, test.msg) + t.Errorf( + "validateListener() returned %v but expected %v for the case of %q", + result, + test.expected, + test.msg, + ) } } } @@ -150,7 +155,12 @@ func TestValidateHTTPSListener(t *testing.T) { for _, test := range tests { result := validateHTTPSListener(test.l, gwNs) if result != test.expected { - t.Errorf("validateHTTPSListener() returned %v but expected %v for the case of %q", result, test.expected, test.msg) + t.Errorf( + "validateHTTPSListener() returned %v but expected %v for the case of %q", + result, + test.expected, + test.msg, + ) } } } diff --git a/internal/state/relationship/relationships_test.go b/internal/state/relationship/relationships_test.go index e378bb2ee..843ea9a44 100644 --- a/internal/state/relationship/relationships_test.go +++ b/internal/state/relationship/relationships_test.go @@ -27,7 +27,10 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { } } - getModifiedRefs := func(svcName v1beta1.ObjectName, mod func([]v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { + getModifiedRefs := func( + svcName v1beta1.ObjectName, + mod func([]v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef, + ) []v1beta1.HTTPBackendRef { return mod(getNormalRefs(svcName)) } @@ -60,7 +63,9 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { { BackendRefs: getModifiedRefs("diff-namespace", func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { - refs[0].Namespace = (*v1beta1.Namespace)(helpers.GetStringPointer("not-test")) + refs[0].Namespace = (*v1beta1.Namespace)( + helpers.GetStringPointer("not-test"), + ) return refs }, ), @@ -78,10 +83,16 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { return append(refs, v1beta1.HTTPBackendRef{ BackendRef: v1beta1.BackendRef{ BackendObjectReference: v1beta1.BackendObjectReference{ - Kind: (*v1beta1.Kind)(helpers.GetStringPointer("Service")), - Name: "multiple-refs2", - Namespace: (*v1beta1.Namespace)(helpers.GetStringPointer("test")), - Port: (*v1beta1.PortNumber)(helpers.GetInt32Pointer(80)), + Kind: (*v1beta1.Kind)( + helpers.GetStringPointer("Service"), + ), + Name: "multiple-refs2", + Namespace: (*v1beta1.Namespace)( + helpers.GetStringPointer("test"), + ), + Port: (*v1beta1.PortNumber)( + helpers.GetInt32Pointer(80), + ), }, }, }) diff --git a/internal/state/secrets.go b/internal/state/secrets.go index e1f8009a2..b5d2e12a8 100644 --- a/internal/state/secrets.go +++ b/internal/state/secrets.go @@ -69,7 +69,8 @@ func (s SecretStoreImpl) Get(nsname types.NamespacedName) *Secret { // SecretDiskMemoryManager manages secrets that are requested by Gateway resources. type SecretDiskMemoryManager interface { // Request marks the secret as requested so that it can be written to disk before reloading NGINX. - // Returns the path to the secret and an error if the secret does not exist in the secret store or the secret is invalid. + // Returns the path to the secret if it exists. + // Returns an error if the secret does not exist in the secret store or the secret is invalid. Request(nsname types.NamespacedName) (string, error) // WriteAllRequestedSecrets writes all requested secrets to disk. WriteAllRequestedSecrets() error @@ -114,7 +115,11 @@ func WithSecretFileManager(fileManager FileManager) SecretDiskMemoryManagerOptio } } -func NewSecretDiskMemoryManager(secretDirectory string, secretStore SecretStore, options ...SecretDiskMemoryManagerOption) *SecretDiskMemoryManagerImpl { +func NewSecretDiskMemoryManager( + secretDirectory string, + secretStore SecretStore, + options ...SecretDiskMemoryManagerOption, +) *SecretDiskMemoryManagerImpl { sm := &SecretDiskMemoryManagerImpl{ requestedSecrets: make(map[types.NamespacedName]requestedSecret), secretStore: secretStore, @@ -136,7 +141,11 @@ func (s *SecretDiskMemoryManagerImpl) Request(nsname types.NamespacedName) (stri } if !secret.Valid { - return "", fmt.Errorf("secret %s is not valid; must be of type %s and contain a valid X509 key pair", nsname, apiv1.SecretTypeTLS) + return "", fmt.Errorf( + "secret %s is not valid; must be of type %s and contain a valid X509 key pair", + nsname, + apiv1.SecretTypeTLS, + ) } ss := requestedSecret{ @@ -172,7 +181,12 @@ func (s *SecretDiskMemoryManagerImpl) WriteAllRequestedSecrets() error { } if err = s.fileManager.Chmod(file, tlsSecretFileMode); err != nil { - return fmt.Errorf("failed to change mode of file %s for secret %s: %w", ss.path, nsname, err) + return fmt.Errorf( + "failed to change mode of file %s for secret %s: %w", + ss.path, + nsname, + err, + ) } contents := generateCertAndKeyFileContent(ss.secret) diff --git a/internal/state/sort.go b/internal/state/sort.go index efc671172..e2ad4ba5b 100644 --- a/internal/state/sort.go +++ b/internal/state/sort.go @@ -27,13 +27,16 @@ Precedence must be given to the Rule with the largest number of (Continuing on t - Header matches. - Query param matches. -If ties still exist across multiple Routes, matching precedence MUST be determined in order of the following criteria, continuing on ties: +If ties still exist across multiple Routes, matching precedence MUST be determined in order of the following criteria, +continuing on ties: - The oldest Route based on creation timestamp. - The Route appearing first in alphabetical order by “{namespace}/{name}”. -If ties still exist within the Route that has been given precedence, matching precedence MUST be granted to the first matching rule meeting the above criteria. +If ties still exist within the Route that has been given precedence, +matching precedence MUST be granted to the first matching rule meeting the above criteria. -higherPriority will determine precedence by comparing len(headers), len(query parameters), creation timestamp, and namespace name. The other criteria are handled by NGINX. +higherPriority will determine precedence by comparing len(headers), len(query parameters), creation timestamp, +and namespace name. The other criteria are handled by NGINX. */ func higherPriority(rule1, rule2 MatchRule) bool { // Get the matches from the rules diff --git a/internal/status/httproute.go b/internal/status/httproute.go index 94591a10d..367f83f37 100644 --- a/internal/status/httproute.go +++ b/internal/status/httproute.go @@ -34,15 +34,15 @@ func prepareHTTPRouteStatus( var ( conditionStatus metav1.ConditionStatus - reason string // FIXME(pleshakov) use RouteConditionReason once we upgrade to v1beta1 + reason string // FIXME(pleshakov) use RouteConditionReason ) if ps.Attached { conditionStatus = metav1.ConditionTrue - reason = "Accepted" // FIXME(pleshakov): use RouteReasonAccepted once we upgrade to v1beta1 + reason = "Accepted" // FIXME(pleshakov): use RouteReasonAccepted } else { conditionStatus = metav1.ConditionFalse - reason = "NotAttached" // FIXME(pleshakov): use a more specific message from the defined constants (available in v1beta1) + reason = "NotAttached" // FIXME(pleshakov): use a more specific message from the defined constants } sectionName := name diff --git a/internal/status/updater.go b/internal/status/updater.go index 350edda04..6bc5c72fa 100644 --- a/internal/status/updater.go +++ b/internal/status/updater.go @@ -87,10 +87,15 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { // FIXME(pleshakov) Skip the status update (API call) if the status hasn't changed. if statuses.GatewayClassStatus != nil { - upd.update(ctx, types.NamespacedName{Name: upd.cfg.GatewayClassName}, &v1beta1.GatewayClass{}, func(object client.Object) { - gc := object.(*v1beta1.GatewayClass) - gc.Status = prepareGatewayClassStatus(*statuses.GatewayClassStatus, upd.cfg.Clock.Now()) - }) + upd.update( + ctx, + types.NamespacedName{Name: upd.cfg.GatewayClassName}, + &v1beta1.GatewayClass{}, + func(object client.Object) { + gc := object.(*v1beta1.GatewayClass) + gc.Status = prepareGatewayClassStatus(*statuses.GatewayClassStatus, upd.cfg.Clock.Now()) + }, + ) } if statuses.GatewayStatus != nil { @@ -123,12 +128,22 @@ func (upd *updaterImpl) Update(ctx context.Context, statuses state.Statuses) { upd.update(ctx, nsname, &v1beta1.HTTPRoute{}, func(object client.Object) { hr := object.(*v1beta1.HTTPRoute) // statuses.GatewayStatus is never nil when len(statuses.HTTPRouteStatuses) > 0 - hr.Status = prepareHTTPRouteStatus(rs, statuses.GatewayStatus.NsName, upd.cfg.GatewayCtlrName, upd.cfg.Clock.Now()) + hr.Status = prepareHTTPRouteStatus( + rs, + statuses.GatewayStatus.NsName, + upd.cfg.GatewayCtlrName, + upd.cfg.Clock.Now(), + ) }) } } -func (upd *updaterImpl) update(ctx context.Context, nsname types.NamespacedName, obj client.Object, statusSetter func(client.Object)) { +func (upd *updaterImpl) update( + ctx context.Context, + nsname types.NamespacedName, + obj client.Object, + statusSetter func(client.Object), +) { // The function handles errors by reporting them in the logs. // FIXME(pleshakov): figure out appropriate log level for these errors. Perhaps 3? diff --git a/internal/status/updater_test.go b/internal/status/updater_test.go index 8dba0db10..e5c833495 100644 --- a/internal/status/updater_test.go +++ b/internal/status/updater_test.go @@ -103,7 +103,12 @@ var _ = Describe("Updater", func() { } } - createExpectedGc = func(status metav1.ConditionStatus, generation int64, reason string, msg string) *v1beta1.GatewayClass { + createExpectedGc = func( + status metav1.ConditionStatus, + generation int64, + reason string, + msg string, + ) *v1beta1.GatewayClass { return &v1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ Name: gcName, @@ -279,7 +284,12 @@ var _ = Describe("Updater", func() { 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 := createExpectedGc( + metav1.ConditionTrue, + 1, + string(v1beta1.GatewayClassConditionStatusAccepted), + "GatewayClass has been accepted", + ) err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) Expect(err).Should(Not(HaveOccurred())) @@ -305,7 +315,11 @@ var _ = Describe("Updater", func() { latestGw := &v1beta1.Gateway{} expectedGw := createExpectedIgnoredGw() - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, latestGw) + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, + latestGw, + ) Expect(err).Should(Not(HaveOccurred())) expectedGw.ResourceVersion = latestGw.ResourceVersion @@ -334,7 +348,12 @@ var _ = Describe("Updater", func() { 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 := createExpectedGc( + metav1.ConditionFalse, + 2, + string(v1beta1.GatewayClassConditionStatusAccepted), + "GatewayClass has been rejected: error", + ) err := client.Get(context.Background(), types.NamespacedName{Name: gcName}, latestGc) Expect(err).Should(Not(HaveOccurred())) @@ -348,7 +367,11 @@ var _ = Describe("Updater", func() { latestGw := &v1beta1.Gateway{} expectedGw := createExpectedGw(metav1.ConditionFalse, string(v1beta1.ListenerReasonInvalid)) - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "gateway"}, latestGw) + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "test", Name: "gateway"}, + latestGw, + ) Expect(err).Should(Not(HaveOccurred())) expectedGw.ResourceVersion = latestGw.ResourceVersion @@ -360,7 +383,11 @@ var _ = Describe("Updater", func() { latestGw := &v1beta1.Gateway{} expectedGw := createExpectedIgnoredGw() - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, latestGw) + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "test", Name: "ignored-gateway"}, + latestGw, + ) Expect(err).Should(Not(HaveOccurred())) expectedGw.ResourceVersion = latestGw.ResourceVersion @@ -373,7 +400,11 @@ var _ = Describe("Updater", func() { latestHR := &v1beta1.HTTPRoute{} expectedHR := createExpectedHR() - err := client.Get(context.Background(), types.NamespacedName{Namespace: "test", Name: "route1"}, latestHR) + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "test", Name: "route1"}, + latestHR, + ) Expect(err).Should(Not(HaveOccurred())) expectedHR.ResourceVersion = latestHR.ResourceVersion