From 826bdb7e30c9ed1e88e1d69de2bd8fb989d126b3 Mon Sep 17 00:00:00 2001 From: z1cheng Date: Thu, 10 Aug 2023 14:21:24 +0000 Subject: [PATCH] Fix errcheck lint errors Signed-off-by: z1cheng --- cmd/dbg/main.go | 20 ++++- cmd/plugin/lints/deployment.go | 5 +- cmd/plugin/lints/ingress.go | 5 +- cmd/plugin/util/util.go | 16 +++- .../ingress/annotations/annotations_test.go | 6 +- .../ingress/annotations/auth/main_test.go | 5 +- .../annotations/authreqglobal/main_test.go | 5 +- .../clientbodybuffersize/main_test.go | 1 + .../annotations/globalratelimit/main_test.go | 5 +- .../annotations/loadbalancing/main_test.go | 1 + internal/ingress/annotations/log/main_test.go | 15 +++- .../annotations/modsecurity/main_test.go | 10 ++- .../annotations/opentelemetry/main_test.go | 15 +++- .../annotations/opentracing/main_test.go | 21 ++++- .../ingress/annotations/parser/main_test.go | 6 +- .../ingress/annotations/rewrite/main_test.go | 35 ++++++-- .../annotations/serversnippet/main_test.go | 1 + .../annotations/serviceupstream/main_test.go | 25 ++++-- .../annotations/sessionaffinity/main_test.go | 6 +- .../ingress/annotations/snippet/main_test.go | 1 + .../annotations/streamsnippet/main_test.go | 1 + .../annotations/xforwardedprefix/main_test.go | 1 + internal/ingress/controller/controller.go | 7 +- internal/ingress/controller/nginx.go | 15 +++- internal/ingress/controller/process/nginx.go | 5 +- internal/ingress/controller/status.go | 8 +- internal/ingress/controller/store/store.go | 80 +++++++++++++++---- .../ingress/controller/store/store_test.go | 61 +++++++++++--- .../ingress/controller/template/configmap.go | 15 +++- .../ingress/controller/template/template.go | 12 ++- .../controller/template/template_test.go | 10 ++- .../metric/collectors/controller_test.go | 16 +++- .../ingress/metric/collectors/process_test.go | 5 +- internal/ingress/metric/collectors/socket.go | 1 + .../ingress/metric/collectors/socket_test.go | 5 +- internal/ingress/status/status.go | 5 +- internal/ingress/status/status_test.go | 26 ++++-- internal/k8s/main.go | 5 +- internal/net/ipnet_test.go | 8 +- internal/net/net.go | 5 +- internal/net/ssl/ssl.go | 11 ++- internal/nginx/main.go | 5 +- internal/nginx/maxmind.go | 2 +- internal/task/queue.go | 5 +- pkg/apis/ingress/types_equals_test.go | 16 +++- pkg/tcpproxy/tcp.go | 10 ++- test/e2e/annotations/affinity.go | 18 +++-- test/e2e/annotations/grpc.go | 9 ++- test/e2e/annotations/upstreamhashby.go | 4 +- test/e2e/framework/k8s.go | 6 +- test/e2e/loadbalance/ewma.go | 4 +- test/e2e/loadbalance/round_robin.go | 4 +- test/e2e/settings/keep-alive.go | 13 ++- test/e2e/status/update.go | 3 +- 54 files changed, 473 insertions(+), 132 deletions(-) diff --git a/cmd/dbg/main.go b/cmd/dbg/main.go index fd3e5ece6..b63bcf5fe 100644 --- a/cmd/dbg/main.go +++ b/cmd/dbg/main.go @@ -154,10 +154,16 @@ func backendsList() { fmt.Println(unmarshalErr) return } - backends := f.([]interface{}) + backends, ok := f.([]interface{}) + if !ok { + fmt.Printf("unexpected type: %T", f) + } for _, backendi := range backends { - backend := backendi.(map[string]interface{}) + backend, ok := backendi.(map[string]interface{}) + if !ok { + fmt.Printf("unexpected type: %T", backendi) + } fmt.Println(backend["name"].(string)) } } @@ -179,10 +185,16 @@ func backendsGet(name string) { fmt.Println(unmarshalErr) return } - backends := f.([]interface{}) + backends, ok := f.([]interface{}) + if !ok { + fmt.Printf("unexpected type: %T", f) + } for _, backendi := range backends { - backend := backendi.(map[string]interface{}) + backend, ok := backendi.(map[string]interface{}) + if !ok { + fmt.Printf("unexpected type: %T", backendi) + } if backend["name"].(string) == name { printed, err := json.MarshalIndent(backend, "", " ") if err != nil { diff --git a/cmd/plugin/lints/deployment.go b/cmd/plugin/lints/deployment.go index f07793f62..ce1712284 100644 --- a/cmd/plugin/lints/deployment.go +++ b/cmd/plugin/lints/deployment.go @@ -35,7 +35,10 @@ type DeploymentLint struct { // Check returns true if the lint detects an issue func (lint DeploymentLint) Check(obj kmeta.Object) bool { - cmp := obj.(*v1.Deployment) + cmp, ok := obj.(*v1.Deployment) + if !ok { + util.PrintError(fmt.Errorf("unexpected type: %T", obj)) + } return lint.f(*cmp) } diff --git a/cmd/plugin/lints/ingress.go b/cmd/plugin/lints/ingress.go index 2d8de5b31..d5ad42e2c 100644 --- a/cmd/plugin/lints/ingress.go +++ b/cmd/plugin/lints/ingress.go @@ -35,7 +35,10 @@ type IngressLint struct { // Check returns true if the lint detects an issue func (lint IngressLint) Check(obj kmeta.Object) bool { - ing := obj.(*networking.Ingress) + ing, ok := obj.(*networking.Ingress) + if !ok { + util.PrintError(fmt.Errorf("unexpected type: %T", obj)) + } return lint.f(ing) } diff --git a/cmd/plugin/util/util.go b/cmd/plugin/util/util.go index 5090fc64a..aa753689a 100644 --- a/cmd/plugin/util/util.go +++ b/cmd/plugin/util/util.go @@ -54,10 +54,18 @@ func ParseVersionString(v string) (major, minor, patch int, err error) { return 0, 0, 0, fmt.Errorf("could not parse %v as a version string (like 0.20.3)", v) } - major, _ = strconv.Atoi(parts[1]) - minor, _ = strconv.Atoi(parts[2]) - patch, _ = strconv.Atoi(parts[3]) - + major, err = strconv.Atoi(parts[1]) + if err != nil { + return 0, 0, 0, err + } + minor, err = strconv.Atoi(parts[2]) + if err != nil { + return 0, 0, 0, err + } + patch, err = strconv.Atoi(parts[3]) + if err != nil { + return 0, 0, 0, err + } return major, minor, patch, nil } diff --git a/internal/ingress/annotations/annotations_test.go b/internal/ingress/annotations/annotations_test.go index 7125dc0be..5f8128e0d 100644 --- a/internal/ingress/annotations/annotations_test.go +++ b/internal/ingress/annotations/annotations_test.go @@ -64,7 +64,11 @@ func (m mockCfg) GetService(name string) (*apiv1.Service, error) { } func (m mockCfg) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { - if secret, _ := m.GetSecret(name); secret != nil { + secret, err := m.GetSecret(name) + if err != nil { + return nil, err + } + if secret != nil { return &resolver.AuthSSLCert{ Secret: name, CAFileName: "/opt/ca.pem", diff --git a/internal/ingress/annotations/auth/main_test.go b/internal/ingress/annotations/auth/main_test.go index d85224776..868f31a05 100644 --- a/internal/ingress/annotations/auth/main_test.go +++ b/internal/ingress/annotations/auth/main_test.go @@ -310,7 +310,10 @@ func dummySecretContent(t *testing.T) (fileName, dir string, s *api.Secret) { t.Error(err) } defer tmpfile.Close() - s, _ = mockSecret{}.GetSecret(defaultDemoSecret) + s, err = mockSecret{}.GetSecret(defaultDemoSecret) + if err != nil { + t.Errorf("unexpected error: %v", err) + } return tmpfile.Name(), dir, s } diff --git a/internal/ingress/annotations/authreqglobal/main_test.go b/internal/ingress/annotations/authreqglobal/main_test.go index 0313edcf5..734f97c0f 100644 --- a/internal/ingress/annotations/authreqglobal/main_test.go +++ b/internal/ingress/annotations/authreqglobal/main_test.go @@ -77,7 +77,10 @@ func TestAnnotation(t *testing.T) { data[parser.GetAnnotationWithPrefix("enable-global-auth")] = "false" ing.SetAnnotations(data) - i, _ := NewParser(&resolver.Mock{}).Parse(ing) + i, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } u, ok := i.(bool) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/clientbodybuffersize/main_test.go b/internal/ingress/annotations/clientbodybuffersize/main_test.go index 0f2c8474a..62257aeb9 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main_test.go +++ b/internal/ingress/annotations/clientbodybuffersize/main_test.go @@ -57,6 +57,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/annotations/globalratelimit/main_test.go b/internal/ingress/annotations/globalratelimit/main_test.go index ded6da6af..b1a7ab71b 100644 --- a/internal/ingress/annotations/globalratelimit/main_test.go +++ b/internal/ingress/annotations/globalratelimit/main_test.go @@ -192,7 +192,10 @@ func TestGlobalRateLimiting(t *testing.T) { t.Errorf("expected error '%v' but got '%v'", testCase.expectedErr, actualErr) } - actualConfig := i.(*Config) + actualConfig, ok := i.(*Config) + if !ok { + t.Errorf("expected Config type but got %T", i) + } if !testCase.expectedConfig.Equal(actualConfig) { expectedJSON, err := json.Marshal(testCase.expectedConfig) if err != nil { diff --git a/internal/ingress/annotations/loadbalancing/main_test.go b/internal/ingress/annotations/loadbalancing/main_test.go index b0442c37f..a5205e523 100644 --- a/internal/ingress/annotations/loadbalancing/main_test.go +++ b/internal/ingress/annotations/loadbalancing/main_test.go @@ -54,6 +54,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/annotations/log/main_test.go b/internal/ingress/annotations/log/main_test.go index df97c2e5b..950612b5b 100644 --- a/internal/ingress/annotations/log/main_test.go +++ b/internal/ingress/annotations/log/main_test.go @@ -76,7 +76,10 @@ func TestIngressAccessLogConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableAccessLogAnnotation)] = "false" ing.SetAnnotations(data) - log, _ := NewParser(&resolver.Mock{}).Parse(ing) + log, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } nginxLogs, ok := log.(*Config) if !ok { t.Errorf("expected a Config type") @@ -94,7 +97,10 @@ func TestIngressRewriteLogConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableRewriteLogAnnotation)] = "true" ing.SetAnnotations(data) - log, _ := NewParser(&resolver.Mock{}).Parse(ing) + log, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error parsing annotations %v", err) + } nginxLogs, ok := log.(*Config) if !ok { t.Errorf("expected a Config type") @@ -112,7 +118,10 @@ func TestInvalidBoolConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableRewriteLogAnnotation)] = "blo" ing.SetAnnotations(data) - log, _ := NewParser(&resolver.Mock{}).Parse(ing) + log, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } nginxLogs, ok := log.(*Config) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/modsecurity/main_test.go b/internal/ingress/annotations/modsecurity/main_test.go index 2ddbdf7e3..0c9b3a9c7 100644 --- a/internal/ingress/annotations/modsecurity/main_test.go +++ b/internal/ingress/annotations/modsecurity/main_test.go @@ -69,8 +69,14 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) - result, _ := ap.Parse(ing) - config := result.(*Config) + result, err := ap.Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + config, ok := result.(*Config) + if !ok { + t.Errorf("unexpected type: %T", result) + } if !config.Equal(&testCase.expected) { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) } diff --git a/internal/ingress/annotations/opentelemetry/main_test.go b/internal/ingress/annotations/opentelemetry/main_test.go index 96a13432c..e5c1de0b3 100644 --- a/internal/ingress/annotations/opentelemetry/main_test.go +++ b/internal/ingress/annotations/opentelemetry/main_test.go @@ -78,7 +78,10 @@ func TestIngressAnnotationOpentelemetrySetTrue(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableOpenTelemetryAnnotation)] = enableAnnotation ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } openTelemetry, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") @@ -105,7 +108,10 @@ func TestIngressAnnotationOpentelemetrySetFalse(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableOpenTelemetryAnnotation)] = "false" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } openTelemetry, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") @@ -182,7 +188,10 @@ func TestIngressAnnotationOpentelemetryUnset(t *testing.T) { data := map[string]string{} ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } _, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/opentracing/main_test.go b/internal/ingress/annotations/opentracing/main_test.go index f6cfcebc8..f59e60438 100644 --- a/internal/ingress/annotations/opentracing/main_test.go +++ b/internal/ingress/annotations/opentracing/main_test.go @@ -78,7 +78,10 @@ func TestIngressAnnotationOpentracingSetTrue(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableOpentracingAnnotation)] = enableAnnotation ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error %v", err) + } openTracing, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") @@ -97,7 +100,10 @@ func TestIngressAnnotationOpentracingSetFalse(t *testing.T) { data[parser.GetAnnotationWithPrefix(enableOpentracingAnnotation)] = "false" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error %v", err) + } openTracing, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") @@ -116,7 +122,10 @@ func TestIngressAnnotationOpentracingTrustSetTrue(t *testing.T) { data[parser.GetAnnotationWithPrefix(opentracingTrustSpanAnnotation)] = enableAnnotation ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error %v", err) + } openTracing, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") @@ -138,7 +147,11 @@ func TestIngressAnnotationOpentracingUnset(t *testing.T) { data := map[string]string{} ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + _, ok := val.(*Config) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/parser/main_test.go b/internal/ingress/annotations/parser/main_test.go index 19c73bbc2..0f7ecb459 100644 --- a/internal/ingress/annotations/parser/main_test.go +++ b/internal/ingress/annotations/parser/main_test.go @@ -215,8 +215,10 @@ func TestGetIntAnnotation(t *testing.T) { func TestStringToURL(t *testing.T) { validURL := "http://bar.foo.com/external-auth" - validParsedURL, _ := url.Parse(validURL) - + validParsedURL, err := url.Parse(validURL) + if err != nil { + t.Errorf("unexpected error: %v", err) + } tests := []struct { title string url string diff --git a/internal/ingress/annotations/rewrite/main_test.go b/internal/ingress/annotations/rewrite/main_test.go index 27c5c0c4e..b68b901b4 100644 --- a/internal/ingress/annotations/rewrite/main_test.go +++ b/internal/ingress/annotations/rewrite/main_test.go @@ -120,7 +120,10 @@ func TestSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("rewrite-target")] = defRoute ing.SetAnnotations(data) - i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) + i, err := NewParser(mockBackend{redirect: true}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok := i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -132,7 +135,10 @@ func TestSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("rewrite-target")] = "/xpto/$1/abc/$2" ing.SetAnnotations(data) - i, _ = NewParser(mockBackend{redirect: true}).Parse(ing) + i, err = NewParser(mockBackend{redirect: true}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok = i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -144,7 +150,10 @@ func TestSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("rewrite-target")] = "/xpto/xas{445}" ing.SetAnnotations(data) - i, _ = NewParser(mockBackend{redirect: true}).Parse(ing) + i, err = NewParser(mockBackend{redirect: true}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok = i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -156,7 +165,10 @@ func TestSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("ssl-redirect")] = "false" ing.SetAnnotations(data) - i, _ = NewParser(mockBackend{redirect: false}).Parse(ing) + i, err = NewParser(mockBackend{redirect: false}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok = i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -173,7 +185,10 @@ func TestForceSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("rewrite-target")] = defRoute ing.SetAnnotations(data) - i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) + i, err := NewParser(mockBackend{redirect: true}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok := i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -185,7 +200,10 @@ func TestForceSSLRedirect(t *testing.T) { data[parser.GetAnnotationWithPrefix("force-ssl-redirect")] = "true" ing.SetAnnotations(data) - i, _ = NewParser(mockBackend{redirect: false}).Parse(ing) + i, err = NewParser(mockBackend{redirect: false}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok = i.(*Config) if !ok { t.Errorf("expected a Redirect type") @@ -242,7 +260,10 @@ func TestUseRegex(t *testing.T) { data[parser.GetAnnotationWithPrefix("use-regex")] = "true" ing.SetAnnotations(data) - i, _ := NewParser(mockBackend{redirect: true}).Parse(ing) + i, err := NewParser(mockBackend{redirect: true}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } redirect, ok := i.(*Config) if !ok { t.Errorf("expected a App Context") diff --git a/internal/ingress/annotations/serversnippet/main_test.go b/internal/ingress/annotations/serversnippet/main_test.go index 601e11a42..72e757dc1 100644 --- a/internal/ingress/annotations/serversnippet/main_test.go +++ b/internal/ingress/annotations/serversnippet/main_test.go @@ -54,6 +54,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/annotations/serviceupstream/main_test.go b/internal/ingress/annotations/serviceupstream/main_test.go index 2751208ec..e6216ee5a 100644 --- a/internal/ingress/annotations/serviceupstream/main_test.go +++ b/internal/ingress/annotations/serviceupstream/main_test.go @@ -77,7 +77,10 @@ func TestIngressAnnotationServiceUpstreamEnabled(t *testing.T) { data[parser.GetAnnotationWithPrefix(serviceUpstreamAnnotation)] = "true" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } enabled, ok := val.(bool) if !ok { t.Errorf("expected a bool type") @@ -96,7 +99,10 @@ func TestIngressAnnotationServiceUpstreamSetFalse(t *testing.T) { data[parser.GetAnnotationWithPrefix(serviceUpstreamAnnotation)] = "false" ing.SetAnnotations(data) - val, _ := NewParser(&resolver.Mock{}).Parse(ing) + val, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } enabled, ok := val.(bool) if !ok { t.Errorf("expected a bool type") @@ -110,7 +116,10 @@ func TestIngressAnnotationServiceUpstreamSetFalse(t *testing.T) { data = map[string]string{} ing.SetAnnotations(data) - val, _ = NewParser(&resolver.Mock{}).Parse(ing) + val, err = NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } enabled, ok = val.(bool) if !ok { t.Errorf("expected a bool type") @@ -137,7 +146,10 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend { func TestParseAnnotationsWithDefaultConfig(t *testing.T) { ing := buildIngress() - val, _ := NewParser(mockBackend{}).Parse(ing) + val, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } enabled, ok := val.(bool) if !ok { @@ -158,7 +170,10 @@ func TestParseAnnotationsOverridesDefaultConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(serviceUpstreamAnnotation)] = "false" ing.SetAnnotations(data) - val, _ := NewParser(mockBackend{}).Parse(ing) + val, err := NewParser(mockBackend{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error: %v", err) + } enabled, ok := val.(bool) if !ok { diff --git a/internal/ingress/annotations/sessionaffinity/main_test.go b/internal/ingress/annotations/sessionaffinity/main_test.go index cffe57fec..cecf8cf8f 100644 --- a/internal/ingress/annotations/sessionaffinity/main_test.go +++ b/internal/ingress/annotations/sessionaffinity/main_test.go @@ -83,7 +83,11 @@ func TestIngressAffinityCookieConfig(t *testing.T) { data[parser.GetAnnotationWithPrefix(annotationAffinityCookieSecure)] = "true" ing.SetAnnotations(data) - affin, _ := NewParser(&resolver.Mock{}).Parse(ing) + affin, err := NewParser(&resolver.Mock{}).Parse(ing) + if err != nil { + t.Errorf("unexpected error parsing annotations: %v", err) + } + nginxAffinity, ok := affin.(*Config) if !ok { t.Errorf("expected a Config type") diff --git a/internal/ingress/annotations/snippet/main_test.go b/internal/ingress/annotations/snippet/main_test.go index 921afeea8..b29b2950d 100644 --- a/internal/ingress/annotations/snippet/main_test.go +++ b/internal/ingress/annotations/snippet/main_test.go @@ -54,6 +54,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/annotations/streamsnippet/main_test.go b/internal/ingress/annotations/streamsnippet/main_test.go index 37c6b7ccd..e8a1ff8d0 100644 --- a/internal/ingress/annotations/streamsnippet/main_test.go +++ b/internal/ingress/annotations/streamsnippet/main_test.go @@ -57,6 +57,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/annotations/xforwardedprefix/main_test.go b/internal/ingress/annotations/xforwardedprefix/main_test.go index d873a4412..f28b6b10e 100644 --- a/internal/ingress/annotations/xforwardedprefix/main_test.go +++ b/internal/ingress/annotations/xforwardedprefix/main_test.go @@ -54,6 +54,7 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) + //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results result, _ := ap.Parse(ing) if result != testCase.expected { t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 19576bc77..0f8fc4ce6 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -189,13 +189,16 @@ func (n *NGINXController) syncIngress(interface{}) error { if !utilingress.IsDynamicConfigurationEnough(pcfg, n.runningConfig) { klog.InfoS("Configuration changes detected, backend reload required") - hash, _ := hashstructure.Hash(pcfg, hashstructure.FormatV1, &hashstructure.HashOptions{ + hash, err := hashstructure.Hash(pcfg, hashstructure.FormatV1, &hashstructure.HashOptions{ TagName: "json", }) + if err != nil { + klog.Errorf("unexpected error hashing configuration: %v", err) + } pcfg.ConfigurationChecksum = fmt.Sprintf("%v", hash) - err := n.OnUpdate(*pcfg) + err = n.OnUpdate(*pcfg) if err != nil { n.metricCollector.IncReloadErrorCount() n.metricCollector.ConfigSuccess(hash, false) diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 8ba8d913b..30f785586 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -687,7 +687,10 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { } if klog.V(2).Enabled() { - src, _ := os.ReadFile(cfgPath) + src, err := os.ReadFile(cfgPath) + if err != nil { + return err + } if !bytes.Equal(src, content) { tmpfile, err := os.CreateTemp("", "new-nginx-cfg") if err != nil { @@ -702,7 +705,10 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { diffOutput, err := exec.Command("diff", "-I", "'# Configuration.*'", "-u", cfgPath, tmpfile.Name()).CombinedOutput() if err != nil { if exitError, ok := err.(*exec.ExitError); ok { - ws := exitError.Sys().(syscall.WaitStatus) + ws, ok := exitError.Sys().(syscall.WaitStatus) + if !ok { + klog.Errorf("unexpected type: %T", exitError.Sys()) + } if ws.ExitStatus() == 2 { klog.Warningf("Failed to executing diff command: %v", err) } @@ -1129,7 +1135,10 @@ func cleanTempNginxCfg() error { return filepath.SkipDir } - dur, _ := time.ParseDuration("-5m") + dur, err := time.ParseDuration("-5m") + if err != nil { + return err + } fiveMinutesAgo := time.Now().Add(dur) if strings.HasPrefix(info.Name(), tempNginxPattern) && info.ModTime().Before(fiveMinutesAgo) { files = append(files, path) diff --git a/internal/ingress/controller/process/nginx.go b/internal/ingress/controller/process/nginx.go index 70227ac2e..fdf501616 100644 --- a/internal/ingress/controller/process/nginx.go +++ b/internal/ingress/controller/process/nginx.go @@ -30,7 +30,10 @@ func IsRespawnIfRequired(err error) bool { return false } - waitStatus := exitError.Sys().(syscall.WaitStatus) + waitStatus, ok := exitError.Sys().(syscall.WaitStatus) + if !ok { + return false + } klog.Warningf(` ------------------------------------------------------------------------------- NGINX master process died (%v): %v diff --git a/internal/ingress/controller/status.go b/internal/ingress/controller/status.go index 3a277ca5e..e061b2cb7 100644 --- a/internal/ingress/controller/status.go +++ b/internal/ingress/controller/status.go @@ -86,8 +86,10 @@ func setupLeaderElection(config *leaderElectionConfig) { } broadcaster := record.NewBroadcaster() - hostname, _ := os.Hostname() - + hostname, err := os.Hostname() + if err != nil { + klog.Errorf("unexpected error getting hostname: %v", err) + } recorder := broadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{ Component: "ingress-leader-elector", Host: hostname, @@ -107,7 +109,7 @@ func setupLeaderElection(config *leaderElectionConfig) { ttl := 30 * time.Second - elector, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ + elector, err = leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{ Lock: lock, LeaseDuration: ttl, RenewDeadline: ttl / 2, diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 69617539f..918dfd41a 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -522,7 +522,10 @@ func New( ingressClassEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - ingressclass := obj.(*networkingv1.IngressClass) + ingressclass, ok := obj.(*networkingv1.IngressClass) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } foundClassByName := false if icConfig.IngressClassByName && ingressclass.Name == icConfig.AnnotationValue { klog.InfoS("adding ingressclass as ingress-class-by-name is configured", "ingressclass", klog.KObj(ingressclass)) @@ -544,7 +547,10 @@ func New( } }, DeleteFunc: func(obj interface{}) { - ingressclass := obj.(*networkingv1.IngressClass) + ingressclass, ok := obj.(*networkingv1.IngressClass) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } if ingressclass.Spec.Controller != icConfig.Controller { klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(ingressclass)) return @@ -560,8 +566,14 @@ func New( } }, UpdateFunc: func(old, cur interface{}) { - oic := old.(*networkingv1.IngressClass) - cic := cur.(*networkingv1.IngressClass) + oic, ok := old.(*networkingv1.IngressClass) + if !ok { + klog.Errorf("unexpected type: %T", old) + } + cic, ok := cur.(*networkingv1.IngressClass) + if !ok { + klog.Errorf("unexpected type: %T", cur) + } if cic.Spec.Controller != icConfig.Controller { klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(cic)) return @@ -584,7 +596,10 @@ func New( secrEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - sec := obj.(*corev1.Secret) + sec, ok := obj.(*corev1.Secret) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } key := k8s.MetaNamespaceKey(sec) if store.defaultSSLCertificate == key { @@ -611,7 +626,10 @@ func New( }, UpdateFunc: func(old, cur interface{}) { if !reflect.DeepEqual(old, cur) { - sec := cur.(*corev1.Secret) + sec, ok := cur.(*corev1.Secret) + if !ok { + klog.Errorf("unexpected type: %T", cur) + } key := k8s.MetaNamespaceKey(sec) if !watchedNamespace(sec.Namespace) { @@ -698,8 +716,14 @@ func New( } }, UpdateFunc: func(old, cur interface{}) { - oeps := old.(*discoveryv1.EndpointSlice) - ceps := cur.(*discoveryv1.EndpointSlice) + oeps, ok := old.(*discoveryv1.EndpointSlice) + if !ok { + klog.Errorf("unexpected type: %T", old) + } + ceps, ok := cur.(*discoveryv1.EndpointSlice) + if !ok { + klog.Errorf("unexpected type: %T", cur) + } if !reflect.DeepEqual(ceps.Endpoints, oeps.Endpoints) { updateCh.In() <- Event{ Type: UpdateEvent, @@ -753,7 +777,10 @@ func New( cmEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - cfgMap := obj.(*corev1.ConfigMap) + cfgMap, ok := obj.(*corev1.ConfigMap) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } key := k8s.MetaNamespaceKey(cfgMap) handleCfgMapEvent(key, cfgMap, "CREATE") }, @@ -762,7 +789,10 @@ func New( return } - cfgMap := cur.(*corev1.ConfigMap) + cfgMap, ok := cur.(*corev1.ConfigMap) + if !ok { + klog.Errorf("unexpected type: %T", cur) + } key := k8s.MetaNamespaceKey(cfgMap) handleCfgMapEvent(key, cfgMap, "UPDATE") }, @@ -770,7 +800,10 @@ func New( serviceHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - svc := obj.(*corev1.Service) + svc, ok := obj.(*corev1.Service) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } if svc.Spec.Type == corev1.ServiceTypeExternalName { updateCh.In() <- Event{ Type: CreateEvent, @@ -779,7 +812,10 @@ func New( } }, DeleteFunc: func(obj interface{}) { - svc := obj.(*corev1.Service) + svc, ok := obj.(*corev1.Service) + if !ok { + klog.Errorf("unexpected type: %T", obj) + } if svc.Spec.Type == corev1.ServiceTypeExternalName { updateCh.In() <- Event{ Type: DeleteEvent, @@ -788,8 +824,14 @@ func New( } }, UpdateFunc: func(old, cur interface{}) { - oldSvc := old.(*corev1.Service) - curSvc := cur.(*corev1.Service) + oldSvc, ok := old.(*corev1.Service) + if !ok { + klog.Errorf("unexpected type: %T", old) + } + curSvc, ok := cur.(*corev1.Service) + if !ok { + klog.Errorf("unexpected type: %T", cur) + } if reflect.DeepEqual(oldSvc, curSvc) { return @@ -824,7 +866,10 @@ func New( } // do not wait for informers to read the configmap configuration - ns, name, _ := k8s.ParseNameNS(configmap) + ns, name, err := k8s.ParseNameNS(configmap) + if err != nil { + klog.Errorf("unexpected error parsing name and ns: %v", err) + } cm, err := client.CoreV1().ConfigMaps(ns).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { klog.Warningf("Unexpected error reading configuration configmap: %v", err) @@ -1058,7 +1103,10 @@ func (s *k8sStore) ListIngresses() []*ingress.Ingress { // filter ingress rules ingresses := make([]*ingress.Ingress, 0) for _, item := range s.listers.IngressWithAnnotation.List() { - ing := item.(*ingress.Ingress) + ing, ok := item.(*ingress.Ingress) + if !ok { + klog.Errorf("unexpected type: %T", item) + } ingresses = append(ingresses, ing) } diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 6f6d9e118..317c0f36c 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -91,7 +91,10 @@ func TestStore(t *testing.T) { t.Fatalf("error: %v", err) } - emptySelector, _ := labels.Parse("") + emptySelector, err := labels.Parse("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } defer te.Stop() //nolint:errcheck // Ignore the error @@ -177,7 +180,11 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } + if e.Obj == nil { continue } @@ -282,7 +289,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -393,7 +403,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -524,7 +537,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -631,7 +647,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -726,7 +745,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -817,7 +839,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -909,7 +934,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -1032,7 +1060,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -1159,7 +1190,10 @@ func TestStore(t *testing.T) { return } - e := evt.(Event) + e, ok := evt.(Event) + if !ok { + return + } if e.Obj == nil { continue } @@ -1174,7 +1208,10 @@ func TestStore(t *testing.T) { } }(updateCh) - namesapceSelector, _ := labels.Parse("foo=bar") + namesapceSelector, err := labels.Parse("foo=bar") + if err != nil { + t.Errorf("unexpected error: %v", err) + } storer := New( ns, namesapceSelector, diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 2c65564af..9dc019bcc 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -263,7 +263,10 @@ func ReadConfig(src map[string]string) config.Configuration { if val, ok := conf[globalAuthSignin]; ok { delete(conf, globalAuthSignin) - signinURL, _ := parser.StringToURL(val) + signinURL, err := parser.StringToURL(val) + if err != nil { + klog.Errorf("string to URL conversion failed: %v", err) + } if signinURL == nil { klog.Warningf("Global auth location denied - %v.", "global-auth-signin setting is undefined and will not be set") } else { @@ -276,7 +279,10 @@ func ReadConfig(src map[string]string) config.Configuration { delete(conf, globalAuthSigninRedirectParam) redirectParam := strings.TrimSpace(val) - dummySigninURL, _ := parser.StringToURL(fmt.Sprintf("%s?%s=dummy", to.GlobalExternalAuth.SigninURL, redirectParam)) + dummySigninURL, err := parser.StringToURL(fmt.Sprintf("%s?%s=dummy", to.GlobalExternalAuth.SigninURL, redirectParam)) + if err != nil { + klog.Errorf("string to URL conversion failed: %v", err) + } if dummySigninURL == nil { klog.Warningf("Global auth redirect parameter denied - %v.", "global-auth-signin-redirect-param setting is invalid and will not be set") } else { @@ -477,7 +483,10 @@ func dictStrToKb(sizeStr string) int { if sizeMatch == nil { return -1 } - size, _ := strconv.Atoi(sizeMatch[1]) // validated already with regex + size, err := strconv.Atoi(sizeMatch[1]) // validated already with regex + if err != nil { + klog.Errorf("unexpected error converting size string %s to int: %v", sizeStr, err) + } if sizeMatch[2] == "" || strings.EqualFold(sizeMatch[2], "m") { size *= 1024 } diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index f4bf4b0d8..ba685efd0 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -978,7 +978,11 @@ func buildNextUpstream(i, r interface{}) string { return "" } - retryNonIdempotent := r.(bool) + retryNonIdempotent, ok := r.(bool) + if !ok { + klog.Errorf("expected a 'bool' type but %T was returned", i) + return "" + } parts := strings.Split(nextUpstream, " ") @@ -1162,7 +1166,11 @@ func buildForwardedFor(input interface{}) string { } func buildAuthSignURL(authSignURL, authRedirectParam string) string { - u, _ := url.Parse(authSignURL) + u, err := url.Parse(authSignURL) + if err != nil { + klog.Errorf("error parsing authSignURL: %v", err) + return "" + } q := u.Query() if authRedirectParam == "" { authRedirectParam = defaultGlobalAuthRedirectParam diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index c82f7fb76..110967711 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -701,7 +701,10 @@ func TestChangeHostPort(t *testing.T) { } func TestTemplateWithData(t *testing.T) { - pwd, _ := os.Getwd() + pwd, err := os.Getwd() + if err != nil { + t.Errorf("unexpected error: %v", err) + } f, err := os.Open(path.Join(pwd, "../../../../test/data/config.json")) if err != nil { t.Errorf("unexpected error reading json file: %v", err) @@ -745,7 +748,10 @@ func TestTemplateWithData(t *testing.T) { } func BenchmarkTemplateWithData(b *testing.B) { - pwd, _ := os.Getwd() + pwd, err := os.Getwd() + if err != nil { + b.Errorf("unexpected error: %v", err) + } f, err := os.Open(path.Join(pwd, "../../../../test/data/config.json")) if err != nil { b.Errorf("unexpected error reading json file: %v", err) diff --git a/internal/ingress/metric/collectors/controller_test.go b/internal/ingress/metric/collectors/controller_test.go index 147a37ad2..15735df42 100644 --- a/internal/ingress/metric/collectors/controller_test.go +++ b/internal/ingress/metric/collectors/controller_test.go @@ -76,9 +76,12 @@ func TestControllerCounters(t *testing.T) { { name: "should set SSL certificates metrics", test: func(cm *Controller) { - t1, _ := time.Parse( + t1, err := time.Parse( time.RFC3339, "2012-11-01T22:08:41+00:00") + if err != nil { + t.Errorf("unexpected error: %v", err) + } servers := []*ingress.Server{ { @@ -228,9 +231,12 @@ func TestRemoveMetrics(t *testing.T) { t.Errorf("registering collector failed: %s", err) } - t1, _ := time.Parse( + t1, err := time.Parse( time.RFC3339, "2012-11-01T22:08:41+00:00") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } servers := []*ingress.Server{ { @@ -275,10 +281,12 @@ func TestRemoveAllSSLMetrics(t *testing.T) { t.Errorf("registering collector failed: %s", err) } - t1, _ := time.Parse( + t1, err := time.Parse( time.RFC3339, "2012-11-01T22:08:41+00:00") - + if err != nil { + t.Errorf("unexpected error: %v", err) + } servers := []*ingress.Server{ { Hostname: "demo", diff --git a/internal/ingress/metric/collectors/process_test.go b/internal/ingress/metric/collectors/process_test.go index 7de27de20..588cbafef 100644 --- a/internal/ingress/metric/collectors/process_test.go +++ b/internal/ingress/metric/collectors/process_test.go @@ -49,7 +49,10 @@ func TestProcessCollector(t *testing.T) { done := make(chan struct{}) go func() { cmd.Wait() //nolint:errcheck // Ignore the error - status := cmd.ProcessState.Sys().(syscall.WaitStatus) + status, ok := cmd.ProcessState.Sys().(syscall.WaitStatus) + if !ok { + t.Errorf("unexpected type: %T", cmd.ProcessState.Sys()) + } if status.Signaled() { t.Logf("Signal: %v", status.Signal()) } else { diff --git a/internal/ingress/metric/collectors/socket.go b/internal/ingress/metric/collectors/socket.go index 3ec55e9da..a70024c57 100644 --- a/internal/ingress/metric/collectors/socket.go +++ b/internal/ingress/metric/collectors/socket.go @@ -107,6 +107,7 @@ var defObjectives = map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001} func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStatusClasses bool, buckets HistogramBuckets, excludeMetrics []string) (*SocketCollector, error) { socket := "/tmp/nginx/prometheus-nginx.socket" // unix sockets must be unlink()ed before being used + //nolint:errcheck // Ignore unlink error _ = syscall.Unlink(socket) listener, err := net.Listen("unix", socket) diff --git a/internal/ingress/metric/collectors/socket_test.go b/internal/ingress/metric/collectors/socket_test.go index 486d79e69..71e9097c9 100644 --- a/internal/ingress/metric/collectors/socket_test.go +++ b/internal/ingress/metric/collectors/socket_test.go @@ -58,7 +58,10 @@ func TestNewUDPLogListener(t *testing.T) { } }() - conn, _ := net.Dial("unix", tmpFile) + conn, err := net.Dial("unix", tmpFile) + if err != nil { + t.Errorf("unexpected error connecting to unix socket: %v", err) + } if _, err := conn.Write([]byte("message")); err != nil { t.Errorf("unexpected error writing to unix socket: %v", err) } diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index 074543878..4d16a0df5 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -341,7 +341,10 @@ func ingressSliceEqual(lhs, rhs []v1.IngressLoadBalancerIngress) bool { } func statusAddressFromService(service string, kubeClient clientset.Interface) ([]v1.IngressLoadBalancerIngress, error) { - ns, name, _ := k8s.ParseNameNS(service) + ns, name, err := k8s.ParseNameNS(service) + if err != nil { + return nil, err + } svc, err := kubeClient.CoreV1().Services(ns).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { return nil, err diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index 9733bd62d..01419708b 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -309,7 +309,10 @@ func TestStatusActions(t *testing.T) { t.Fatalf("expected a valid Sync") } - fk := fkSync.(*statusSync) + fk, ok := fkSync.(*statusSync) + if !ok { + t.Errorf("unexpected type: %T", fkSync) + } // start it and wait for the election and syn actions stopCh := make(chan struct{}) @@ -586,7 +589,11 @@ func TestRunningAddressesWithPods(t *testing.T) { fk := buildStatusSync() fk.PublishService = "" - r, _ := fk.runningAddresses() + r, err := fk.runningAddresses() + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if r == nil { t.Fatalf("returned nil but expected valid []networking.IngressLoadBalancerIngress") } @@ -604,7 +611,10 @@ func TestRunningAddressesWithPublishStatusAddress(t *testing.T) { fk := buildStatusSync() fk.PublishStatusAddress = localhost - ra, _ := fk.runningAddresses() + ra, err := fk.runningAddresses() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if ra == nil { t.Fatalf("returned nil but expected valid []networking.IngressLoadBalancerIngress") } @@ -622,7 +632,10 @@ func TestRunningAddressesWithPublishStatusAddresses(t *testing.T) { fk := buildStatusSync() fk.PublishStatusAddress = "127.0.0.1,1.1.1.1" - ra, _ := fk.runningAddresses() + ra, err := fk.runningAddresses() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if ra == nil { t.Fatalf("returned nil but expected valid []networking.IngressLoadBalancerIngress") } @@ -644,7 +657,10 @@ func TestRunningAddressesWithPublishStatusAddressesAndSpaces(t *testing.T) { fk := buildStatusSync() fk.PublishStatusAddress = "127.0.0.1, 1.1.1.1" - ra, _ := fk.runningAddresses() + ra, err := fk.runningAddresses() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if ra == nil { t.Fatalf("returned nil but expected valid []networking.IngressLoadBalancerIngresst") } diff --git a/internal/k8s/main.go b/internal/k8s/main.go index 033244b9d..5e93e560d 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -148,7 +148,10 @@ const IngressNGINXController = "k8s.io/ingress-nginx" // NetworkingIngressAvailable checks if the package "k8s.io/api/networking/v1" // is available or not and if Ingress V1 is supported (k8s >= v1.19.0) func NetworkingIngressAvailable(client clientset.Interface) bool { - version119, _ := version.ParseGeneric("v1.19.0") + version119, err := version.ParseGeneric("v1.19.0") + if err != nil { + return false + } serverVersion, err := client.Discovery().ServerVersion() if err != nil { diff --git a/internal/net/ipnet_test.go b/internal/net/ipnet_test.go index 95e6b9c32..8e460aae9 100644 --- a/internal/net/ipnet_test.go +++ b/internal/net/ipnet_test.go @@ -36,13 +36,17 @@ func TestNewIPSet(t *testing.T) { } func TestParseCIDRs(t *testing.T) { - cidr, _ := ParseCIDRs("invalid.com") + cidr, err := ParseCIDRs("invalid.com") + if err == nil { + t.Errorf("expected error but got nil") + } + if cidr != nil { t.Errorf("expected %v but got %v", nil, cidr) } expected := []string{"192.0.0.1", "192.0.1.0/24"} - cidr, err := ParseCIDRs("192.0.0.1, 192.0.1.0/24") + cidr, err = ParseCIDRs("192.0.0.1, 192.0.1.0/24") if err != nil { t.Errorf("unexpected error %v", err) } diff --git a/internal/net/net.go b/internal/net/net.go index 5a394a7b5..b0952d9cc 100644 --- a/internal/net/net.go +++ b/internal/net/net.go @@ -52,7 +52,10 @@ func IsIPv6Enabled() bool { } for _, addr := range addrs { - ip, _, _ := _net.ParseCIDR(addr.String()) + ip, _, err := _net.ParseCIDR(addr.String()) + if err != nil { + return false + } if IsIPV6(ip) { return true } diff --git a/internal/net/ssl/ssl.go b/internal/net/ssl/ssl.go index 01b8b9408..c6c519d51 100644 --- a/internal/net/ssl/ssl.go +++ b/internal/net/ssl/ssl.go @@ -503,9 +503,14 @@ func NewTLSListener(certificate, key string) *TLSListener { l.load() - _, _ = file.NewFileWatcher(certificate, l.load) - _, _ = file.NewFileWatcher(key, l.load) - + _, err := file.NewFileWatcher(certificate, l.load) + if err != nil { + klog.Errorf("unexpected error: %v", err) + } + _, err = file.NewFileWatcher(key, l.load) + if err != nil { + klog.Errorf("unexpected error: %v", err) + } return &l } diff --git a/internal/nginx/main.go b/internal/nginx/main.go index 9a4439d16..fc586e9e8 100644 --- a/internal/nginx/main.go +++ b/internal/nginx/main.go @@ -163,7 +163,10 @@ func Version() string { // IsRunning returns true if a process with the name 'nginx' is found func IsRunning() bool { - processes, _ := ps.Processes() + processes, err := ps.Processes() + if err != nil { + klog.ErrorS(err, "unexpected error obtaining process list") + } for _, p := range processes { if p.Executable() == "nginx" { return true diff --git a/internal/nginx/maxmind.go b/internal/nginx/maxmind.go index 67a53c50f..8e601c731 100644 --- a/internal/nginx/maxmind.go +++ b/internal/nginx/maxmind.go @@ -101,7 +101,7 @@ func DownloadGeoLite2DB(attempts int, period time.Duration) error { var lastErr error retries := 0 - _ = wait.ExponentialBackoff(defaultRetry, func() (bool, error) { + lastErr = wait.ExponentialBackoff(defaultRetry, func() (bool, error) { var dlError error for _, dbName := range strings.Split(MaxmindEditionIDs, ",") { dlError = downloadDatabase(dbName) diff --git a/internal/task/queue.go b/internal/task/queue.go index 4bc9a40a7..f92f2a501 100644 --- a/internal/task/queue.go +++ b/internal/task/queue.go @@ -115,7 +115,10 @@ func (t *Queue) worker() { } ts := time.Now().UnixNano() - item := key.(Element) + item, ok := key.(Element) + if !ok { + klog.ErrorS(nil, "invalid item type", "key", key) + } if item.Timestamp != 0 && t.lastSync > item.Timestamp { klog.V(3).InfoS("skipping sync", "key", item.Key, "last", t.lastSync, "now", item.Timestamp) t.queue.Forget(key) diff --git a/pkg/apis/ingress/types_equals_test.go b/pkg/apis/ingress/types_equals_test.go index 0893162d3..53643f912 100644 --- a/pkg/apis/ingress/types_equals_test.go +++ b/pkg/apis/ingress/types_equals_test.go @@ -25,19 +25,29 @@ import ( ) func TestEqualConfiguration(t *testing.T) { - ap, _ := filepath.Abs("../../../test/manifests/configuration-a.json") + ap, err := filepath.Abs("../../../test/manifests/configuration-a.json") + if err != nil { + t.Errorf("unexpected error: %v", err) + } a, err := readJSON(ap) if err != nil { t.Errorf("unexpected error reading JSON file: %v", err) } - bp, _ := filepath.Abs("../../../test/manifests/configuration-b.json") + bp, err := filepath.Abs("../../../test/manifests/configuration-b.json") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + b, err := readJSON(bp) if err != nil { t.Errorf("unexpected error reading JSON file: %v", err) } - cp, _ := filepath.Abs("../../../test/manifests/configuration-c.json") + cp, err := filepath.Abs("../../../test/manifests/configuration-c.json") + if err != nil { + t.Errorf("unexpected error: %v", err) + } c, err := readJSON(cp) if err != nil { t.Errorf("unexpected error reading JSON file: %v", err) diff --git a/pkg/tcpproxy/tcp.go b/pkg/tcpproxy/tcp.go index 7fe7296e5..eda4a2746 100644 --- a/pkg/tcpproxy/tcp.go +++ b/pkg/tcpproxy/tcp.go @@ -91,8 +91,14 @@ func (p *TCPProxy) Handle(conn net.Conn) { if proxy.ProxyProtocol { // write out the Proxy Protocol header - localAddr := conn.LocalAddr().(*net.TCPAddr) - remoteAddr := conn.RemoteAddr().(*net.TCPAddr) + localAddr, ok := conn.LocalAddr().(*net.TCPAddr) + if !ok { + klog.Errorf("unexpected type: %T", conn.LocalAddr()) + } + remoteAddr, ok := conn.RemoteAddr().(*net.TCPAddr) + if !ok { + klog.Errorf("unexpected type: %T", conn.RemoteAddr()) + } protocol := "UNKNOWN" if remoteAddr.IP.To4() != nil { protocol = "TCP4" diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index f87db27a2..b64581ef6 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -219,7 +219,8 @@ var _ = framework.DescribeAnnotation("affinity session-cookie-name", func() { assert.Nil(ginkgo.GinkgoT(), err, "loading GMT location") assert.NotNil(ginkgo.GinkgoT(), local, "expected a location but none returned") - duration, _ := time.ParseDuration("48h") + duration, err := time.ParseDuration("48h") + assert.Nil(ginkgo.GinkgoT(), err, "parsing duration") expected := time.Now().In(local).Add(duration).Format("Mon, 02-Jan-06 15:04") f.HTTPTestClient(). @@ -478,12 +479,13 @@ var _ = framework.DescribeAnnotation("affinity session-cookie-name", func() { strings.Contains(server, "listen 443") }) - f.HTTPTestClientWithTLSConfig(&tls.Config{ServerName: host, InsecureSkipVerify: true}). //nolint:gosec // Ignore the gosec error in testing - GET("/"). - WithURL(f.GetURL(framework.HTTPS)). - WithHeader("Host", host). - Expect(). - Status(http.StatusOK). - Header("Set-Cookie").Contains("; Secure") + //nolint:gosec // Ignore the gosec error in testing + f.HTTPTestClientWithTLSConfig(&tls.Config{ServerName: host, InsecureSkipVerify: true}). + GET("/"). + WithURL(f.GetURL(framework.HTTPS)). + WithHeader("Host", host). + Expect(). + Status(http.StatusOK). + Header("Set-Cookie").Contains("; Secure") }) }) diff --git a/test/e2e/annotations/grpc.go b/test/e2e/annotations/grpc.go index 910768696..4c121df49 100644 --- a/test/e2e/annotations/grpc.go +++ b/test/e2e/annotations/grpc.go @@ -103,7 +103,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { return strings.Contains(server, "grpc_pass grpc://upstream_balancer;") }) - conn, _ := grpc.Dial(f.GetNginxIP()+":443", + conn, err := grpc.Dial(f.GetNginxIP()+":443", grpc.WithTransportCredentials( credentials.NewTLS(&tls.Config{ ServerName: echoHost, @@ -111,6 +111,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { }), ), ) + assert.Nil(ginkgo.GinkgoT(), err, "error creating a connection") defer conn.Close() client := pb.NewGRPCBinClient(conn) @@ -163,7 +164,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { return strings.Contains(server, "grpc_pass grpc://upstream_balancer;") }) - conn, _ := grpc.Dial(f.GetNginxIP()+":443", + conn, err := grpc.Dial(f.GetNginxIP()+":443", grpc.WithTransportCredentials( credentials.NewTLS(&tls.Config{ ServerName: echoHost, @@ -171,6 +172,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { }), ), ) + assert.Nil(ginkgo.GinkgoT(), err) defer conn.Close() client := pb.NewGRPCBinClient(conn) @@ -227,7 +229,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { return strings.Contains(server, "grpc_pass grpcs://upstream_balancer;") }) - conn, _ := grpc.Dial(f.GetNginxIP()+":443", + conn, err := grpc.Dial(f.GetNginxIP()+":443", grpc.WithTransportCredentials( credentials.NewTLS(&tls.Config{ ServerName: echoHost, @@ -235,6 +237,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { }), ), ) + assert.Nil(ginkgo.GinkgoT(), err) defer conn.Close() client := pb.NewGRPCBinClient(conn) diff --git a/test/e2e/annotations/upstreamhashby.go b/test/e2e/annotations/upstreamhashby.go index f7316ceee..c9f71af18 100644 --- a/test/e2e/annotations/upstreamhashby.go +++ b/test/e2e/annotations/upstreamhashby.go @@ -55,7 +55,9 @@ func startIngress(f *framework.Framework, annotations map[string]string) map[str assert.Nil(ginkgo.GinkgoT(), err) - re, _ := regexp.Compile(fmt.Sprintf(`Hostname: %v.*`, framework.EchoService)) + re, err := regexp.Compile(fmt.Sprintf(`Hostname: %v.*`, framework.EchoService)) + assert.Nil(ginkgo.GinkgoT(), err, "error compiling regex") + podMap := map[string]bool{} for i := 0; i < 100; i++ { diff --git a/test/e2e/framework/k8s.go b/test/e2e/framework/k8s.go index 673e0879f..cb1a28d9e 100644 --- a/test/e2e/framework/k8s.go +++ b/test/e2e/framework/k8s.go @@ -152,7 +152,11 @@ func waitForPodsReady(kubeClientSet kubernetes.Interface, timeout time.Duration, r := 0 for i := range pl.Items { - if isRunning, _ := podRunningReady(&pl.Items[i]); isRunning { + isRunning, err := podRunningReady(&pl.Items[i]) + if err != nil { + return false, err + } + if isRunning { r++ } } diff --git a/test/e2e/loadbalance/ewma.go b/test/e2e/loadbalance/ewma.go index 0f455f405..f457e6357 100644 --- a/test/e2e/loadbalance/ewma.go +++ b/test/e2e/loadbalance/ewma.go @@ -53,7 +53,9 @@ var _ = framework.DescribeSetting("[Load Balancer] EWMA", func() { assert.Nil(ginkgo.GinkgoT(), err) assert.Equal(ginkgo.GinkgoT(), algorithm, "ewma") - re, _ := regexp.Compile(fmt.Sprintf(`%v.*`, framework.EchoService)) + re, err := regexp.Compile(fmt.Sprintf(`%v.*`, framework.EchoService)) + assert.Nil(ginkgo.GinkgoT(), err, "error compiling regex") + replicaRequestCount := map[string]int{} for i := 0; i < 30; i++ { diff --git a/test/e2e/loadbalance/round_robin.go b/test/e2e/loadbalance/round_robin.go index 1b40e63d0..5f6667143 100644 --- a/test/e2e/loadbalance/round_robin.go +++ b/test/e2e/loadbalance/round_robin.go @@ -45,7 +45,9 @@ var _ = framework.DescribeSetting("[Load Balancer] round-robin", func() { return strings.Contains(server, "server_name load-balance.com") }) - re, _ := regexp.Compile(fmt.Sprintf(`%v.*`, framework.EchoService)) + re, err := regexp.Compile(fmt.Sprintf(`%v.*`, framework.EchoService)) + assert.Nil(ginkgo.GinkgoT(), err, "error compiling regex") + replicaRequestCount := map[string]int{} for i := 0; i < 600; i++ { diff --git a/test/e2e/settings/keep-alive.go b/test/e2e/settings/keep-alive.go index ca89e7db2..167f5ac18 100644 --- a/test/e2e/settings/keep-alive.go +++ b/test/e2e/settings/keep-alive.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/assert" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -58,7 +59,8 @@ var _ = framework.DescribeSetting("keep-alive keep-alive-requests", func() { f.UpdateNginxConfigMapData("upstream-keepalive-connections", "128") f.WaitForNginxConfiguration(func(server string) bool { - match, _ := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive 128;`, server) + match, err := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive 128;`, server) + assert.Nil(ginkgo.GinkgoT(), err, "unexpected error matching the upstream keepalive time") return match }) }) @@ -67,7 +69,8 @@ var _ = framework.DescribeSetting("keep-alive keep-alive-requests", func() { f.UpdateNginxConfigMapData("upstream-keepalive-timeout", "120") f.WaitForNginxConfiguration(func(server string) bool { - match, _ := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_timeout\s*120s;`, server) + match, err := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_timeout\s*120s;`, server) + assert.Nil(ginkgo.GinkgoT(), err, "unexpected error matching the upstream keepalive time") return match }) }) @@ -76,7 +79,8 @@ var _ = framework.DescribeSetting("keep-alive keep-alive-requests", func() { f.UpdateNginxConfigMapData("upstream-keepalive-time", "75s") f.WaitForNginxConfiguration(func(server string) bool { - match, _ := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_time\s*75s;`, server) + match, err := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_time\s*75s;`, server) + assert.Nil(ginkgo.GinkgoT(), err, "unexpected error matching the upstream keepalive time") return match }) }) @@ -85,7 +89,8 @@ var _ = framework.DescribeSetting("keep-alive keep-alive-requests", func() { f.UpdateNginxConfigMapData("upstream-keepalive-requests", "200") f.WaitForNginxConfiguration(func(server string) bool { - match, _ := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_requests\s*200;`, server) + match, err := regexp.MatchString(`upstream\supstream_balancer\s\{[\s\S]*keepalive_requests\s*200;`, server) + assert.Nil(ginkgo.GinkgoT(), err, "unexpected error matching the upstream keepalive time") return match }) }) diff --git a/test/e2e/status/update.go b/test/e2e/status/update.go index 046752d2b..35b2b63cf 100644 --- a/test/e2e/status/update.go +++ b/test/e2e/status/update.go @@ -134,7 +134,8 @@ func getHostIP() net.IP { } defer conn.Close() - localAddr := conn.LocalAddr().(*net.UDPAddr) + localAddr, ok := conn.LocalAddr().(*net.UDPAddr) + assert.False(ginkgo.GinkgoT(), ok, "unexpected type: %T", conn.LocalAddr()) return localAddr.IP }