diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index fa0f9423c..38b7f6677 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -124,7 +124,8 @@ func createStaticModeCommand() *cobra.Command { Short: "Configure NGINX in the scope of a single Gateway resource", RunE: func(cmd *cobra.Command, args []string) error { logger := zap.New() - logger.Info("Starting NGINX Kubernetes Gateway in static mode", + logger.Info( + "Starting NGINX Kubernetes Gateway in static mode", "version", version, "commit", commit, "date", date, @@ -184,7 +185,8 @@ func createProvisionerModeCommand() *cobra.Command { Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { logger := zap.New() - logger.Info("Starting NGINX Kubernetes Gateway Provisioner", + logger.Info( + "Starting NGINX Kubernetes Gateway Provisioner", "version", version, "commit", commit, "date", date, diff --git a/docs/developer/go-style-guide.md b/docs/developer/go-style-guide.md index d291efd83..4be183e44 100644 --- a/docs/developer/go-style-guide.md +++ b/docs/developer/go-style-guide.md @@ -45,20 +45,27 @@ signaller <- false // is this a signal? is this an error? ### Consistent Line Breaks -When breaking up a long function definition, call, or struct initialization, choose to break after each parameter, +When breaking up a (1) long function definition, (2) call, or (3) struct initialization, choose to break after each parameter, argument, or field. DO: ```go +// 1 func longFunctionDefinition( paramX int, paramY string, paramZ bool, ) (string, error){} -// and +// 2 +callWithManyArguments( + arg1, + arg2, + arg3, +) +// 3 s := myStruct{ field1: 1, field2: 2, @@ -69,6 +76,7 @@ s := myStruct{ DO NOT: ```go +// 1 func longFunctionDefinition(paramX int, paramY string, paramZ bool, ) (string, error){} @@ -80,12 +88,25 @@ func longFunctionDefinition( paramZ bool, ) (string, error){} -// or +// 2 +callWithManyArguments(arg1, arg2, + arg3) + +// 3 s := myStruct{field1: 1, field2: 2, field3: 3} ``` +> **Exception**: Calls to ginkgo helper functions like below in tests are OK (because of the readability): +> +>```go +>DescribeTable("Edge cases for events", +> func(e interface{}) { +> . . . +>) +>``` + When constructing structs pass members during initialization. Example: diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 6b615ec45..f9afdcd32 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -133,7 +133,9 @@ func (upd *updaterImpl) update( err := upd.cfg.Client.Get(ctx, nsname, obj) if err != nil { if !apierrors.IsNotFound(err) { - upd.cfg.Logger.Error(err, "Failed to get the recent version the resource when updating status", + upd.cfg.Logger.Error( + err, + "Failed to get the recent version the resource when updating status", "namespace", nsname.Namespace, "name", nsname.Name, "kind", obj.GetObjectKind().GroupVersionKind().Kind) @@ -145,7 +147,9 @@ func (upd *updaterImpl) update( err = upd.cfg.Client.Status().Update(ctx, obj) if err != nil { - upd.cfg.Logger.Error(err, "Failed to update status", + upd.cfg.Logger.Error( + err, + "Failed to update status", "namespace", nsname.Namespace, "name", nsname.Name, "kind", obj.GetObjectKind().GroupVersionKind().Kind) diff --git a/internal/mode/provisioner/handler.go b/internal/mode/provisioner/handler.go index 3e760608e..e53860d76 100644 --- a/internal/mode/provisioner/handler.go +++ b/internal/mode/provisioner/handler.go @@ -117,7 +117,8 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) { h.provisions[nsname] = deployment - h.logger.Info("Created deployment", + h.logger.Info( + "Created deployment", "deployment", client.ObjectKeyFromObject(deployment), "gateway", nsname, ) @@ -135,7 +136,8 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) { delete(h.provisions, nsname) - h.logger.Info("Deleted deployment", + h.logger.Info( + "Deleted deployment", "deployment", client.ObjectKeyFromObject(deployment), "gateway", nsname, ) diff --git a/internal/mode/static/nginx/config/validation/common.go b/internal/mode/static/nginx/config/validation/common.go index 988e7848f..ca2b04d97 100644 --- a/internal/mode/static/nginx/config/validation/common.go +++ b/internal/mode/static/nginx/config/validation/common.go @@ -41,8 +41,11 @@ var escapedStringsNoVarExpansionFmtRegexp = regexp.MustCompile("^" + escapedStri // If the value is invalid, the function returns an error that includes the specified examples of valid values. func validateEscapedStringNoVarExpansion(value string, examples []string) error { if !escapedStringsNoVarExpansionFmtRegexp.MatchString(value) { - msg := k8svalidation.RegexError(escapedStringsNoVarExpansionErrMsg, escapedStringsNoVarExpansionFmt, - examples...) + msg := k8svalidation.RegexError( + escapedStringsNoVarExpansionErrMsg, + escapedStringsNoVarExpansionFmt, + examples..., + ) return errors.New(msg) } return nil diff --git a/internal/mode/static/nginx/config/validation/common_test.go b/internal/mode/static/nginx/config/validation/common_test.go index 925d5ac8f..9d8525b79 100644 --- a/internal/mode/static/nginx/config/validation/common_test.go +++ b/internal/mode/static/nginx/config/validation/common_test.go @@ -8,39 +8,56 @@ import ( func TestValidateEscapedString(t *testing.T) { validator := func(value string) error { return validateEscapedString(value, []string{"example"}) } - testValidValuesForSimpleValidator(t, validator, + testValidValuesForSimpleValidator( + t, + validator, `test`, `test test`, `\"`, - `\\`) - testInvalidValuesForSimpleValidator(t, validator, + `\\`, + ) + testInvalidValuesForSimpleValidator( + t, + validator, `\`, - `test"test`) + `test"test`, + ) } func TestValidateEscapedStringNoVarExpansion(t *testing.T) { validator := func(value string) error { return validateEscapedStringNoVarExpansion(value, []string{"example"}) } - testValidValuesForSimpleValidator(t, validator, + testValidValuesForSimpleValidator( + t, + validator, `test`, `test test`, `\"`, - `\\`) - testInvalidValuesForSimpleValidator(t, validator, + `\\`, + ) + testInvalidValuesForSimpleValidator( + t, + validator, `\`, `test"test`, - `$test`) + `$test`, + ) } func TestValidateValidHeaderName(t *testing.T) { validator := func(value string) error { return validateHeaderName(value) } - testValidValuesForSimpleValidator(t, validator, + testValidValuesForSimpleValidator( + t, + validator, `Content-Encoding`, `X-Forwarded-For`, // max supported length is 256, generate string with 16*16 chars (256) - strings.Repeat("very-long-header", 16)) - testInvalidValuesForSimpleValidator(t, validator, + strings.Repeat("very-long-header", 16), + ) + testInvalidValuesForSimpleValidator( + t, + validator, `\`, `test test`, `test"test`, @@ -49,5 +66,6 @@ func TestValidateValidHeaderName(t *testing.T) { "host", "my-header[]", "my-header&", - strings.Repeat("very-long-header", 16)+"1") + strings.Repeat("very-long-header", 16)+"1", + ) } diff --git a/internal/mode/static/nginx/config/validation/framework_test.go b/internal/mode/static/nginx/config/validation/framework_test.go index 33f935ca5..412deaa04 100644 --- a/internal/mode/static/nginx/config/validation/framework_test.go +++ b/internal/mode/static/nginx/config/validation/framework_test.go @@ -43,11 +43,16 @@ func testValidValuesForSupportedValuesValidator[T configValue]( f supportedValuesValidatorFunc[T], values ...T, ) { - runValidatorTests(t, func(g *WithT, v T) { - valid, supportedValues := f(v) - g.Expect(valid).To(BeTrue(), createFailureMessage(v)) - g.Expect(supportedValues).To(BeNil(), createFailureMessage(v)) - }, "valid_value", values...) + runValidatorTests( + t, + func(g *WithT, v T) { + valid, supportedValues := f(v) + g.Expect(valid).To(BeTrue(), createFailureMessage(v)) + g.Expect(supportedValues).To(BeNil(), createFailureMessage(v)) + }, + "valid_value", + values..., + ) } func testInvalidValuesForSupportedValuesValidator[T configValue]( @@ -56,11 +61,16 @@ func testInvalidValuesForSupportedValuesValidator[T configValue]( supportedValuesMap map[T]struct{}, values ...T, ) { - runValidatorTests(t, func(g *WithT, v T) { - valid, supportedValues := f(v) - g.Expect(valid).To(BeFalse(), createFailureMessage(v)) - g.Expect(supportedValues).To(Equal(getSortedKeysAsString(supportedValuesMap)), createFailureMessage(v)) - }, "invalid_value", values...) + runValidatorTests( + t, + func(g *WithT, v T) { + valid, supportedValues := f(v) + g.Expect(valid).To(BeFalse(), createFailureMessage(v)) + g.Expect(supportedValues).To(Equal(getSortedKeysAsString(supportedValuesMap)), createFailureMessage(v)) + }, + "invalid_value", + values..., + ) } func TestValidateInSupportedValues(t *testing.T) { @@ -74,12 +84,19 @@ func TestValidateInSupportedValues(t *testing.T) { return validateInSupportedValues(value, supportedValues) } - testValidValuesForSupportedValuesValidator(t, validator, + testValidValuesForSupportedValuesValidator( + t, + validator, "value1", "value2", - "value3") - testInvalidValuesForSupportedValuesValidator(t, validator, supportedValues, - "value4") + "value3", + ) + testInvalidValuesForSupportedValuesValidator( + t, + validator, + supportedValues, + "value4", + ) } func TestGetSortedKeysAsString(t *testing.T) { diff --git a/internal/mode/static/nginx/config/validation/http_filters_test.go b/internal/mode/static/nginx/config/validation/http_filters_test.go index d3abeb109..a3a14a265 100644 --- a/internal/mode/static/nginx/config/validation/http_filters_test.go +++ b/internal/mode/static/nginx/config/validation/http_filters_test.go @@ -8,49 +8,74 @@ import ( func TestValidateRedirectScheme(t *testing.T) { validator := HTTPRedirectValidator{} - testValidValuesForSupportedValuesValidator(t, validator.ValidateRedirectScheme, + testValidValuesForSupportedValuesValidator( + t, + validator.ValidateRedirectScheme, "http", - "https") - - testInvalidValuesForSupportedValuesValidator(t, validator.ValidateRedirectScheme, supportedRedirectSchemes, - "test") + "https", + ) + + testInvalidValuesForSupportedValuesValidator( + t, + validator.ValidateRedirectScheme, + supportedRedirectSchemes, + "test", + ) } func TestValidateRedirectHostname(t *testing.T) { validator := HTTPRedirectValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateRedirectHostname, - "example.com") - - testInvalidValuesForSimpleValidator(t, validator.ValidateRedirectHostname, - "example.com$") + testValidValuesForSimpleValidator( + t, + validator.ValidateRedirectHostname, + "example.com", + ) + + testInvalidValuesForSimpleValidator( + t, + validator.ValidateRedirectHostname, + "example.com$", + ) } func TestValidateRedirectPort(t *testing.T) { validator := HTTPRedirectValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateRedirectPort, + testValidValuesForSimpleValidator( + t, + validator.ValidateRedirectPort, math.MinInt32, - math.MaxInt32) + math.MaxInt32, + ) } func TestValidateRedirectStatusCode(t *testing.T) { validator := HTTPRedirectValidator{} - testValidValuesForSupportedValuesValidator(t, validator.ValidateRedirectStatusCode, + testValidValuesForSupportedValuesValidator( + t, + validator.ValidateRedirectStatusCode, 301, 302) - testInvalidValuesForSupportedValuesValidator(t, validator.ValidateRedirectStatusCode, supportedRedirectStatusCodes, - 404) + testInvalidValuesForSupportedValuesValidator( + t, + validator.ValidateRedirectStatusCode, + supportedRedirectStatusCodes, + 404, + ) } func TestValidateRequestHeaderName(t *testing.T) { validator := HTTPRequestHeaderValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateRequestHeaderName, + testValidValuesForSimpleValidator( + t, + validator.ValidateRequestHeaderName, "Content-Encoding", - "Connection") + "Connection", + ) testInvalidValuesForSimpleValidator(t, validator.ValidateRequestHeaderName, "$Content-Encoding") } @@ -58,13 +83,19 @@ func TestValidateRequestHeaderName(t *testing.T) { func TestValidateRequestHeaderValue(t *testing.T) { validator := HTTPRequestHeaderValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateRequestHeaderValue, + testValidValuesForSimpleValidator( + t, + validator.ValidateRequestHeaderValue, "my-cookie-name", "ssl_(server_name}", "example/1234==", - "1234:3456") + "1234:3456", + ) - testInvalidValuesForSimpleValidator(t, validator.ValidateRequestHeaderValue, + testInvalidValuesForSimpleValidator( + t, + validator.ValidateRequestHeaderValue, "$Content-Encoding", - `"example"`) + `"example"`, + ) } diff --git a/internal/mode/static/nginx/config/validation/http_njs_match_test.go b/internal/mode/static/nginx/config/validation/http_njs_match_test.go index ea5d4862b..06de3709f 100644 --- a/internal/mode/static/nginx/config/validation/http_njs_match_test.go +++ b/internal/mode/static/nginx/config/validation/http_njs_match_test.go @@ -7,79 +7,122 @@ import ( func TestValidatePathInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidatePathInMatch, + testValidValuesForSimpleValidator( + t, + validator.ValidatePathInMatch, "/", "/path", - "/path/subpath-123") - testInvalidValuesForSimpleValidator(t, validator.ValidatePathInMatch, + "/path/subpath-123", + ) + testInvalidValuesForSimpleValidator( + t, + validator.ValidatePathInMatch, "/ ", "/path{", "/path}", "/path;", "path", "", - "/path$") + "/path$", + ) } func TestValidateHeaderNameInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateHeaderNameInMatch, - "header") - testInvalidValuesForSimpleValidator(t, validator.ValidateHeaderNameInMatch, + testValidValuesForSimpleValidator( + t, + validator.ValidateHeaderNameInMatch, + "header", + ) + testInvalidValuesForSimpleValidator( + t, + validator.ValidateHeaderNameInMatch, ":", - "") + "", + ) } func TestValidateHeaderValueInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateHeaderValueInMatch, - "value") - testInvalidValuesForSimpleValidator(t, validator.ValidateHeaderValueInMatch, + testValidValuesForSimpleValidator( + t, + validator.ValidateHeaderValueInMatch, + "value", + ) + testInvalidValuesForSimpleValidator( + t, + validator.ValidateHeaderValueInMatch, ":", - "") + "", + ) } func TestValidateQueryParamNameInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateQueryParamNameInMatch, - "param") - testInvalidValuesForSimpleValidator(t, validator.ValidateQueryParamNameInMatch, - "") + testValidValuesForSimpleValidator( + t, + validator.ValidateQueryParamNameInMatch, + "param", + ) + testInvalidValuesForSimpleValidator( + t, + validator.ValidateQueryParamNameInMatch, + "", + ) } func TestValidateQueryParamValueInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidateQueryParamValueInMatch, - "value") - testInvalidValuesForSimpleValidator(t, validator.ValidateQueryParamValueInMatch, - "") + testValidValuesForSimpleValidator( + t, + validator.ValidateQueryParamValueInMatch, + "value", + ) + testInvalidValuesForSimpleValidator( + t, + validator.ValidateQueryParamValueInMatch, + "", + ) } func TestValidateMethodInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSupportedValuesValidator(t, validator.ValidateMethodInMatch, + testValidValuesForSupportedValuesValidator( + t, + validator.ValidateMethodInMatch, "GET", "HEAD", "POST", "PUT", "DELETE", "OPTIONS", - "PATCH") - testInvalidValuesForSupportedValuesValidator(t, validator.ValidateMethodInMatch, supportedMethods, + "PATCH", + ) + testInvalidValuesForSupportedValuesValidator( + t, + validator.ValidateMethodInMatch, + supportedMethods, "GOT", - "TRACE") + "TRACE", + ) } func TestValidateCommonMatchPart(t *testing.T) { - testValidValuesForSimpleValidator(t, validateCommonNJSMatchPart, - "test") - testInvalidValuesForSimpleValidator(t, validateCommonNJSMatchPart, + testValidValuesForSimpleValidator( + t, + validateCommonNJSMatchPart, + "test", + ) + testInvalidValuesForSimpleValidator( + t, + validateCommonNJSMatchPart, "", " ", - "$") + "$", + ) } diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go index 2f954f64d..715148d56 100644 --- a/internal/mode/static/state/graph/httproute_test.go +++ b/internal/mode/static/state/graph/httproute_test.go @@ -471,7 +471,8 @@ func TestBuildRoute(t *testing.T) { Conditions: []conditions.Condition{ staticConds.NewRouteUnsupportedValue( `All rules are invalid: spec.rules[0].filters[0].requestRedirect.hostname: ` + - `Invalid value: "invalid.example.com": invalid hostname`), + `Invalid value: "invalid.example.com": invalid hostname`, + ), }, Rules: []Rule{ { diff --git a/internal/mode/static/state/relationship/relationships_test.go b/internal/mode/static/state/relationship/relationships_test.go index ce15a03cf..b7d32ccae 100644 --- a/internal/mode/static/state/relationship/relationships_test.go +++ b/internal/mode/static/state/relationship/relationships_test.go @@ -45,7 +45,8 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { BackendRefs: getNormalRefs("svc1"), // duplicate }, { - BackendRefs: getModifiedRefs("invalid-kind", + BackendRefs: getModifiedRefs( + "invalid-kind", func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { refs[0].Kind = (*v1beta1.Kind)(helpers.GetStringPointer("Invalid")) return refs @@ -53,7 +54,8 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { ), }, { - BackendRefs: getModifiedRefs("nil-namespace", + BackendRefs: getModifiedRefs( + "nil-namespace", func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { refs[0].Namespace = nil return refs @@ -61,7 +63,8 @@ func TestGetBackendServiceNamesFromRoute(t *testing.T) { ), }, { - BackendRefs: getModifiedRefs("diff-namespace", + BackendRefs: getModifiedRefs( + "diff-namespace", func(refs []v1beta1.HTTPBackendRef) []v1beta1.HTTPBackendRef { refs[0].Namespace = (*v1beta1.Namespace)( helpers.GetStringPointer("not-test"), @@ -159,8 +162,12 @@ func TestCapturerImpl_DecrementRouteCount(t *testing.T) { } if tc.expectedRefCount != count { - t.Errorf("decrementRefCount() test case %q expected ref count to be %d, got %d", tc.msg, - tc.expectedRefCount, count) + t.Errorf( + "decrementRefCount() test case %q expected ref count to be %d, got %d", + tc.msg, + tc.expectedRefCount, + count, + ) } } }