Merge pull request #6972 from inteon/fix_linters

Fix linters: misspell, gocritic, bodyclose, unused, unconvert, gosimple, ginkgolinter and wastedassign

.golangci.yaml

@@ -3,33 +3,25 @@ issues:
     - linters:
         - dogsled
         - errcheck
-        - misspell
         - contextcheck
         - unparam
         - promlinter
         - errname
         - tenv
         - exhaustive
-        - gocritic
         - nilerr
-        - bodyclose
         - loggercheck
         - forbidigo
         - interfacebloat
         - predeclared
-        - unused
-        - unconvert
         - usestdlibvars
         - noctx
         - nilnil
-        - gosimple
         - nakedret
         - asasalint
-        - ginkgolinter
         - goprintffuncname
         - ineffassign
         - musttag
-        - wastedassign
         - nosprintfhostport
         - exportloopref
         - gomoddirectives

cmd/controller/app/options/options_test.go

@@ -22,7 +22,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/sets"
 
-	//"github.com/cert-manager/cert-manager/controller-binary/app/options"
 	config "github.com/cert-manager/cert-manager/internal/apis/config/controller"
 	defaults "github.com/cert-manager/cert-manager/internal/apis/config/controller/v1alpha1"
 	"github.com/cert-manager/cert-manager/internal/apis/config/controller/validation"

cmd/controller/app/start.go

@@ -53,9 +53,7 @@ const componentController = "controller"
 func NewServerCommand(ctx context.Context) *cobra.Command {
 	return newServerCommand(
 		ctx,
-		func(ctx context.Context, cfg *config.ControllerConfiguration) error {
-			return Run(ctx, cfg)
-		},
+		Run,
 		os.Args[1:],
 	)
 }

hack/extractcrd/main.go

@@ -80,8 +80,8 @@ func main() {
 			continue
 		}
 
-		doc = string(strings.TrimPrefix(doc, "---"))
-		doc = string(strings.TrimSpace(doc))
+		doc = strings.TrimPrefix(doc, "---")
+		doc = strings.TrimSpace(doc)
 
 		if wantedCRDName == nil {
 			if foundAny {

internal/apis/certmanager/validation/certificate.go

@@ -254,7 +254,7 @@ func validateIssuerRef(issuerRef cmmeta.ObjectReference, fldPath *field.Path) fi
 }
 
 func validateIPAddresses(a *internalcmapi.CertificateSpec, fldPath *field.Path) field.ErrorList {
-	if len(a.IPAddresses) <= 0 {
+	if len(a.IPAddresses) == 0 {
 		return nil
 	}
 	el := field.ErrorList{}
@@ -268,7 +268,7 @@ func validateIPAddresses(a *internalcmapi.CertificateSpec, fldPath *field.Path)
 }
 
 func validateEmailAddresses(a *internalcmapi.CertificateSpec, fldPath *field.Path) field.ErrorList {
-	if len(a.EmailAddresses) <= 0 {
+	if len(a.EmailAddresses) == 0 {
 		return nil
 	}
 	el := field.ErrorList{}

internal/apis/certmanager/validation/issuer.go

@@ -443,13 +443,10 @@ func ValidateACMEChallengeSolverDNS01(p *cmacme.ACMEChallengeSolverDNS01, fldPat
 				if p.AzureDNS.ManagedIdentity != nil {
 					el = append(el, field.Forbidden(fldPath.Child("azureDNS", "managedIdentity"), "managed identity can not be used at the same time as clientID, clientSecretSecretRef or tenantID"))
 				}
-			} else {
-				// using managed identity
-				if p.AzureDNS.ManagedIdentity != nil && len(p.AzureDNS.ManagedIdentity.ClientID) > 0 && len(p.AzureDNS.ManagedIdentity.ResourceID) > 0 {
-					el = append(el, field.Forbidden(fldPath.Child("azureDNS", "managedIdentity"), "managedIdentityClientID and managedIdentityResourceID cannot both be specified"))
-				}
-
+			} else if p.AzureDNS.ManagedIdentity != nil && len(p.AzureDNS.ManagedIdentity.ClientID) > 0 && len(p.AzureDNS.ManagedIdentity.ResourceID) > 0 {
+				el = append(el, field.Forbidden(fldPath.Child("azureDNS", "managedIdentity"), "managedIdentityClientID and managedIdentityResourceID cannot both be specified"))
 			}
+
 			// SubscriptionID must always be defined
 			if len(p.AzureDNS.SubscriptionID) == 0 {
 				el = append(el, field.Required(fldPath.Child("azureDNS", "subscriptionID"), ""))
@@ -569,7 +566,7 @@ func ValidateACMEChallengeSolverDNS01(p *cmacme.ACMEChallengeSolverDNS01, fldPat
 			}
 
 			if len(ValidateSecretKeySelector(&p.RFC2136.TSIGSecret, fldPath.Child("rfc2136", "tsigSecretSecretRef"))) == 0 {
-				if len(p.RFC2136.TSIGKeyName) <= 0 {
+				if len(p.RFC2136.TSIGKeyName) == 0 {
 					el = append(el, field.Required(fldPath.Child("rfc2136", "tsigKeyName"), ""))
 				}
 

internal/apis/certmanager/validation/issuer_test.go

@@ -47,6 +47,7 @@ var (
 		Key: "validkey",
 	}
 	// TODO (JS): Missing test for validCloudflareProvider
+	// nolint: unused
 	validCloudflareProvider = cmacme.ACMEIssuerDNS01ProviderCloudflare{
 		APIKey: &validSecretKeyRef,
 		Email:  "valid",

internal/controller/certificates/policies/checks.go

@@ -277,7 +277,7 @@ func CurrentCertificateNearingExpiry(c clock.Clock) Func {
 
 		renewIn := renewalTime.Time.Sub(c.Now())
 		if renewIn > 0 {
-			//renewal time is in future, no need to renew
+			// renewal time is in future, no need to renew
 			return "", "", false
 		}
 

internal/vault/vault.go

@@ -457,7 +457,8 @@ func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1
 		}
 		defaultAudience += v.issuer.GetName()
 
-		audiences := append(kubernetesAuth.ServiceAccountRef.TokenAudiences, defaultAudience)
+		audiences := append([]string(nil), kubernetesAuth.ServiceAccountRef.TokenAudiences...)
+		audiences = append(audiences, defaultAudience)
 
 		tokenrequest, err := v.createToken(context.Background(), kubernetesAuth.ServiceAccountRef.Name, &authv1.TokenRequest{
 			Spec: authv1.TokenRequestSpec{
@@ -472,7 +473,7 @@ func (v *Vault) requestTokenWithKubernetesAuth(client Client, kubernetesAuth *v1
 				// Vault backend can bind the kubernetes auth backend role to the service account and specific namespace of the service account.
 				// Providing additional audiences is not considered a major non-mitigatable security risk
 				// as if someone creates an Issuer in another namespace/globally with the same audiences
-				// in attempt to highjack the certificate vault (if role config mandates sa:namespace) won't authorise the conneciton
+				// in attempt to highjack the certificate vault (if role config mandates sa:namespace) won't authorise the connection
 				// as token subject won't match vault role requirement to have SA originated from the specific namespace.
 				Audiences: audiences,
 

internal/vault/vault_test.go

@@ -1097,10 +1097,10 @@ type testNewConfigT struct {
 func TestNewConfig(t *testing.T) {
 	caBundleSecretRefFakeSecretLister := func(namespace, secret, key, cert string) *listers.FakeSecretLister {
 		return listers.FakeSecretListerFrom(listers.NewFakeSecretLister(), func(f *listers.FakeSecretLister) {
-			f.SecretsFn = func(namespace string) clientcorev1.SecretNamespaceLister {
+			f.SecretsFn = func(listerNamespace string) clientcorev1.SecretNamespaceLister {
 				return listers.FakeSecretNamespaceListerFrom(listers.NewFakeSecretNamespaceLister(), func(fn *listers.FakeSecretNamespaceLister) {
 					fn.GetFn = func(name string) (*corev1.Secret, error) {
-						if name == secret && namespace == namespace {
+						if name == secret && listerNamespace == namespace {
 							return &corev1.Secret{
 								Data: map[string][]byte{
 									key: []byte(cert),
@@ -1114,10 +1114,10 @@ func TestNewConfig(t *testing.T) {
 	}
 	clientCertificateSecretRefFakeSecretLister := func(namespace, secret, caKey, caCert, clientKey, clientCert, privateKey, privateKeyCert string) *listers.FakeSecretLister {
 		return listers.FakeSecretListerFrom(listers.NewFakeSecretLister(), func(f *listers.FakeSecretLister) {
-			f.SecretsFn = func(namespace string) clientcorev1.SecretNamespaceLister {
+			f.SecretsFn = func(listerNamespace string) clientcorev1.SecretNamespaceLister {
 				return listers.FakeSecretNamespaceListerFrom(listers.NewFakeSecretNamespaceLister(), func(fn *listers.FakeSecretNamespaceLister) {
 					fn.GetFn = func(name string) (*corev1.Secret, error) {
-						if name == secret && namespace == namespace {
+						if name == secret && listerNamespace == namespace {
 							return &corev1.Secret{
 								Data: map[string][]byte{
 									caKey:      []byte(caCert),

make/config/samplewebhook/sample/main.go

@@ -56,7 +56,7 @@ type customDNSProviderSolver struct {
 	// 3. uncomment the relevant code in the Initialize method below
 	// 4. ensure your webhook's service account has the required RBAC role
 	//    assigned to it for interacting with the Kubernetes APIs you need.
-	//client kubernetes.Clientset
+	// client kubernetes.Clientset
 }
 
 // customDNSProviderConfig is a structure that is used to decode into when
@@ -79,8 +79,8 @@ type customDNSProviderConfig struct {
 	// These fields will be set by users in the
 	// `issuer.spec.acme.dns01.providers.webhook.config` field.
 
-	//Email           string `json:"email"`
-	//APIKeySecretRef cmmeta.SecretKeySelector `json:"apiKeySecretRef"`
+	// Email           string `json:"email"`
+	// APIKeySecretRef cmmeta.SecretKeySelector `json:"apiKeySecretRef"`
 }
 
 // Name is used as the name for this DNS solver when referencing it on the ACME
@@ -135,12 +135,12 @@ func (c *customDNSProviderSolver) Initialize(kubeClientConfig *rest.Config, stop
 	///// UNCOMMENT THE BELOW CODE TO MAKE A KUBERNETES CLIENTSET AVAILABLE TO
 	///// YOUR CUSTOM DNS PROVIDER
 
-	//cl, err := kubernetes.NewForConfig(kubeClientConfig)
-	//if err != nil {
-	//	return err
-	//}
+	// cl, err := kubernetes.NewForConfig(kubeClientConfig)
+	// if err != nil {
+	// 	return err
+	// }
 	//
-	//c.client = cl
+	// c.client = cl
 
 	///// END OF CODE TO MAKE KUBERNETES CLIENTSET AVAILABLE
 	return nil

pkg/acme/util.go

@@ -28,8 +28,7 @@ import (
 // The 'valid' state is a special case, as it is a final state for Challenges but
 // not for Orders.
 func IsFinalState(s cmacme.State) bool {
-	switch s {
-	case cmacme.Valid:
+	if s == cmacme.Valid {
 		return true
 	}
 	return IsFailureState(s)

pkg/controller/acmechallenges/controller_test.go

@@ -29,11 +29,6 @@ import (
 	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
 
-const (
-	randomFinalizer         = "random.acme.cert-manager.io"
-	maxConcurrentChallenges = 60
-)
-
 func TestRunScheduler(t *testing.T) {
 	tests := map[string]struct {
 		maxConcurrentChallenges int

pkg/controller/acmechallenges/sync.go

@@ -222,18 +222,19 @@ func handleError(ch *cmacme.Challenge, err error) error {
 	if acmeErr, ok = err.(*acmeapi.Error); !ok {
 		return err
 	}
-	switch acmeErr.ProblemType {
+
 	// This response type is returned when an authorization has expired or the
 	// request is in some way malformed.
 	// In this case, we should mark the challenge as expired so that the order
 	// can be retried.
 	// TODO: don't mark *all* malformed errors as expired, we may be able to be
 	// more informative to the user by further inspecting the Error response.
-	case "urn:ietf:params:acme:error:malformed":
+	if acmeErr.ProblemType == "urn:ietf:params:acme:error:malformed" {
 		ch.Status.State = cmacme.Expired
 		// absorb the error as updating the challenge's status will trigger a sync
 		return nil
 	}
+
 	if acmeErr.StatusCode >= 400 && acmeErr.StatusCode < 500 {
 		ch.Status.State = cmacme.Errored
 		ch.Status.Reason = fmt.Sprintf("Failed to retrieve Order resource: %v", err)

pkg/controller/acmeorders/sync.go

@@ -51,7 +51,7 @@ const (
 
 var (
 	// RequeuePeriod is the default period after which an Order should be re-queued.
-	// It can be overriden in tests.
+	// It can be overridden in tests.
 	RequeuePeriod = time.Second * 5
 )
 

pkg/controller/certificate-shim/gateways/controller.go

@@ -19,7 +19,6 @@ package controller
 import (
 	"context"
 	"fmt"
-	"time"
 
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -36,12 +35,6 @@ import (
 
 const (
 	ControllerName = "gateway-shim"
-
-	// resyncPeriod is set to 10 hours across cert-manager. These 10 hours come
-	// from a discussion on the controller-runtime project that boils down to:
-	// never change this without an explicit reason.
-	// https://github.com/kubernetes-sigs/controller-runtime/pull/88#issuecomment-408500629
-	resyncPeriod = 10 * time.Hour
 )
 
 type controller struct {

pkg/controller/certificate-shim/sync.go

@@ -92,8 +92,7 @@ func SyncFnFor(
 		// "kubernetes.io/tls-acme" annotation are only enabled for the Ingress
 		// resource.
 		var autoAnnotations []string
-		switch ingLike.(type) {
-		case *networkingv1.Ingress:
+		if _, ok := ingLike.(*networkingv1.Ingress); ok {
 			autoAnnotations = defaults.DefaultAutoCertificateAnnotations
 		}
 
@@ -284,11 +283,9 @@ func validateGatewayListenerBlock(path *field.Path, l gwapi.Listener, ingLike me
 	if l.TLS.Mode == nil {
 		errs = append(errs, field.Required(path.Child("tls").Child("mode"),
 			"the mode field is required"))
-	} else {
-		if *l.TLS.Mode != gwapi.TLSModeTerminate {
-			errs = append(errs, field.NotSupported(path.Child("tls").Child("mode"),
-				*l.TLS.Mode, []string{string(gwapi.TLSModeTerminate)}))
-		}
+	} else if *l.TLS.Mode != gwapi.TLSModeTerminate {
+		errs = append(errs, field.NotSupported(path.Child("tls").Child("mode"),
+			*l.TLS.Mode, []string{string(gwapi.TLSModeTerminate)}))
 	}
 
 	return errs

pkg/controller/certificate-shim/sync_test.go

@@ -19,7 +19,6 @@ package shimhelper
 import (
 	"context"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/go-logr/logr"
@@ -3264,17 +3263,6 @@ func TestSync(t *testing.T) {
 
 }
 
-type fakeHelper struct {
-	issuer cmapi.GenericIssuer
-}
-
-func (f *fakeHelper) GetGenericIssuer(ref cmmeta.ObjectReference, ns string) (cmapi.GenericIssuer, error) {
-	if f.issuer == nil {
-		return nil, fmt.Errorf("no issuer specified on fake helper")
-	}
-	return f.issuer, nil
-}
-
 func TestIssuerForIngress(t *testing.T) {
 	type testT struct {
 		Ingress       *networkingv1.Ingress

pkg/controller/certificaterequests/ca/ca.go

@@ -66,13 +66,11 @@ func init() {
 
 func NewCA(ctx *controllerpkg.Context) certificaterequests.Issuer {
 	return &CA{
-		issuerOptions: ctx.IssuerOptions,
-		secretsLister: ctx.KubeSharedInformerFactory.Secrets().Lister(),
-		reporter:      crutil.NewReporter(ctx.Clock, ctx.Recorder),
-		templateGenerator: func(cr *cmapi.CertificateRequest) (*x509.Certificate, error) {
-			return pki.CertificateTemplateFromCertificateRequest(cr)
-		},
-		signingFn: pki.SignCSRTemplate,
+		issuerOptions:     ctx.IssuerOptions,
+		secretsLister:     ctx.KubeSharedInformerFactory.Secrets().Lister(),
+		reporter:          crutil.NewReporter(ctx.Clock, ctx.Recorder),
+		templateGenerator: pki.CertificateTemplateFromCertificateRequest,
+		signingFn:         pki.SignCSRTemplate,
 	}
 }
 

pkg/controller/certificaterequests/controller.go

@@ -88,8 +88,8 @@ type Controller struct {
 	issuerLister        cmlisters.IssuerLister
 	clusterIssuerLister cmlisters.ClusterIssuerLister
 
-	//registerExtraInformers is a list of functions that CertificateRequest
-	//controllers can use to register custom informers.
+	// registerExtraInformers is a list of functions that CertificateRequest
+	// controllers can use to register custom informers.
 	registerExtraInformers []RegisterExtraInformerFn
 
 	// Issuer to call sign function

pkg/controller/certificates/issuing/issuing_controller.go

@@ -403,7 +403,7 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt
 		return err
 	}
 
-	//Set status.revision to revision of the CertificateRequest
+	// Set status.revision to revision of the CertificateRequest
 	crt.Status.Revision = &nextRevision
 
 	// Remove Issuing status condition

pkg/controller/certificates/issuing/secret_manager.go

@@ -55,7 +55,7 @@ func (c *controller) ensureSecretData(ctx context.Context, log logr.Logger, crt
 	log = log.WithValues("secret", secret.Name)
 
 	// If there is no certificate or private key data available at the target
-	// Secret then exit early. The absense of these keys should cause an issuance
+	// Secret then exit early. The absence of these keys should cause an issuance
 	// of the Certificate, so there is no need to run post issuance checks.
 	if secret.Data == nil ||
 		len(secret.Data[corev1.TLSCertKey]) == 0 ||
@@ -85,11 +85,11 @@ func (c *controller) ensureSecretData(ctx context.Context, log logr.Logger, crt
 	if isViolation {
 		switch reason {
 		case policies.InvalidCertificate, policies.ManagedFieldsParseError:
-			//An error here indicates that the managed fields are malformed and the
-			//decoder doesn't understand the managed fields on the Secret, or the
-			//signed certificate data could not be decoded. There is nothing more the
-			//controller can do here, so we exit nil so this controller doesn't end in
-			//an infinite loop.
+			// An error here indicates that the managed fields are malformed and the
+			// decoder doesn't understand the managed fields on the Secret, or the
+			// signed certificate data could not be decoded. There is nothing more the
+			// controller can do here, so we exit nil so this controller doesn't end in
+			// an infinite loop.
 			log.Error(errors.New(message), "failed to determine whether the SecretTemplate matches Secret")
 			return nil
 		default: