lint: Integrate golangcilint (#68)

* lint: Integrate golangcilint Signed-off-by: Quique Llorente <[email protected]> * lint: Fix deadcode Signed-off-by: Quique Llorente <[email protected]> * lint: Exclude dupl from tests Signed-off-by: Quique Llorente <[email protected]> * lint: fix errcheck Signed-off-by: Quique Llorente <[email protected]> * lint: fix gochecknoinits Signed-off-by: Quique Llorente <[email protected]> * lint: Fix gocritic Signed-off-by: Quique Llorente <[email protected]> * lint: fix gocyclo Signed-off-by: Quique Llorente <[email protected]> * lint: fix gofmt Signed-off-by: Quique Llorente <[email protected]> * lint: fix gomnd Signed-off-by: Quique Llorente <[email protected]> * lint: fix gosec Signed-off-by: Quique Llorente <[email protected]> * lint: fix govet Signed-off-by: Quique Llorente <[email protected]> * lint: fix misspell Signed-off-by: Quique Llorente <[email protected]> * lint: fix stylecheck Signed-off-by: Quique Llorente <[email protected]> * lint: fix unconvert Signed-off-by: Quique Llorente <[email protected]> * lint: fix unparam Signed-off-by: Quique Llorente <[email protected]> * lint: fix whitespace Signed-off-by: Quique Llorente <[email protected]> * lint: fix lint for webhook server Signed-off-by: Quique Llorente <[email protected]> * lint: fix lll Signed-off-by: Quique Llorente <[email protected]> * lint: fix goheader Signed-off-by: Quique Llorente <[email protected]>
qinqon · Mar 17, 2022 · 60bb3a5 · 60bb3a5
1 parent 22a7a67
commit 60bb3a5
Show file tree

Hide file tree

Showing 28 changed files with 694 additions and 311 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -5,6 +5,22 @@ on:
   pull_request:
     branches: [ main ]
 jobs:
+  lint:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check out code
+      uses: actions/checkout@v2
+    - uses: arnested/go-version-action@v1
+      id: go-version
+    - name: Set up Go
+      uses: actions/setup-go@v1
+      with:
+        go-version: ${{ steps.go-version.outputs.minimal }}
+    - name: golangci-lint
+      uses: golangci/golangci-lint-action@v2
+      with:
+          version: v1.42.1
   build:
     runs-on: ubuntu-latest
     steps:

diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,114 @@
+# Copyright 2021 The NMPolicy Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+
+linters-settings:
+  dupl:
+    threshold: 100
+  funlen:
+    lines: 100
+    statements: 50
+  gci:
+    local-prefixes: github.com/qinqon/kube-admisssion-webhook
+  goconst:
+    min-len: 2
+    min-occurrences: 2
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - ifElseChain
+      - octalLiteral
+      - whyNoLint
+      - wrapperFunc
+  gocyclo:
+    min-complexity: 15
+  goheader:
+    values:
+      regexp: 
+        year-regexp: 2\d\d\d
+    template-path: header.tpl
+  goimports:
+    local-prefixes: github.com/qinqon/kube-admisssion-webhook
+  gomnd:
+    settings:
+      mnd:
+        # don't include the "operation" and "assign"
+        checks: argument,case,condition,return
+  govet:
+    check-shadowing: true
+  lll:
+    line-length: 140
+  maligned:
+    suggest-new: true
+  misspell:
+    locale: US
+  nolintlint:
+    allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space)
+    allow-unused: false # report any unused nolint directives
+    require-explanation: false # don't require an explanation for nolint directives
+    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+
+issues:
+  exclude-rules:
+    - path: _test.go
+      linters:
+        - dupl
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - deadcode
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - exportloopref
+    - exhaustive
+    - funlen
+    - gochecknoinits
+    - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goheader
+    - goimports
+    - gomnd
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - lll
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - rowserrcheck
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck
+    - whitespace
diff --git a/Makefile b/Makefile
@@ -4,12 +4,8 @@ WHAT ?= ./pkg/...
 
 all: test
 
-format:
-	hack/whitespace.sh format
-	gofmt -w ./pkg
-
-vet:
-	go vet ./pkg/...
+lint:
+	hack/lint.sh
 
 testenv:
 	hack/setup-testenv.sh

diff --git a/hack/lint.sh b/hack/lint.sh
@@ -0,0 +1,8 @@
+#!/bin/bash -xe
+
+golangci_lint_version=v1.42.1
+if [ ! -f $(go env GOPATH)/bin/golangci-lint ]; then
+    curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin $golangci_lint_version
+fi
+golangci-lint run
+
diff --git a/header.tpl b/header.tpl
@@ -0,0 +1,13 @@
+ * Copyright {{ year-regexp }} Kube Admission Webhook Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *	  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
diff --git a/pkg/certificate/certificate_suite_test.go b/pkg/certificate/certificate_suite_test.go
@@ -1,3 +1,19 @@
+/*
+ * Copyright 2022 Kube Admission Webhook Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *	  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package certificate
 
 import (
@@ -105,6 +121,8 @@ func deleteResources() {
 
 var _ = BeforeSuite(func() {
 
+	klog.InitFlags(nil)
+
 	testEnv = &envtest.Environment{
 		UseExistingCluster: &useCluster,
 	}
@@ -116,7 +134,7 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred(), "should success creating client")
 
 	// Ideally we create/delete the namespace at every test but, envtest
-	// cannot delete namespaces [1] so we just create it at the beggining
+	// cannot delete namespaces [1] so we just create it at the beginning
 	// of the test suite.
 	//
 	// [1] https://book.kubebuilder.io/reference/testing/envtest.html?highlight=envtest#testing-considerations
@@ -137,10 +155,6 @@ var _ = AfterSuite(func() {
 	Expect(err).ToNot(HaveOccurred(), "should success stopping testenv")
 })
 
-func init() {
-	klog.InitFlags(nil)
-}
-
 func TestCertificate(t *testing.T) {
 	RegisterFailHandler(Fail)
 	junitReporter := reporters.NewJUnitReporter("junit.certificate_suite_test.xml")

diff --git a/pkg/certificate/cleanup.go b/pkg/certificate/cleanup.go
@@ -1,3 +1,19 @@
+/*
+ * Copyright 2022 Kube Admission Webhook Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *	  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package certificate
 
 import (
@@ -36,13 +52,13 @@ func (m *Manager) earliestElapsedForServiceCertsCleanup() (time.Duration, error)
 	}
 
 	elapsedTimesForCleanup := []time.Duration{}
-	for service, _ := range services {
-
+	for service := range services {
 		certs, err := m.getTLSCerts(service)
 		if err != nil {
 			return time.Duration(0), fmt.Errorf("failed getting TLS keypair from service %s to calculate cleanup next run: %w", service, err)
 		}
-		elapsedTimeForCleanup, err := m.earliestElapsedForCleanup(m.log.WithName("earliestElapsedForServiceCertsCleanup").WithValues("service", service), certs)
+		elapsedTimeForCleanup, err := m.earliestElapsedForCleanup(
+			m.log.WithName("earliestElapsedForServiceCertsCleanup").WithValues("service", service), certs)
 		if err != nil {
 			return time.Duration(0), err
 		}
@@ -84,7 +100,7 @@ func (m *Manager) earliestCleanupDeadlineForCerts(certificates []*x509.Certifica
 
 func (m *Manager) cleanUpCABundle() error {
 	m.log.Info("cleanUpCABundle")
-	_, err := m.updateWebhookCABundleWithFunc(func([]byte) ([]byte, error) {
+	err := m.updateWebhookCABundleWithFunc(func([]byte) ([]byte, error) {
 		cas, err := m.getCACertsFromCABundle()
 		if err != nil {
 			return nil, errors.Wrap(err, "failed getting ca certs to start cleanup")
@@ -112,23 +128,27 @@ func (m *Manager) cleanUpServiceCerts() error {
 		return fmt.Errorf("failed getting services to do the cleanup: %w", err)
 	}
 
-	for service, _ := range services {
-		m.applySecret(service, corev1.SecretTypeTLS, nil, func(secret corev1.Secret, keyPair *triple.KeyPair) (*corev1.Secret, error) {
-			certPEM, found := secret.Data[corev1.TLSCertKey]
-			if !found {
-				return nil, errors.Wrapf(err, "TLS cert not found at secret %s to clean up ", service)
-			}
-
-			certs, err := triple.ParseCertsPEM(certPEM)
-			if err != nil {
-				return nil, errors.Wrapf(err, "failed parsing TLS cert PEM at secret %s to clean up", service)
-			}
-
-			cleanedCerts := m.cleanUpCertificates(certs)
-			pem := triple.EncodeCertsPEM(cleanedCerts)
-			secret.Data[corev1.TLSCertKey] = pem
-			return &secret, nil
-		})
+	for service := range services {
+		applyErr := m.applySecret(service, corev1.SecretTypeTLS, nil,
+			func(secret *corev1.Secret, keyPair *triple.KeyPair) (*corev1.Secret, error) {
+				certPEM, found := secret.Data[corev1.TLSCertKey]
+				if !found {
+					return nil, errors.Wrapf(err, "TLS cert not found at secret %s to clean up ", service)
+				}
+
+				certs, err := triple.ParseCertsPEM(certPEM)
+				if err != nil {
+					return nil, errors.Wrapf(err, "failed parsing TLS cert PEM at secret %s to clean up", service)
+				}
+
+				cleanedCerts := m.cleanUpCertificates(certs)
+				pem := triple.EncodeCertsPEM(cleanedCerts)
+				secret.Data[corev1.TLSCertKey] = pem
+				return secret, nil
+			})
+		if applyErr != nil {
+			return applyErr
+		}
 	}
 	return nil
 }

diff --git a/pkg/certificate/cleanup_test.go b/pkg/certificate/cleanup_test.go
@@ -1,3 +1,19 @@
+/*
+ * Copyright 2022 Kube Admission Webhook Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *	  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package certificate
 
 import (

diff --git a/pkg/certificate/client.go b/pkg/certificate/client.go
@@ -1,3 +1,19 @@
+/*
+ * Copyright 2022 Kube Admission Webhook Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *	  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package certificate
 
 import (
@@ -10,12 +26,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	pollInterval = 5 * time.Second
+	pollTimeout  = 30 * time.Second
+)
+
 // get wraps controller-runtime client `Get` to ensure that client cache
 // is ready, sometimes after controller-runtime manager is ready the
 // cache is still not ready, specially if you webhook or plain runnable
 // is being used since it miss some controller bits.
 func (m *Manager) get(key types.NamespacedName, value client.Object) error {
-	return wait.PollImmediate(5*time.Second, 30*time.Second, func() (bool, error) {
+	return wait.PollImmediate(pollInterval, pollTimeout, func() (bool, error) {
 		err := m.client.Get(context.TODO(), key, value)
 		if err != nil {
 			if _, cacheNotStarted := err.(*cache.ErrCacheNotStarted); cacheNotStarted {