Merge pull request #4037 from SgtCoDFish/staticparty-ash

Static Analysis Fixes from Static Analysis Party

pkg/controller/acmeorders/util.go

@@ -18,9 +18,7 @@ package acmeorders
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
-	"hash/fnv"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -81,21 +79,6 @@ func buildChallenge(ctx context.Context, cl acmecl.Interface, issuer cmapi.Gener
 	}, nil
 }
 
-func hashChallenge(spec cmacme.ChallengeSpec) (uint32, error) {
-	specBytes, err := json.Marshal(spec)
-	if err != nil {
-		return 0, err
-	}
-
-	hashF := fnv.New32()
-	_, err = hashF.Write(specBytes)
-	if err != nil {
-		return 0, err
-	}
-
-	return hashF.Sum32(), nil
-}
-
 func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, issuer cmapi.GenericIssuer, o *cmacme.Order, authz cmacme.ACMEAuthorization) (*cmacme.ChallengeSpec, error) {
 	log := logf.FromContext(ctx, "challengeSpecForAuthorization")
 	dbg := log.V(logf.DebugLevel)

pkg/controller/certificaterequests/approver/approver.go

@@ -69,7 +69,7 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin
 	c.queue = workqueue.NewNamedRateLimitingQueue(controllerpkg.DefaultItemBasedRateLimiter(), ControllerName)
 
 	certificateRequestInformer := ctx.SharedInformerFactory.Certmanager().V1().CertificateRequests()
-	mustSync := append([]cache.InformerSynced{certificateRequestInformer.Informer().HasSynced})
+	mustSync := []cache.InformerSynced{certificateRequestInformer.Informer().HasSynced}
 	certificateRequestInformer.Informer().AddEventHandler(&controllerpkg.QueuingEventHandler{Queue: c.queue})
 
 	c.certificateRequestLister = certificateRequestInformer.Lister()

pkg/controller/certificaterequests/vault/vault_test.go

@@ -532,7 +532,10 @@ func runTest(t *testing.T, test testT) {
 	}
 
 	controller := certificaterequests.New(apiutil.IssuerVault, vault)
-	controller.Register(test.builder.Context)
+	if _, _, err := controller.Register(test.builder.Context); err != nil {
+		t.Errorf("failed to register context with controller: %v", err)
+	}
+
 	test.builder.Start()
 
 	err := controller.Sync(context.Background(), test.certificateRequest)

pkg/controller/certificates/trigger/policies/policies.go

@@ -54,7 +54,7 @@ type Input struct {
 // in the 'reason' and 'message' return parameters if so.
 type Func func(Input) (reason, message string, reissue bool)
 
-// A chain of PolicyFuncs to be evaluated in order.
+// A Chain of PolicyFuncs to be evaluated in order.
 type Chain []Func
 
 // Evaluate will evaluate the entire policy chain using the provided input.
@@ -118,7 +118,7 @@ func SecretPublicKeysDiffer(input Input) (string, string, bool) {
 
 func SecretPrivateKeyMatchesSpec(input Input) (string, string, bool) {
 	if input.Secret.Data == nil || len(input.Secret.Data[corev1.TLSPrivateKeyKey]) == 0 {
-		return SecretMismatch, fmt.Sprintf("Existing issued Secret does not contain private key data"), true
+		return SecretMismatch, "Existing issued Secret does not contain private key data", true
 	}
 
 	pkBytes := input.Secret.Data[corev1.TLSPrivateKeyKey]

pkg/controller/clusterissuers/sync_test.go

@@ -52,7 +52,10 @@ func TestUpdateIssuerStatus(t *testing.T) {
 	defer b.Stop()
 
 	c := &controller{}
-	c.Register(b.Context)
+	if _, _, err := c.Register(b.Context); err != nil {
+		t.Errorf("failed to register context against controller: %v", err)
+		return
+	}
 	b.Start()
 
 	fakeClient := b.FakeCMClient()

pkg/controller/ingress-shim/sync.go

@@ -33,7 +33,6 @@ import (
 	cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1"
 	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
-	"github.com/jetstack/cert-manager/pkg/logs"
 	logf "github.com/jetstack/cert-manager/pkg/logs"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
@@ -143,7 +142,7 @@ func validateIngressTLSBlock(tlsBlock networkingv1beta1.IngressTLS) []error {
 
 func (c *controller) buildCertificates(ctx context.Context, ing *networkingv1beta1.Ingress,
 	issuerName, issuerKind, issuerGroup string) (new, update []*cmapi.Certificate, _ error) {
-	log := logs.FromContext(ctx)
+	log := logf.FromContext(ctx)
 
 	var newCrts []*cmapi.Certificate
 	var updateCrts []*cmapi.Certificate
@@ -187,7 +186,7 @@ func (c *controller) buildCertificates(ctx context.Context, ing *networkingv1bet
 		// check if a Certificate for this TLS entry already exists, and if it
 		// does then skip this entry
 		if existingCrt != nil {
-			log := logs.WithRelatedResource(log, existingCrt)
+			log := logf.WithRelatedResource(log, existingCrt)
 			log.V(logf.DebugLevel).Info("certificate already exists for ingress resource, ensuring it is up to date")
 
 			if metav1.GetControllerOf(existingCrt) == nil {
@@ -298,7 +297,7 @@ func setIssuerSpecificConfig(crt *cmapi.Certificate, ing *networkingv1beta1.Ingr
 	}
 
 	// for ACME issuers
-	editInPlaceVal, _ := ingAnnotations[cmacme.IngressEditInPlaceAnnotationKey]
+	editInPlaceVal := ingAnnotations[cmacme.IngressEditInPlaceAnnotationKey]
 	editInPlace := editInPlaceVal == "true"
 	if editInPlace {
 		if crt.Annotations == nil {

pkg/controller/test/context_builder.go

@@ -45,8 +45,8 @@ import (
 
 func init() {
 	logs.InitLogs(nil)
-	flag.Set("alsologtostderr", fmt.Sprintf("%t", true))
-	flag.Lookup("v").Value.Set("4")
+	_ = flag.Set("alsologtostderr", fmt.Sprintf("%t", true))
+	_ = flag.Lookup("v").Value.Set("4")
 }
 
 // Builder is a structure used to construct new Contexts for use during tests.

pkg/ctl/scheme.go

@@ -14,9 +14,10 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// This package was created to have a scheme that has the internal cert-manager types,
+// Package ctl was created to have a scheme that has the internal cert-manager types,
 // and their conversion functions as well as the List object type registered, which is needed for ctl command like
 // `convert` or `create certificaterequest`.
+
 package ctl
 
 import (
@@ -58,13 +59,13 @@ func init() {
 	utilruntime.Must(metainternalversion.AddToScheme(Scheme))
 
 	// Adds the conversion between internalmeta.List and corev1.List
-	Scheme.AddConversionFunc((*corev1.List)(nil), (*metainternalversion.List)(nil), func(a, b interface{}, scope conversion.Scope) error {
+	_ = Scheme.AddConversionFunc((*corev1.List)(nil), (*metainternalversion.List)(nil), func(a, b interface{}, scope conversion.Scope) error {
 		metaList := &metav1.List{}
 		metaList.Items = a.(*corev1.List).Items
 		return metainternalversion.Convert_v1_List_To_internalversion_List(metaList, b.(*metainternalversion.List), scope)
 	})
 
-	Scheme.AddConversionFunc((*metainternalversion.List)(nil), (*corev1.List)(nil), func(a, b interface{}, scope conversion.Scope) error {
+	_ = Scheme.AddConversionFunc((*metainternalversion.List)(nil), (*corev1.List)(nil), func(a, b interface{}, scope conversion.Scope) error {
 		metaList := &metav1.List{}
 		err := metainternalversion.Convert_internalversion_List_To_v1_List(a.(*metainternalversion.List), metaList, scope)
 		if err != nil {

pkg/internal/apis/certmanager/validation/certificate_for_issuer_test.go

@@ -31,7 +31,6 @@ import (
 
 const (
 	defaultTestIssuerName = "test-issuer"
-	defaultTestCrtName    = "test-crt"
 	defaultTestNamespace  = gen.DefaultTestNamespace
 )
 

pkg/issuer/acme/dns/acmedns/acmedns.go

@@ -40,19 +40,19 @@ type DNSProvider struct {
 // Credentials and acme-dns server host are given in environment variables
 func NewDNSProvider(dns01Nameservers []string) (*DNSProvider, error) {
 	host := os.Getenv("ACME_DNS_HOST")
-	accountJson := os.Getenv("ACME_DNS_ACCOUNT_JSON")
-	return NewDNSProviderHostBytes(host, []byte(accountJson), dns01Nameservers)
+	accountJSON := os.Getenv("ACME_DNS_ACCOUNT_JSON")
+	return NewDNSProviderHostBytes(host, []byte(accountJSON), dns01Nameservers)
 }
 
 // NewDNSProviderHostBytes returns a DNSProvider instance configured for ACME DNS
 // acme-dns server host is given in a string
 // credentials are stored in json in the given string
-func NewDNSProviderHostBytes(host string, accountJson []byte, dns01Nameservers []string) (*DNSProvider, error) {
+func NewDNSProviderHostBytes(host string, accountJSON []byte, dns01Nameservers []string) (*DNSProvider, error) {
 	client := goacmedns.NewClient(host)
 
 	var accounts map[string]goacmedns.Account
-	if err := json.Unmarshal(accountJson, &accounts); err != nil {
-		return nil, fmt.Errorf("Error unmarshalling accountJson: %s", err)
+	if err := json.Unmarshal(accountJSON, &accounts); err != nil {
+		return nil, fmt.Errorf("Error unmarshalling accountJSON: %s", err)
 	}
 
 	return &DNSProvider{

pkg/issuer/acme/dns/rfc2136/provider_test.go

@@ -39,7 +39,11 @@ func TestRunSuiteWithTSIG(t *testing.T) {
 	if err := server.Run(ctx); err != nil {
 		t.Fatalf("failed to start test server: %v", err)
 	}
-	defer server.Shutdown()
+	defer func() {
+		if err := server.Shutdown(); err != nil {
+			t.Errorf("failed to gracefully shut down test server: %v", err)
+		}
+	}()
 
 	var validConfig = cmacme.ACMEIssuerDNS01ProviderRFC2136{
 		Nameserver: server.ListenAddr(),
@@ -74,7 +78,11 @@ func TestRunSuiteNoTSIG(t *testing.T) {
 	if err := server.Run(ctx); err != nil {
 		t.Fatalf("failed to start test server: %v", err)
 	}
-	defer server.Shutdown()
+	defer func() {
+		if err := server.Shutdown(); err != nil {
+			t.Errorf("failed to gracefully shut down test server: %v", err)
+		}
+	}()
 
 	var validConfig = cmacme.ACMEIssuerDNS01ProviderRFC2136{
 		Nameserver: server.ListenAddr(),

pkg/issuer/vault/setup.go

@@ -34,7 +34,6 @@ const (
 	errorVault = "VaultError"
 
 	messageVaultClientInitFailed         = "Failed to initialize Vault client: "
-	messageVaultHealthCheckFailed        = "Failed to call Vault health check: "
 	messageVaultStatusVerificationFailed = "Vault is not initialized or is sealed"
 	messageVaultConfigRequired           = "Vault config cannot be empty"
 	messageServerAndPathRequired         = "Vault server and path are required fields"

pkg/issuer/venafi/client/request.go

@@ -140,7 +140,6 @@ func convertCustomFieldsToVcert(customFields []api.CustomField) ([]certificate.C
 			switch field.Type {
 			case api.CustomFieldTypePlain, "":
 				fieldType = certificate.CustomFieldPlain
-				break
 			default:
 				return nil, ErrCustomFieldsType{Type: field.Type}
 			}

pkg/logs/logs.go

@@ -65,7 +65,7 @@ func InitLogs(fs *flag.FlagSet) {
 		fs = flag.CommandLine
 	}
 	klog.InitFlags(fs)
-	fs.Set("logtostderr", "true")
+	_ = fs.Set("logtostderr", "true")
 
 	log.SetOutput(GlogWriter{})
 	log.SetFlags(0)

pkg/webhook/server/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
         "@io_k8s_apimachinery//pkg/runtime:go_default_library",
         "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library",
         "@io_k8s_apimachinery//pkg/runtime/serializer/json:go_default_library",
+        "@io_k8s_apimachinery//pkg/util/runtime:go_default_library",
         "@io_k8s_component_base//cli/flag:go_default_library",
         "@io_k8s_sigs_controller_runtime//pkg/log:go_default_library",
     ],

pkg/webhook/server/server.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
+	runtimeutil "k8s.io/apimachinery/pkg/util/runtime"
 	ciphers "k8s.io/component-base/cli/flag"
 	crlog "sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -55,8 +56,9 @@ var (
 
 func init() {
 	apiextensionsinstall.Install(defaultScheme)
-	admissionv1beta1.AddToScheme(defaultScheme)
-	admissionv1.AddToScheme(defaultScheme)
+
+	runtimeutil.Must(admissionv1beta1.AddToScheme(defaultScheme))
+	runtimeutil.Must(admissionv1.AddToScheme(defaultScheme))
 
 	// we need to add the options to empty v1
 	// TODO fix the server code to avoid this
@@ -207,6 +209,7 @@ func (s *Server) Run(stopCh <-chan struct{}) error {
 
 	s.Log.V(logf.DebugLevel).Info("waiting for server to shutdown")
 	waitForAll(healthzChan, certSourceChan, listenerChan)
+
 	s.Log.V(logf.InfoLevel).Info("server shutdown successfully")
 
 	return err
@@ -377,7 +380,6 @@ func (s *Server) handle(inner handleFunc) func(w http.ResponseWriter, req *http.
 		codec := json.NewSerializerWithOptions(json.DefaultMetaFactory, s.scheme(), s.scheme(), json.SerializerOptions{
 			Pretty: true,
 		})
-		codec.Decode(data, nil, nil)
 		obj, _, err := codec.Decode(data, nil, nil)
 		if err != nil {
 			s.Log.Error(err, "failed to decode request body")