Merge pull request #10 from srl-labs/linting

lint before it is too late
srl-labs · Jul 29, 2022 · 02f0cb4 · 02f0cb4
2 parents 056b5a8 + 347091a
commit 02f0cb4
Show file tree

Hide file tree

Showing 14 changed files with 712 additions and 338 deletions.
diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml
@@ -23,30 +23,18 @@ jobs:
       - name: kubebuilder tests
         run: make test
 
-  staticcheck:
-    runs-on: ubuntu-20.04
-    steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-go@v2
-        with:
-          go-version: ${{ env.GOVER }}
-      - name: Staticcheck
-        run: |
-          go install honnef.co/go/tools/cmd/staticcheck@latest
-          staticcheck ./...
-
-  golang-ci:
+  golang-ci-lint:
     runs-on: ubuntu-20.04
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           fetch-depth: 0
       - uses: actions/setup-go@v2
         with:
           go-version: ${{ env.GOVER }}
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+        uses: golangci/golangci-lint-action@v3
         with:
-          args: --timeout=3m
-          skip-go-installation: true
+          args: --config=./.github/workflows/linters/.golangci.yml
+          version: "v1.47.2"
diff --git a/.github/workflows/linters/.golangci.yml b/.github/workflows/linters/.golangci.yml
@@ -0,0 +1,77 @@
+run:
+  # running w/ 1.17 because we dont actually need/use 1.18 things and 1.18 breaks some linters.
+  go: "1.17"
+  timeout: 5m
+  skip-dirs:
+    - private
+    - bin
+
+output:
+  sort-results: true
+
+linters:
+  enable:
+    - nlreturn
+    - forbidigo
+    - gofumpt
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - exhaustive
+    - funlen
+    - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goimports
+    - gomnd
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - lll
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - revive
+    - rowserrcheck
+    - exportloopref
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+    - whitespace
+    - asciicheck
+    - gochecknoglobals
+    - gocognit
+    - godot
+    - godox
+    - goerr113
+    - nestif
+    - prealloc
+    - testpackage
+    - wsl
+linters-settings:
+  lll:
+    line-length: 140
+
+issues:
+  # https://github.com/golangci/golangci-lint/issues/2439#issuecomment-1002912465
+  exclude-use-default: false
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - gomnd
+        - dupl
+        - structcheck
+        - unused
+        - unparam
diff --git a/.mk/lint.mk b/.mk/lint.mk
@@ -0,0 +1,32 @@
+GOFUMPT_CMD := docker run --rm -it -v $(shell pwd):/work ghcr.io/hellt/gofumpt:0.3.1
+GOFUMPT_FLAGS := -l -w .
+
+GODOT_CMD := docker run --rm -it -v $(shell pwd):/work ghcr.io/hellt/godot:1.4.11
+GODOT_FLAGS := -w .
+
+GOLINES_CMD := docker run --rm -it -v $(shell pwd):/work ghcr.io/hellt/golines:0.10.0 golines
+GOLINES_FLAGS := -w .
+
+GOLANGCI_CMD := docker run -it --rm -v $$(pwd):/app -w /app golangci/golangci-lint:v1.47.2 golangci-lint
+GOLANGCI_FLAGS := --config ./.github/workflows/linters/.golangci.yml run -v --fix
+
+
+# when running in a CI env we use locally installed bind
+ifdef CI
+	GOFUMPT_CMD := gofumpt
+endif
+
+
+format: gofumpt godot golines # apply Go formatters
+
+gofumpt:
+	${GOFUMPT_CMD} ${GOFUMPT_FLAGS}
+
+godot:
+	${GODOT_CMD} ${GODOT_FLAGS}
+
+golines:
+	${GOLINES_CMD} ${GOLINES_FLAGS}
+
+golangci: # linting with golang-ci lint container
+	${GOLANGCI_CMD} ${GOLANGCI_FLAGS}
diff --git a/Makefile b/Makefile
@@ -1,3 +1,4 @@
+include .mk/lint.mk
 
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
@@ -84,28 +85,22 @@ deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in
 undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/config.
 	$(KUSTOMIZE) build config/default | kubectl delete -f -
 
-
+# check source for latest methods on getting these binaries
+# https://github.com/kubernetes-sigs/kubebuilder/blob/95f55fcf8b87397f7fdf97456b10ff70b31f728e/pkg/plugins/golang/v3/scaffolds/internal/templates/makefile.go
 CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
 controller-gen: ## Download controller-gen locally if necessary.
-	$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1)
+	$(call go-install,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1)
 
 KUSTOMIZE = $(shell pwd)/bin/kustomize
 kustomize: ## Download kustomize locally if necessary.
-	$(call go-get-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v3@v3.8.7)
+	$(call go-install,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v3@v3.8.7)
 
-# go-get-tool will 'go get' any package $2 and install it to $1.
-PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
-define go-get-tool
+# go-install will 'go get' any package $2 and install it to $1.
+PROJECT_DIR := $(shell dirname $(abspath $(firstword $(MAKEFILE_LIST))))
+define go-install
 @[ -f $(1) ] || { \
 set -e ;\
-TMP_DIR=$$(mktemp -d) ;\
-cd $$TMP_DIR ;\
-go mod init tmp ;\
 echo "Downloading $(2)" ;\
-GOBIN=$(PROJECT_DIR)/bin go get $(2) ;\
-rm -rf $$TMP_DIR ;\
+GOBIN=$(PROJECT_DIR)/bin go install $(2) ;\
 }
 endef
-
-ci-lint: # linting with golang-ci lint
-	docker run -it --rm -v $$(pwd):/app -w /app golangci/golangci-lint:latest  golangci-lint run -v --timeout 2m
diff --git a/api/clientset/v1alpha1/srlinux.go b/api/clientset/v1alpha1/srlinux.go
@@ -1,7 +1,11 @@
+// Package v1alpha1 is an v1alpha version of a Clientset for SR Linux customer resource.
 package v1alpha1
 
+// note to my future self: see https://www.martin-helmich.de/en/blog/kubernetes-crd-client.html for details
+
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -16,15 +20,27 @@ import (
 	typesv1alpha1 "github.com/srl-labs/srl-controller/api/types/v1alpha1"
 )
 
+// ErrUpdateFailed occurs when update operation fails on srlinux CR.
+var ErrUpdateFailed = errors.New("operation update failed")
+
 // SrlinuxInterface provides access to the Srlinux CRD.
 type SrlinuxInterface interface {
 	List(ctx context.Context, opts metav1.ListOptions) (*typesv1alpha1.SrlinuxList, error)
 	Get(ctx context.Context, name string, options metav1.GetOptions) (*typesv1alpha1.Srlinux, error)
 	Create(ctx context.Context, srlinux *typesv1alpha1.Srlinux) (*typesv1alpha1.Srlinux, error)
 	Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error
 	Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error)
-	Unstructured(ctx context.Context, name string, opts metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error)
-	Update(ctx context.Context, obj *unstructured.Unstructured, opts metav1.UpdateOptions) (*typesv1alpha1.Srlinux, error)
+	Unstructured(
+		ctx context.Context,
+		name string,
+		opts metav1.GetOptions,
+		subresources ...string,
+	) (*unstructured.Unstructured, error)
+	Update(
+		ctx context.Context,
+		obj *unstructured.Unstructured,
+		opts metav1.UpdateOptions,
+	) (*typesv1alpha1.Srlinux, error)
 }
 
 // Interface is the clientset interface for srlinux.
@@ -38,7 +54,7 @@ type Clientset struct {
 	restClient rest.Interface
 }
 
-var gvr = schema.GroupVersionResource{
+var gvr = schema.GroupVersionResource{ // nolint: gochecknoglobals
 	Group:    typesv1alpha1.GroupVersion.Group,
 	Version:  typesv1alpha1.GroupVersion.Version,
 	Resource: "srlinuxes",
@@ -51,21 +67,26 @@ func NewForConfig(c *rest.Config) (*Clientset, error) {
 	config.APIPath = "/apis"
 	config.NegotiatedSerializer = scheme.Codecs.WithoutConversion()
 	config.UserAgent = rest.DefaultKubernetesUserAgent()
+
 	dClient, err := dynamic.NewForConfig(c)
 	if err != nil {
 		return nil, err
 	}
+
 	dInterface := dClient.Resource(gvr)
+
 	rClient, err := rest.RESTClientFor(&config)
 	if err != nil {
 		return nil, err
 	}
+
 	return &Clientset{
 		dInterface: dInterface,
 		restClient: rClient,
 	}, nil
 }
 
+// Srlinux initializes srlinuxClient struct which implements SrlinuxInterface.
 func (c *Clientset) Srlinux(namespace string) SrlinuxInterface {
 	return &srlinuxClient{
 		dInterface: c.dInterface,
@@ -80,7 +101,11 @@ type srlinuxClient struct {
 	ns         string
 }
 
-func (s *srlinuxClient) List(ctx context.Context, opts metav1.ListOptions) (*typesv1alpha1.SrlinuxList, error) {
+// List gets a list of SRLinux resources.
+func (s *srlinuxClient) List(
+	ctx context.Context,
+	opts metav1.ListOptions,
+) (*typesv1alpha1.SrlinuxList, error) {
 	result := typesv1alpha1.SrlinuxList{}
 	err := s.restClient.
 		Get().
@@ -93,7 +118,12 @@ func (s *srlinuxClient) List(ctx context.Context, opts metav1.ListOptions) (*typ
 	return &result, err
 }
 
-func (s *srlinuxClient) Get(ctx context.Context, name string, opts metav1.GetOptions) (*typesv1alpha1.Srlinux, error) {
+// Get gets SRLinux resource.
+func (s *srlinuxClient) Get(
+	ctx context.Context,
+	name string,
+	opts metav1.GetOptions,
+) (*typesv1alpha1.Srlinux, error) {
 	result := typesv1alpha1.Srlinux{}
 	err := s.restClient.
 		Get().
@@ -107,7 +137,11 @@ func (s *srlinuxClient) Get(ctx context.Context, name string, opts metav1.GetOpt
 	return &result, err
 }
 
-func (s *srlinuxClient) Create(ctx context.Context, srlinux *typesv1alpha1.Srlinux) (*typesv1alpha1.Srlinux, error) {
+// Create creates SRLinux resource.
+func (s *srlinuxClient) Create(
+	ctx context.Context,
+	srlinux *typesv1alpha1.Srlinux,
+) (*typesv1alpha1.Srlinux, error) {
 	result := typesv1alpha1.Srlinux{}
 	err := s.restClient.
 		Post().
@@ -120,8 +154,12 @@ func (s *srlinuxClient) Create(ctx context.Context, srlinux *typesv1alpha1.Srlin
 	return &result, err
 }
 
-func (s *srlinuxClient) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) {
+func (s *srlinuxClient) Watch(
+	ctx context.Context,
+	opts metav1.ListOptions,
+) (watch.Interface, error) {
 	opts.Watch = true
+
 	return s.restClient.
 		Get().
 		Namespace(s.ns).
@@ -130,31 +168,43 @@ func (s *srlinuxClient) Watch(ctx context.Context, opts metav1.ListOptions) (wat
 		Watch(ctx)
 }
 
-func (t *srlinuxClient) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error {
-	return t.restClient.
+func (s *srlinuxClient) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error {
+	return s.restClient.
 		Delete().
-		Namespace(t.ns).
+		Namespace(s.ns).
 		Resource(gvr.Resource).
 		VersionedParams(&opts, scheme.ParameterCodec).
 		Name(name).
 		Do(ctx).
 		Error()
 }
 
-func (s *srlinuxClient) Update(ctx context.Context, obj *unstructured.Unstructured, opts metav1.UpdateOptions) (*typesv1alpha1.Srlinux, error) {
+func (s *srlinuxClient) Update(
+	ctx context.Context,
+	obj *unstructured.Unstructured,
+	opts metav1.UpdateOptions,
+) (*typesv1alpha1.Srlinux, error) {
 	result := typesv1alpha1.Srlinux{}
+
 	obj, err := s.dInterface.Namespace(s.ns).UpdateStatus(ctx, obj, metav1.UpdateOptions{})
 	if err != nil {
 		return nil, err
 	}
+
 	err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &result)
 	if err != nil {
-		return nil, fmt.Errorf("failed to type assert return to srlinux")
+		return nil, fmt.Errorf("failed to type assert return to srlinux: %w", ErrUpdateFailed)
 	}
+
 	return &result, nil
 }
 
-func (s *srlinuxClient) Unstructured(ctx context.Context, name string, opts metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) {
+func (s *srlinuxClient) Unstructured(
+	ctx context.Context,
+	name string,
+	opts metav1.GetOptions,
+	subresources ...string,
+) (*unstructured.Unstructured, error) {
 	return s.dInterface.Namespace(s.ns).Get(ctx, name, opts, subresources...)
 }