Merge pull request #360 from inteon/fix_linters

Fix all linter issues and un-ignore golanci-lint linter exceptions

.golangci.yaml

@@ -1,28 +1,3 @@
-issues:
-  exclude-rules:
-    - linters:
-        - errcheck
-        - typecheck
-        - staticcheck
-        - gosimple
-        - contextcheck
-        - promlinter
-        - prealloc
-        - containedctx
-        - gci
-        - errorlint
-        - gosec
-        - unparam
-        - gocritic
-        - gofmt
-        - unconvert
-        - misspell
-        - mirror
-        - wastedassign
-        - ineffassign
-        - unused
-        - dupword
-      text: ".*"
 linters:
   # Explicitly define all enabled linters
   disable-all: true

cmd/trust-manager/app/app.go

@@ -140,12 +140,14 @@ func NewCommand() *cobra.Command {
 			}
 
 			// Add readiness check that the manager's informers have been synced.
-			mgr.AddReadyzCheck("informers_synced", func(req *http.Request) error {
+			if err := mgr.AddReadyzCheck("informers_synced", func(req *http.Request) error {
 				if mgr.GetCache().WaitForCacheSync(req.Context()) {
 					return nil
 				}
 				return errors.New("informers not synced")
-			})
+			}); err != nil {
+				return fmt.Errorf("failed to add readiness check: %w", err)
+			}
 
 			ctx := ctrl.SetupSignalHandler()
 
@@ -155,7 +157,9 @@ func NewCommand() *cobra.Command {
 			}
 
 			// Register webhook handlers with manager.
-			webhook.Register(mgr, webhook.Options{Log: opts.Logr.WithName("webhook")})
+			if err := webhook.Register(mgr, webhook.Options{Log: opts.Logr.WithName("webhook")}); err != nil {
+				return fmt.Errorf("failed to register webhook: %w", err)
+			}
 
 			// Start all runnables and controller
 			return mgr.Start(ctx)

cmd/trust-manager/app/options/options.go

@@ -26,12 +26,13 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
-	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/client-go/rest"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/klog/v2"
 
 	"github.com/cert-manager/trust-manager/pkg/bundle"
+
+	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
 // Options is a struct to hold options for trust-manager

pkg/bundle/bundle.go

@@ -293,7 +293,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 				continue
 			}
 
-			if !metav1.IsControlledBy(&target, &bundle) {
+			if !metav1.IsControlledBy(&target, &bundle) /* #nosec G601 -- False positive. See https://github.com/golang/go/discussions/56010 */ {
 				targetLog.V(2).Info("skipping sync for target as it is not controlled by bundle")
 				continue
 			}

pkg/bundle/internal/ssa_client/bundle_status.go

@@ -19,10 +19,11 @@ package ssa_client
 import (
 	"encoding/json"
 
-	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/client-go/applyconfigurations/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 )
 
 type bundleStatusApplyConfiguration struct {

pkg/bundle/sync.go

@@ -26,6 +26,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/go-logr/logr"
+	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -42,8 +44,6 @@ import (
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/ssa_client"
 	"github.com/cert-manager/trust-manager/pkg/util"
-	"github.com/go-logr/logr"
-	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
 )
 
 const (
@@ -233,7 +233,7 @@ func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKey
 		if !ok {
 			return "", notFoundError{fmt.Errorf("no data found in Secret %s/%s at key %q", secret.Namespace, secret.Name, ref.Key)}
 		}
-		results.WriteString(string(data))
+		results.Write(data)
 		results.WriteByte('\n')
 	}
 	return results.String(), nil
@@ -753,7 +753,7 @@ func deduplicateBundles(bundles []string) ([]string, error) {
 
 	LOOP:
 		for {
-			block, certBytes = pem.Decode([]byte(certBytes))
+			block, certBytes = pem.Decode(certBytes)
 			if block == nil {
 				break LOOP
 			}

pkg/bundle/util.go

@@ -51,7 +51,7 @@ func (b *bundle) setBundleCondition(
 	existingConditions []trustapi.BundleCondition,
 	patchConditions *[]trustapi.BundleCondition,
 	newCondition trustapi.BundleCondition,
-) trustapi.BundleCondition {
+) trustapi.BundleCondition { // nolint:unparam
 	newCondition.LastTransitionTime = metav1.Time{Time: b.clock.Now()}
 
 	// Reset the LastTransitionTime if the status hasn't changed

pkg/util/cert_pool.go

@@ -38,14 +38,14 @@ func newCertPool(filterExpired bool) *certPool {
 }
 
 // Append certificate to a pool
-func (cp *certPool) appendCertFromPEM(PEMdata []byte) error {
-	if PEMdata == nil {
+func (cp *certPool) appendCertFromPEM(pemData []byte) error {
+	if pemData == nil {
 		return fmt.Errorf("certificate data can't be nil")
 	}
 
 	for {
 		var block *pem.Block
-		block, PEMdata = pem.Decode(PEMdata)
+		block, pemData = pem.Decode(pemData)
 
 		if block == nil {
 			break

pkg/util/pem.go

@@ -27,7 +27,7 @@ import (
 // only valid CERTIFICATE PEM blocks. If successful, returns the validated PEM blocks with any
 // comments or extra data stripped.
 
-// This validation is broadly similar to the standard library funtion
+// This validation is broadly similar to the standard library function
 // crypto/x509.CertPool.AppendCertsFromPEM - that is, we decode each PEM block at a time and parse
 // it as a certificate.
 

pkg/util/pem_test.go

@@ -205,6 +205,8 @@ gySihG5glByO5ZajFBNBIhjOF6+yfN1Bo5XjJ7bGwVIhGoRPHCtbvsnfuQ5ySz95
 CFD1BItRnQM=
 -----END CERTIFICATE-----`
 
+// #nosec G101 -- This is a test PK, ideally we would dynamically
+// generate this pair, but this should not be a security risk.
 const privateKey = `-----BEGIN EC PRIVATE KEY-----
 MHcCAQEEIHThSpdYMjW1k4K2r8RwhIGmknKrr0XKQLOJeL2fVoxToAoGCCqGSM49
 AwEHoUQDQgAEoMocv03WW/kCmyYM7CN7Ge7J5NOhJOKUYjF15NRBevWbxd8GYsvj

pkg/webhook/validation.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"sync"
 
 	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -34,14 +33,12 @@ import (
 // validator validates against trust.cert-manager.io resources.
 type validator struct {
 	log logr.Logger
-
-	lock sync.RWMutex
 }
 
 var _ admission.CustomValidator = &validator{}
 
 func (v *validator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
-	return v.validate(ctx, obj)
+	return v.validate(obj)
 }
 
 func (v *validator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
@@ -68,15 +65,15 @@ func (v *validator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.O
 		el = append(el, field.Invalid(path.Child("target", "secret"), "", "target secret removal is not allowed"))
 		return nil, el.ToAggregate()
 	}
-	return v.validate(ctx, newObj)
+	return v.validate(newObj)
 }
 
 func (v *validator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
 	// always allow deletes
 	return nil, nil
 }
 
-func (v *validator) validate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) {
 	bundle, ok := obj.(*trustapi.Bundle)
 	if !ok {
 		return nil, fmt.Errorf("expected a Bundle, but got a %T", obj)

pkg/webhook/validation_test.go

@@ -394,9 +394,9 @@ func Test_validate(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			log, ctx := ktesting.NewTestContext(t)
+			log, _ := ktesting.NewTestContext(t)
 			v := &validator{log: log}
-			gotWarnings, gotErr := v.validate(ctx, test.bundle)
+			gotWarnings, gotErr := v.validate(test.bundle)
 			if test.expErr == nil && gotErr != nil {
 				t.Errorf("got an unexpected error: %v", gotErr)
 			} else if test.expErr != nil && (gotErr == nil || *test.expErr != gotErr.Error()) {

pkg/webhook/webhook.go

@@ -35,13 +35,14 @@ type Options struct {
 func Register(mgr manager.Manager, opts Options) error {
 	opts.Log.Info("registering webhook endpoints")
 	validator := &validator{log: opts.Log.WithName("validation")}
-	err := builder.WebhookManagedBy(mgr).
+	if err := builder.WebhookManagedBy(mgr).
 		For(&trustapi.Bundle{}).
 		WithValidator(validator).
-		Complete()
-	if err != nil {
+		Complete(); err != nil {
 		return fmt.Errorf("error registering webhook: %v", err)
 	}
-	mgr.AddReadyzCheck("validator", mgr.GetWebhookServer().StartedChecker())
+	if err := mgr.AddReadyzCheck("validator", mgr.GetWebhookServer().StartedChecker()); err != nil {
+		return fmt.Errorf("error adding ready check: %v", err)
+	}
 	return nil
 }

test/dummy/certificates.go

@@ -78,6 +78,7 @@ MEUCIQCeN2/Z7jSJJK7m7kcZ/UgJIqbzKS1ktycUQ50+dhqNogIgaTYRjIxZFJ3u
 VhGzjAqH8YyuEObapwh4bTZkapwoDZQ=
 -----END CERTIFICATE-----`
 
+	// nolint: dupword
 	// NB: TestCertificate2 is expected to have the following properties:
 	// 1. Same Subject as TestCertificate1
 	// 2. Self signed (issuer == subject)

test/env/data.go

@@ -22,9 +22,6 @@ import (
 	"fmt"
 	"strings"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-
 	jks "github.com/pavlo-v-chernykh/keystore-go/v4"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -35,6 +32,9 @@ import (
 	"github.com/cert-manager/trust-manager/pkg/bundle"
 	"github.com/cert-manager/trust-manager/pkg/util"
 	"github.com/cert-manager/trust-manager/test/dummy"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 )
 
 const (
@@ -161,20 +161,21 @@ func checkBundleSyncedInternal(ctx context.Context, cl client.Client, bundleName
 	var bundle trustapi.Bundle
 	Expect(cl.Get(ctx, client.ObjectKey{Name: bundleName}, &bundle)).NotTo(HaveOccurred())
 
-	gotData := ""
-	if bundle.Spec.Target.ConfigMap != nil {
+	var gotData string
+	switch {
+	case bundle.Spec.Target.ConfigMap != nil:
 		var configMap corev1.ConfigMap
 		if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: bundle.Name}, &configMap); err != nil {
 			return fmt.Errorf("failed to get configMap %s/%s when checking bundle sync: %w", namespace, bundle.Name, err)
 		}
 		gotData = configMap.Data[bundle.Spec.Target.ConfigMap.Key]
-	} else if bundle.Spec.Target.Secret != nil {
+	case bundle.Spec.Target.Secret != nil:
 		var secret corev1.Secret
 		if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: bundle.Name}, &secret); err != nil {
 			return fmt.Errorf("failed to get secret %s/%s when checking bundle sync: %w", namespace, bundle.Name, err)
 		}
 		gotData = string(secret.Data[bundle.Spec.Target.Secret.Key])
-	} else {
+	default:
 		return fmt.Errorf("invalid bundle spec targets: %v", bundle.Spec.Target)
 	}
 
@@ -229,7 +230,7 @@ func CheckBundleSyncedStartsWith(ctx context.Context, cl client.Client, name str
 	})
 }
 
-func checkBundleSyncedAllNamespacesInternal(ctx context.Context, cl client.Client, bundleName string, checker func(namespace string) error) error {
+func checkBundleSyncedAllNamespacesInternal(ctx context.Context, cl client.Client, checker func(namespace string) error) error {
 	var namespaceList corev1.NamespaceList
 	if err := cl.List(ctx, &namespaceList); err != nil {
 		return fmt.Errorf("failed to list namespaces: %w", err)
@@ -257,14 +258,14 @@ func checkBundleSyncedAllNamespacesInternal(ctx context.Context, cl client.Clien
 
 // CheckBundleSyncedAllNamespaces calls CheckBundleSynced for all namespaces and returns an error if any of them failed
 func CheckBundleSyncedAllNamespaces(ctx context.Context, cl client.Client, name string, expectedData string) error {
-	return checkBundleSyncedAllNamespacesInternal(ctx, cl, name, func(namespace string) error {
+	return checkBundleSyncedAllNamespacesInternal(ctx, cl, func(namespace string) error {
 		return CheckBundleSynced(ctx, cl, name, namespace, expectedData)
 	})
 }
 
 // CheckBundleSyncedAllNamespacesStartsWith calls CheckBundleSyncedStartsWith for all namespaces and returns an error if any of them failed
 func CheckBundleSyncedAllNamespacesStartsWith(ctx context.Context, cl client.Client, name string, startingData string) error {
-	return checkBundleSyncedAllNamespacesInternal(ctx, cl, name, func(namespace string) error {
+	return checkBundleSyncedAllNamespacesInternal(ctx, cl, func(namespace string) error {
 		return CheckBundleSyncedStartsWith(ctx, cl, name, namespace, startingData)
 	})
 }

test/env/ginkgo.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
-
 	"github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/util/wait"
 )

test/integration/bundle/integration.go

@@ -19,15 +19,15 @@ package test
 import (
 	"os"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
 	"k8s.io/utils/ptr"
-
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 )
 
 var (

test/integration/bundle/suite.go

@@ -23,10 +23,6 @@ import (
 	"os"
 	"time"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-	. "github.com/onsi/gomega/gstruct"
-
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -45,6 +41,10 @@ import (
 	"github.com/cert-manager/trust-manager/pkg/fspkg"
 	"github.com/cert-manager/trust-manager/test/dummy"
 	testenv "github.com/cert-manager/trust-manager/test/env"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gstruct"
 )
 
 const (

test/smoke/smoke_test.go

@@ -20,11 +20,11 @@ import (
 	"flag"
 	"testing"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-
 	"github.com/cert-manager/trust-manager/test/env"
 	"github.com/cert-manager/trust-manager/test/smoke/config"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
 )
 
 var (