Fix linting errors (#983)

* Fix linting errors: ```sh $ golangci-lint run ./... WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the golangci/golangci-lint#2649. pkg/spec/spec.go:30:2: SA1019: package io/ioutil is deprecated: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck) "io/ioutil" ^ pkg/spec/extension.go:139:5: SA4006: this value of `item` is never used (staticcheck) item := &openapi3.PathItem{} ^ cmd/manager/main.go:366:11: Error return value of `c.AddFunc` is not checked (errcheck) c.AddFunc("@daily", func() { ^ internal/envoy/config/hcm_test.go:34:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead. (staticcheck) "github.com/golang/protobuf/proto" ^ smoketests/basic/basic_test.go:20:2: `defaultName` is unused (deadcode) defaultName = "default" ^ cmd/kusk/internal/mocking/server/server.go:61:9: Error return value of `io.Copy` is not checked (errcheck) io.Copy(io.Discard, reader) ^ cmd/kusk/internal/overlays/overlays.go:138:9: Error return value of `io.Copy` is not checked (errcheck) io.Copy(io.Discard, reader) ^ smoketests/common/commonSuite.go:36:20: Error return value is not checked (errcheck) kuskv1.AddToScheme(scheme) ^ smoketests/common/commonSuite.go:37:20: Error return value is not checked (errcheck) corev1.AddToScheme(scheme) ^ smoketests/common/commonSuite.go:38:18: Error return value is not checked (errcheck) apps.AddToScheme(scheme) ^ api/v1alpha1/webhook_suite_test.go:55:5: `cfg` is unused (deadcode) var cfg *rest.Config ^ api/v1alpha1/envoyfleet_webhook.go:49:2: SA9004: only the first constant in this group has an explicit type (staticcheck) EnvoyFleetMutatingWebhookPath string = "/mutate-gateway-kusk-io-v1alpha1-envoyfleet" ^ internal/envoy/auth/parser.go:95:3: S1021: should merge variable declaration with assignment on next line (gosimple) var err error ^ smoketests/basic_auth/basic_auth_test.go:47:2: `testPort` is unused (deadcode) testPort = 82 ^ smoketests/mocking/mocking_test.go:26:2: `testPort` is unused (deadcode) testPort = 82 ^ internal/validation/const.go:41:2: `reAdjustSubstitutions` is unused (deadcode) reAdjustSubstitutions = regexp.MustCompile(`\\(\d+)`) ^ internal/validation/extension.go:9:2: SA1019: package io/ioutil is deprecated: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck) "io/ioutil" ^ internal/webhooks/certs.go:244:31: S1019: should use make([]string, count) instead (gosimple) operations := make([]string, count, count) ^ cmd/kusk/cmd/root.go:94:2: `cmdGroupCommands` is unused (deadcode) cmdGroupCommands = "2-Commands" ^ cmd/kusk/cmd/version_test.go:34:6: `test_NewVersionCommand` is unused (deadcode) func test_NewVersionCommand(t *testing.T) { ^ cmd/kusk/cmd/add_gateway.go:244:8: Error return value of `c.List` is not checked (errcheck) c.List(context.Background(), &services, &client.ListOptions{}) ^ cmd/kusk/cmd/dashboard.go:159:18: Error return value of `process.Execute` is not checked (errcheck) process.Execute(browserOpenCMD, browserOpenArgs...) ^ cmd/kusk/cmd/install.go:329:15: Error return value of `os.MkdirAll` is not checked (errcheck) os.MkdirAll(filePath, os.ModePerm) ^ cmd/kusk/cmd/mock.go:459:26: Error return value of `mockCmd.MarkFlagRequired` is not checked (errcheck) mockCmd.MarkFlagRequired("in") ^ cmd/kusk/cmd/root.go:63:33: Error return value of `analytics.SendAnonymousCMDInfo` is not checked (errcheck) analytics.SendAnonymousCMDInfo(nil) ^ cmd/kusk/cmd/validate.go:63:30: Error return value of `validateCmd.MarkFlagRequired` is not checked (errcheck) validateCmd.MarkFlagRequired("file") ^ cmd/kusk/cmd/root.go:125:9: S1005: unnecessary assignment to the blank identifier (gosimple) for k, _ := range groups { ^ cmd/kusk/cmd/deploy.go:208:2: SA4006: this value of `parsedApiSpec` is never used (staticcheck) parsedApiSpec := &openapi3.T{} ^ cmd/kusk/cmd/mock.go:117:4: SA4006: this value of `err` is never used (staticcheck) popFunc, err := pushDirectory(filepath.Dir(absoluteApiSpecPath)) ^ internal/controllers/envoyfleet_resources.go:43:2: `kuskGatewayManagerImageName` is unused (deadcode) kuskGatewayManagerImageName = "kusk-gateway" ^ internal/controllers/parser.go:742:6: `generateMockID` is unused (deadcode) func generateMockID(path string, method string, operationID string) string { ^ internal/controllers/suite_test.go:49:5: `cfg` is unused (deadcode) var cfg *rest.Config ^ internal/controllers/parser.go:424:126: S1002: should omit comparison to bool constant, can be simplified to `!*finalOpts.Validation.Request.Enabled` (gosimple) if finalOpts.Validation == nil || finalOpts.Validation.Request == nil || finalOpts.Validation.Request.Enabled == nil || *finalOpts.Validation.Request.Enabled == false { ``` --- Signed-off-by: Mohamed Bana <[email protected]> * address PR comments and fix linting issues that appeared after merging main * address PR comments and fix linting issues that appeared after merging main * attempt to fix test version command test * add golangci lint pipeline * add golangci lint pipeline * add timeout to golangci lint pipeline * remove commented out action params Signed-off-by: Mohamed Bana <[email protected]> Co-authored-by: Kyle Hodgetts <[email protected]>
kubeshop · Nov 29, 2022 · 644f876 · 644f876
1 parent 7eb2f70
commit 644f876
Show file tree

Hide file tree

Showing 34 changed files with 143 additions and 110 deletions.
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -0,0 +1,28 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+      - main
+  pull_request:
+permissions:
+  contents: read
+  # allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.18
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
+          args: --timeout=10m0s
+          only-new-issues: true
diff --git a/api/v1alpha1/envoyfleet_webhook.go b/api/v1alpha1/envoyfleet_webhook.go
@@ -46,8 +46,8 @@ var (
 )
 
 const (
-	EnvoyFleetMutatingWebhookPath   string = "/mutate-gateway-kusk-io-v1alpha1-envoyfleet"
-	EnvoyFleetValidatingWebhookPath        = "/validate-gateway-kusk-io-v1alpha1-envoyfleet"
+	EnvoyFleetMutatingWebhookPath   = "/mutate-gateway-kusk-io-v1alpha1-envoyfleet"
+	EnvoyFleetValidatingWebhookPath = "/validate-gateway-kusk-io-v1alpha1-envoyfleet"
 )
 
 //+kubebuilder:webhook:path=/mutate-gateway-kusk-io-v1alpha1-envoyfleet,mutating=true,failurePolicy=fail,sideEffects=None,groups=gateway.kusk.io,resources=envoyfleet,verbs=create;update,versions=v1alpha1,name=menvoyfleet.kb.io,admissionReviewVersions=v1

diff --git a/api/v1alpha1/webhook_suite_test.go b/api/v1alpha1/webhook_suite_test.go
@@ -39,7 +39,6 @@ import (
 	. "github.com/onsi/gomega"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -52,7 +51,6 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
-var cfg *rest.Config
 var k8sClient client.Client
 var testEnv *envtest.Environment
 var ctx context.Context

diff --git a/cmd/kusk/cmd/add_gateway.go b/cmd/kusk/cmd/add_gateway.go
@@ -30,14 +30,15 @@ import (
 	"strconv"
 	"strings"
 
-	kuskv1 "github.com/kubeshop/kusk-gateway/api/v1alpha1"
-	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/errors"
-	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/utils"
 	"github.com/manifoldco/promptui"
 	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	kuskv1 "github.com/kubeshop/kusk-gateway/api/v1alpha1"
+	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/errors"
+	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/utils"
 )
 
 var (
@@ -241,7 +242,9 @@ func validatePort(input string) error {
 		return err
 	}
 	services := corev1.ServiceList{}
-	c.List(context.Background(), &services, &client.ListOptions{})
+	if err := c.List(context.Background(), &services, &client.ListOptions{}); err != nil {
+		return err
+	}
 
 	for _, svc := range services.Items {
 		if svc.Spec.Type == "LoadBalancer" {

diff --git a/cmd/kusk/cmd/dashboard.go b/cmd/kusk/cmd/dashboard.go
@@ -156,7 +156,11 @@ The flags --envoyfleet.namespace and --envoyfleet.name can be used to change the
 		<-readyCh
 
 		browserOpenCMD, browserOpenArgs := getBrowserOpenCmdAndArgs("http://localhost:8080")
-		process.Execute(browserOpenCMD, browserOpenArgs...)
+		if _, err := process.Execute(browserOpenCMD, browserOpenArgs...); err != nil {
+			fmt.Fprintln(os.Stderr, err)
+			reportError(err)
+		}
+
 		wg.Wait()
 	},
 }

diff --git a/cmd/kusk/cmd/deploy.go b/cmd/kusk/cmd/deploy.go
@@ -205,7 +205,7 @@ var deployCmd = &cobra.Command{
 func getParsedAndValidatedOpenAPISpec(overlaySpecPath, apiSpecPath string) (string, error) {
 	const KuskExtensionKey = "x-kusk"
 
-	parsedApiSpec := &openapi3.T{}
+	var parsedApiSpec *openapi3.T
 	var err error
 
 	if overlaySpecPath != "" {

diff --git a/cmd/kusk/cmd/install.go b/cmd/kusk/cmd/install.go
@@ -326,7 +326,9 @@ func unzip(path string) (string, error) {
 				distroDir = filePath
 			}
 
-			os.MkdirAll(filePath, os.ModePerm)
+			if err := os.MkdirAll(filePath, os.ModePerm); err != nil {
+				return "", err
+			}
 			continue
 		}
 

diff --git a/cmd/kusk/cmd/mock.go b/cmd/kusk/cmd/mock.go
@@ -115,6 +115,10 @@ $ kusk mock -i https://url.to.api.com
 			}
 
 			popFunc, err := pushDirectory(filepath.Dir(absoluteApiSpecPath))
+			if err != nil {
+				reportError(err)
+				kuskui.PrintError(err.Error())
+			}
 			defer func() {
 				if err := popFunc(); err != nil {
 					reportError(err)
@@ -455,10 +459,16 @@ func decorateLogEntry(entry mockingServer.AccessLogEntry) string {
 
 func init() {
 	rootCmd.AddCommand(mockCmd)
-	mockCmd.Flags().StringVarP(&apiSpecPath, "in", "i", "", "path to openapi spec you wish to mock")
-	mockCmd.MarkFlagRequired("in")
 
-	mockCmd.Flags().Uint32VarP(&mockServerPort, "port", "p", 0, "port to expose mock server on. If none specified, will search for next available port starting from 8080")
+	const (
+		inFlagName   = "in"
+		portFlagName = "port"
+	)
+
+	mockCmd.Flags().StringVarP(&apiSpecPath, inFlagName, "i", "", "path to openapi spec you wish to mock")
+	_ = mockCmd.MarkFlagRequired(inFlagName)
+
+	mockCmd.Flags().Uint32VarP(&mockServerPort, portFlagName, "p", 0, "port to expose mock server on. If none specified, will search for next available port starting from 8080")
 }
 
 var mockDescription = `Description:
@@ -472,7 +482,7 @@ Mock Command:
 kusk mock -i path-to-openapi-file.yaml
 kusk mock -i https://url.to.api.com
 
-Schema example: 
+Schema example:
 openapi: 3.0.0
 info:
   title: todo-backend-api
@@ -516,23 +526,23 @@ paths:
                 completed: true
                 order: 13
                 url: "http://mockedURL.com"
- 
-Generated JSON Response: 
+
+Generated JSON Response:
 {
   "completed": false,
   "order": 1957493166,
   "title": "Inventore ut.",
   "url": "http://langosh.name/andreanne.parker"
 }
- 
-Generated XML Response: 
+
+Generated XML Response:
  application/xml:
   example:
    title: "Mocked XML title"
    completed: true
    order: 13
    url: "http://mockedURL.com"
- 
+
 XML Respose from Defined Examples:
  <doc>
   <completed>true</completed>
@@ -547,6 +557,6 @@ completed: true
 order: 13
 url: "http://mockedURL.com"
 
-Stop Mock Server: 
+Stop Mock Server:
 ctrt+c
 `
diff --git a/cmd/kusk/cmd/root.go b/cmd/kusk/cmd/root.go
@@ -35,12 +35,13 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
+	"github.com/mattn/go-isatty"
+
 	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/errors"
 	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/kuskui"
 	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/utils"
 	"github.com/kubeshop/kusk-gateway/pkg/analytics"
 	"github.com/kubeshop/kusk-gateway/pkg/build"
-	"github.com/mattn/go-isatty"
 )
 
 var cfgFile string
@@ -60,7 +61,7 @@ var rootCmd = &cobra.Command{
 	Short: "",
 	Long:  ``,
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-		analytics.SendAnonymousCMDInfo(nil)
+		_ = analytics.SendAnonymousCMDInfo(nil)
 
 		versionCheck(cmd)
 
@@ -80,8 +81,7 @@ var rootCmd = &cobra.Command{
 // Execute adds all child commands to the root command and sets flags appropriately.
 // This is called by main.main(). It only needs to happen once to the rootCmd.
 func Execute() {
-	err := rootCmd.Execute()
-	if err != nil {
+	if err := rootCmd.Execute(); err != nil {
 		errors.NewErrorReporter(rootCmd, err).Report()
 		kuskui.PrintError(err.Error())
 		os.Exit(1)
@@ -91,14 +91,12 @@ func Execute() {
 const (
 	cmdGroupAnnotation = "GroupAnnotation"
 	cmdMngmCmdGroup    = "1-Management commands"
-	cmdGroupCommands   = "2-Commands"
 	cmdGroupCobra      = "other"
 
 	cmdGroupDelimiter = "-"
 )
 
 func helpMessageByGroups(cmd *cobra.Command) string {
-
 	groups := map[string][]string{}
 	for _, c := range cmd.Commands() {
 		var groupName string
@@ -122,7 +120,7 @@ func helpMessageByGroups(cmd *cobra.Command) string {
 	delete(groups, cmdGroupCobra)
 
 	groupNames := []string{}
-	for k, _ := range groups {
+	for k := range groups {
 		groupNames = append(groupNames, k)
 	}
 	sort.Strings(groupNames)

diff --git a/cmd/kusk/cmd/validate.go b/cmd/kusk/cmd/validate.go
@@ -27,9 +27,10 @@ package cmd
 import (
 	"fmt"
 
+	"github.com/spf13/cobra"
+
 	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/errors"
 	"github.com/kubeshop/kusk-gateway/cmd/kusk/internal/kuskui"
-	"github.com/spf13/cobra"
 )
 
 var validateCmd = &cobra.Command{
@@ -58,7 +59,7 @@ var validateCmd = &cobra.Command{
 
 func init() {
 	rootCmd.AddCommand(validateCmd)
-
-	validateCmd.Flags().StringVarP(&apiSpecPath, "in", "i", "", "file path or URL to OpenAPI spec file to generate mappings from. e.g. --in apispec.yaml")
-	validateCmd.MarkFlagRequired("file")
+	const inFlagName = "in"
+	validateCmd.Flags().StringVarP(&apiSpecPath, inFlagName, "i", "", "file path or URL to OpenAPI spec file to generate mappings from. e.g. --in apispec.yaml")
+	_ = validateCmd.MarkFlagRequired(inFlagName)
 }
diff --git a/cmd/kusk/cmd/version.go b/cmd/kusk/cmd/version.go
@@ -69,7 +69,7 @@ func NewVersionCommand(writer io.Writer, version string) *cobra.Command {
 				}
 			}
 
-			fmt.Fprintf(writer, "%s\n\n", formattedVersion)
+			_, _ = fmt.Fprintf(writer, "%s\n\n", formattedVersion)
 
 			c, err := utils.GetK8sClient()
 			if err != nil {

diff --git a/cmd/kusk/cmd/version_test.go b/cmd/kusk/cmd/version_test.go
@@ -31,15 +31,15 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func test_NewVersionCommand(t *testing.T) {
+func Test_NewVersionCommand(t *testing.T) {
 	t.Parallel()
-	assert := assert.New(t)
+	assertions := assert.New(t)
 
 	writer := bytes.NewBufferString("")
 	version := "_some-version_"
 	command := NewVersionCommand(writer, version)
-	command.RunE(nil, []string{})
+	_ = command.RunE(command, []string{})
 
 	actual := writer.String()
-	assert.Contains(actual, version)
+	assertions.Contains(actual, version)
 }
diff --git a/cmd/kusk/internal/mocking/server/server.go b/cmd/kusk/internal/mocking/server/server.go
@@ -58,7 +58,9 @@ func (m *MockServer) Start(ctx context.Context) (string, error) {
 
 	// wait for download to complete, discard output
 	defer reader.Close()
-	io.Copy(io.Discard, reader)
+	if _, err := io.Copy(io.Discard, reader); err != nil {
+		return "", fmt.Errorf("download of mock server image did not complete: %w", err)
+	}
 
 	u, err := url.Parse(m.apiToMock)
 	if err != nil {

diff --git a/cmd/kusk/internal/overlays/overlays.go b/cmd/kusk/internal/overlays/overlays.go
@@ -135,7 +135,9 @@ func applyOverlay(path string, extends string) (string, error) {
 	}
 
 	defer reader.Close()
-	io.Copy(io.Discard, reader)
+	if _, err := io.Copy(io.Discard, reader); err != nil {
+		return "", fmt.Errorf("download of %v image did not complete: %w", imageName, err)
+	}
 
 	volumes := fmt.Sprintf("-v=%s:/overlay.yaml", abs)
 	var extendVolume string

diff --git a/cmd/kusk/internal/utils/utils.go b/cmd/kusk/internal/utils/utils.go
@@ -33,22 +33,23 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 
-	kuskv1 "github.com/kubeshop/kusk-gateway/api/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	aggv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 
+	kuskv1 "github.com/kubeshop/kusk-gateway/api/v1alpha1"
+
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func GetK8sClient() (client.Client, error) {
 	scheme := runtime.NewScheme()
-	appsv1.AddToScheme(scheme)
-	kuskv1.AddToScheme(scheme)
-	corev1.AddToScheme(scheme)
-	aggv1.AddToScheme(scheme)
+	_ = appsv1.AddToScheme(scheme)
+	_ = kuskv1.AddToScheme(scheme)
+	_ = corev1.AddToScheme(scheme)
+	_ = aggv1.AddToScheme(scheme)
 
 	config, err := getConfig()
 	if err != nil {

diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -277,8 +277,8 @@ func main() {
 		OpenApiParser:      spec.NewParser(&openapi3.Loader{IsExternalRefsAllowed: true}),
 	}
 
-	analytics.SendAnonymousInfo(ctx, controllerConfigManager.Client, "kusk", "kusk-gateway manager bootstrapping")
-	heartBeat(ctx, controllerConfigManager.Client)
+	_ = analytics.SendAnonymousInfo(ctx, controllerConfigManager.Client, "kusk", "kusk-gateway manager bootstrapping")
+	heartBeat(ctx, controllerConfigManager.Client, logger)
 
 	// The watcher for k8s secrets to trigger the refresh of configuration in case certificates secrets change.
 	go func() {
@@ -356,15 +356,20 @@ func main() {
 
 	setupLog.Info("Starting manager")
 	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
-		analytics.SendAnonymousInfo(ctx, controllerConfigManager.Client, "kusk", fmt.Sprintf("kusk-gateway manager bootstrapping failed with error %s", err.Error()))
+		_ = analytics.SendAnonymousInfo(ctx, controllerConfigManager.Client, "kusk", fmt.Sprintf("kusk-gateway manager bootstrapping failed with error %s", err.Error()))
 		setupLog.Error(err, "Problem running manager")
 		os.Exit(1)
 	}
 }
-func heartBeat(ctx context.Context, client client.Client) {
+
+func heartBeat(ctx context.Context, client client.Client, logger logr.Logger) {
 	c := cron.New()
-	c.AddFunc("@daily", func() {
-		analytics.SendAnonymousInfo(ctx, client, "kusk-heartbeat", "kusk-gateway daily heartbeat")
+	err := c.AddFunc("@daily", func() {
+		_ = analytics.SendAnonymousInfo(ctx, client, "kusk-heartbeat", "kusk-gateway daily heartbeat")
 	})
+	if err != nil {
+		_ = analytics.SendAnonymousInfo(ctx, client, "kusk-heartbeat", fmt.Sprintf("kusk-gateway failed to add daily heartbeat: %s", err.Error()))
+	}
+
 	c.Start()
 }