From 32cda961622f2db80150c05ada6fa521ee923517 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 22 Jun 2021 17:57:17 -0700 Subject: [PATCH 1/3] Add more linters --- .github/workflows/lint.yml | 2 +- .golangci.yml | 26 ++++++++++++++++++++++---- Makefile | 4 ++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8e1442bc..d8bc1d9f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -29,6 +29,6 @@ jobs: - name: Checkout Repository uses: actions/checkout@v2 - name: Lint Code - uses: golangci/golangci-lint-action@v2.5.2 + uses: golangci/golangci-lint-action@v2 with: args: --timeout ${{ env.GOLANGCI_TIMEOUT }} diff --git a/.golangci.yml b/.golangci.yml index 25b1c5fe..96ec11d1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,13 +4,31 @@ linters-settings: linters: enable: + - asciicheck + - deadcode + - errcheck + - errorlint + - gofmt + - gofumpt - goimports + - gosec - gosimple - govet + - ineffassign + - makezero - misspell - - gofmt - - unparam - - unconvert + - nilerr + - noctx + - predeclared + - staticcheck - structcheck - - errcheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - wastedassign disable-all: true +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/Makefile b/Makefile index e22e6190..8cb1a67a 100644 --- a/Makefile +++ b/Makefile @@ -85,6 +85,10 @@ fmt: ## Run go fmt against code. vet: ## Run go vet against code. go vet ./... +lint: ## Run golangci-lint against code. + docker run --pull always --rm -v $(shell pwd):/nginx-ingress-operator -w /nginx-ingress-operator -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:latest golangci-lint --color always run + + ENVTEST_ASSETS_DIR=$(shell pwd)/testbin test: manifests generate fmt vet ## Run tests. mkdir -p ${ENVTEST_ASSETS_DIR} From 77426176e4578f0ba8e7c2da570cfa1c3b6a0bb0 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 22 Jun 2021 17:57:31 -0700 Subject: [PATCH 2/3] Fix error wrapping --- controllers/crds.go | 12 ++++++------ controllers/prerequisites.go | 12 ++++++------ controllers/utils.go | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/controllers/crds.go b/controllers/crds.go index 94996a99..fcd83e6a 100644 --- a/controllers/crds.go +++ b/controllers/crds.go @@ -44,7 +44,7 @@ func kicCRDs() ([]*v1.CustomResourceDefinition, error) { for _, path := range manifests { f, err := os.Open(path) if err != nil { - return nil, fmt.Errorf("failed to open the CRD manifest %v: %v", path, err) + return nil, fmt.Errorf("failed to open the CRD manifest %v: %w", path, err) } var crd v1.CustomResourceDefinition @@ -52,12 +52,12 @@ func kicCRDs() ([]*v1.CustomResourceDefinition, error) { err = yaml.NewYAMLOrJSONDecoder(f, decoderBufferSize).Decode(&crd) if err != nil { - return nil, fmt.Errorf("failed to parse the CRD manifest %v: %v", path, err) + return nil, fmt.Errorf("failed to parse the CRD manifest %v: %w", path, err) } err = f.Close() if err != nil { - return nil, fmt.Errorf("failed to close the CRD manifest %v: %v", path, err) + return nil, fmt.Errorf("failed to close the CRD manifest %v: %w", path, err) } crds = append(crds, &crd) @@ -87,10 +87,10 @@ func createKICCustomResourceDefinitions(log logr.Logger, mgr manager.Manager) er log.V(1).Info(fmt.Sprintf("no previous CRD %v found, creating a new one.", crd.Name)) _, err = crdsClient.Create(context.TODO(), crd, metav1.CreateOptions{}) if err != nil { - return fmt.Errorf("error creating CRD %v: %v", crd.Name, err) + return fmt.Errorf("error creating CRD %v: %w", crd.Name, err) } } else { - return fmt.Errorf("error getting CRD %v: %v", crd.Name, err) + return fmt.Errorf("error getting CRD %v: %w", crd.Name, err) } } else { // Update CRDs if they already exist @@ -98,7 +98,7 @@ func createKICCustomResourceDefinitions(log logr.Logger, mgr manager.Manager) er oldCRD.Spec = crd.Spec _, err = crdsClient.Update(context.TODO(), oldCRD, metav1.UpdateOptions{}) if err != nil { - return fmt.Errorf("error updating CRD %v: %v", crd.Name, err) + return fmt.Errorf("error updating CRD %v: %w", crd.Name, err) } } } diff --git a/controllers/prerequisites.go b/controllers/prerequisites.go index 46993088..fda7b90a 100644 --- a/controllers/prerequisites.go +++ b/controllers/prerequisites.go @@ -140,10 +140,10 @@ func (r *NginxIngressControllerReconciler) createCommonResources(log logr.Logger log.Info("no previous ClusterRole found, creating a new one.") err = r.Create(context.TODO(), cr) if err != nil { - return fmt.Errorf("error creating ClusterRole: %v", err) + return fmt.Errorf("error creating ClusterRole: %w", err) } } else { - return fmt.Errorf("error getting ClusterRole: %v", err) + return fmt.Errorf("error getting ClusterRole: %w", err) } } else { // For updates in the ClusterRole permissions (eg new CRDs of the Ingress Controller). @@ -151,7 +151,7 @@ func (r *NginxIngressControllerReconciler) createCommonResources(log logr.Logger cr := clusterRoleForNginxIngressController(clusterRoleName) err = r.Update(context.TODO(), cr) if err != nil { - return fmt.Errorf("error updating ClusterRole: %v", err) + return fmt.Errorf("error updating ClusterRole: %w", err) } } @@ -164,12 +164,12 @@ func (r *NginxIngressControllerReconciler) createCommonResources(log logr.Logger } if err != nil { - return fmt.Errorf("error creating ClusterRoleBinding: %v", err) + return fmt.Errorf("error creating ClusterRoleBinding: %w", err) } err = createKICCustomResourceDefinitions(log, r.Mgr) if err != nil { - return fmt.Errorf("error creating KIC CRDs: %v", err) + return fmt.Errorf("error creating KIC CRDs: %w", err) } if r.SccAPIExists { @@ -184,7 +184,7 @@ func (r *NginxIngressControllerReconciler) createCommonResources(log logr.Logger } if err != nil { - return fmt.Errorf("error creating SecurityContextConstraints: %v", err) + return fmt.Errorf("error creating SecurityContextConstraints: %w", err) } } diff --git a/controllers/utils.go b/controllers/utils.go index fdf91a17..cb2919a0 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -205,7 +205,7 @@ func GetK8sVersion(client kubernetes.Interface) (v *version.Version, err error) runningVersion, err := version.ParseGeneric(serverVersion.String()) if err != nil { - return nil, fmt.Errorf("unexpected error parsing running Kubernetes version: %v", err) + return nil, fmt.Errorf("unexpected error parsing running Kubernetes version: %w", err) } return runningVersion, nil From ec14839f048cf231ddb0cdcf671216bbfeb560ae Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 22 Jun 2021 18:05:00 -0700 Subject: [PATCH 3/3] Remove unused code --- controllers/suite_test.go | 2 -- controllers/utils.go | 18 ------------------ 2 files changed, 20 deletions(-) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 00647f83..f9feac05 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -23,7 +23,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/envtest/printer" @@ -38,7 +37,6 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var ( - cfg *rest.Config k8sClient client.Client testEnv *envtest.Environment ) diff --git a/controllers/utils.go b/controllers/utils.go index cb2919a0..371ba0fc 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -146,24 +146,6 @@ func hasDifferentArguments(container corev1.Container, instance *k8sv1alpha1.Ngi return !reflect.DeepEqual(newArgs, container.Args) } -func contains(list []string, s string) bool { - for _, v := range list { - if v == s { - return true - } - } - return false -} - -func remove(list []string, s string) []string { - for i, v := range list { - if v == s { - list = append(list[:i], list[i+1:]...) - } - } - return list -} - func VerifySCCAPIExists() (bool, error) { cfg, err := config.GetConfig() if err != nil {