Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable lll linter and make changes #287

Merged
merged 2 commits into from
Nov 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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