Skip to content

Commit

Permalink
Enable lll linter and make changes (#287)
Browse files Browse the repository at this point in the history
Sets line-length limit to 120 characters and makes necessary changes
  • Loading branch information
kate-osborn authored Nov 3, 2022
1 parent aebbdeb commit 29caa00
Show file tree
Hide file tree
Showing 28 changed files with 1,331 additions and 875 deletions.
4 changes: 3 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ linters-settings:
govet:
enable:
- fieldalignment

lll:
line-length: 120
linters:
enable:
- asciicheck
Expand All @@ -49,6 +50,7 @@ linters:
- gosimple
- govet
- ineffassign
- lll
- makezero
- misspell
- nilerr
Expand Down
14 changes: 7 additions & 7 deletions cmd/gateway/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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() {
Expand Down
7 changes: 4 additions & 3 deletions cmd/gateway/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion internal/events/first_eventbatch_preparer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
44 changes: 25 additions & 19 deletions internal/events/first_eventbatch_preparer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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"}}

Expand Down Expand Up @@ -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() {
Expand Down
85 changes: 69 additions & 16 deletions internal/events/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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"},
},
),
)
})

Expand Down Expand Up @@ -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},
}

Expand All @@ -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
Expand Down
17 changes: 9 additions & 8 deletions internal/helpers/helpers.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package helpers contains helper functions for unit tests.
package helpers

import (
Expand All @@ -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
}
Loading

0 comments on commit 29caa00

Please sign in to comment.