Merge pull request #6973 from inteon/fix_linters

Fix linters: unparam, tenv, loggercheck, forbidigo, predeclared, usestdlibvars, asasalint, goprintffuncname, ineffassign, nosprintfhostport and exportloopref

.golangci.yaml

@@ -4,26 +4,15 @@ issues:
         - dogsled
         - errcheck
         - contextcheck
-        - unparam
         - promlinter
         - errname
-        - tenv
         - exhaustive
         - nilerr
-        - loggercheck
-        - forbidigo
         - interfacebloat
-        - predeclared
-        - usestdlibvars
         - noctx
         - nilnil
         - nakedret
-        - asasalint
-        - goprintffuncname
-        - ineffassign
         - musttag
-        - nosprintfhostport
-        - exportloopref
         - gomoddirectives
       text: ".*"
     - linters:

cmd/cainjector/app/controller.go

@@ -173,7 +173,7 @@ func Run(opts *config.CAInjectorConfiguration, ctx context.Context) error {
 
 	err = cainjector.RegisterAllInjectors(ctx, mgr, setupOptions)
 	if err != nil {
-		log.Error(err, "failed to register controllers", err)
+		log.Error(err, "failed to register controllers")
 		return err
 	}
 

hack/extractcrd/main.go

@@ -61,6 +61,8 @@ func main() {
 		os.Exit(1)
 	}
 
+	outWriter := os.Stdout
+
 	docs := docSeparatorRegexp.Split(string(rawYAMLBytes), -1)
 
 	decoder := crdDecoder()
@@ -85,15 +87,15 @@ func main() {
 
 		if wantedCRDName == nil {
 			if foundAny {
-				fmt.Println("---")
+				fmt.Fprintln(outWriter, "---")
 			}
-			fmt.Println(doc)
+			fmt.Fprintln(outWriter, doc)
 			foundAny = true
 			continue
 		} else {
 			crdName := strings.ToLower(crd.Spec.Names.Plural)
 			if crdName == *wantedCRDName {
-				fmt.Println(doc)
+				fmt.Fprintln(outWriter, doc)
 				return
 			}
 		}

hack/prune-junit-xml/prunexml.go

@@ -37,6 +37,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"regexp"
 	"strconv"
@@ -92,12 +93,14 @@ type JUnitFailure struct {
 var fuzzNameRegex = regexp.MustCompile(`^(.*)\/fuzz_\d+$`)
 
 func main() {
+	logger := log.New(os.Stderr, "", 0)
+
 	maxTextSize := flag.Int("max-text-size", 1, "maximum size of attribute or text (in MB)")
 	flag.Parse()
 
 	if flag.NArg() > 0 {
 		for _, path := range flag.Args() {
-			fmt.Printf("processing junit xml file : %s\n", path)
+			logger.Printf("processing junit xml file : %s\n", path)
 			xmlReader, err := os.Open(path)
 			if err != nil {
 				panic(err)
@@ -108,7 +111,7 @@ func main() {
 				panic(err)
 			}
 
-			pruneXML(suites, *maxTextSize*1e6) // convert MB into bytes (roughly!)
+			pruneXML(logger, suites, *maxTextSize*1e6) // convert MB into bytes (roughly!)
 
 			xmlWriter, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
 			if err != nil {
@@ -119,12 +122,12 @@ func main() {
 			if err != nil {
 				panic(err)
 			}
-			fmt.Println("done.")
+			logger.Println("done.")
 		}
 	}
 }
 
-func pruneXML(suites *JUnitTestSuites, maxBytes int) {
+func pruneXML(logger *log.Logger, suites *JUnitTestSuites, maxBytes int) {
 	// filter empty testSuites
 	filteredSuites := []JUnitTestSuite{}
 	for _, suite := range suites.Suites {
@@ -182,14 +185,14 @@ func pruneXML(suites *JUnitTestSuites, maxBytes int) {
 		for _, testcase := range suite.TestCases {
 			if testcase.SkipMessage != nil {
 				if len(testcase.SkipMessage.Message) > maxBytes {
-					fmt.Printf("clipping skip message in test case : %s\n", testcase.Name)
+					logger.Printf("clipping skip message in test case : %s\n", testcase.Name)
 					testcase.SkipMessage.Message = "[... clipped...]" +
 						testcase.SkipMessage.Message[len(testcase.SkipMessage.Message)-maxBytes:]
 				}
 			}
 			if testcase.Failure != nil {
 				if len(testcase.Failure.Contents) > maxBytes {
-					fmt.Printf("clipping failure message in test case : %s\n", testcase.Name)
+					logger.Printf("clipping failure message in test case : %s\n", testcase.Name)
 					testcase.Failure.Contents = "[... clipped...]" +
 						testcase.Failure.Contents[len(testcase.Failure.Contents)-maxBytes:]
 				}

hack/prune-junit-xml/prunexml_test.go

@@ -24,6 +24,8 @@ package main
 import (
 	"bufio"
 	"bytes"
+	"log"
+	"os"
 	"strings"
 	"testing"
 
@@ -92,8 +94,9 @@ func TestPruneXML(t *testing.T) {
 		<testcase classname="internal/controller/certificaterequests" name="Test_serializeApplyStatus" time="1.610000"></testcase>
 	</testsuite>
 </testsuites>`
+	logger := log.New(os.Stderr, "", 0)
 	suites, _ := fetchXML(strings.NewReader(sourceXML))
-	pruneXML(suites, 32)
+	pruneXML(logger, suites, 32)
 	var output bytes.Buffer
 	writer := bufio.NewWriter(&output)
 	_ = streamXML(writer, suites)

internal/apis/acme/validation/challenge.go

@@ -27,15 +27,15 @@ import (
 )
 
 func ValidateChallengeUpdate(a *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, []string) {
-	old, ok := oldObj.(*cmacme.Challenge)
-	new := newObj.(*cmacme.Challenge)
+	oldChallenge, ok := oldObj.(*cmacme.Challenge)
+	newChallenge := newObj.(*cmacme.Challenge)
 	// if oldObj is not set, the Update operation is always valid.
-	if !ok || old == nil {
+	if !ok || oldChallenge == nil {
 		return nil, nil
 	}
 
 	el := field.ErrorList{}
-	if !reflect.DeepEqual(old.Spec, new.Spec) {
+	if !reflect.DeepEqual(oldChallenge.Spec, newChallenge.Spec) {
 		el = append(el, field.Forbidden(field.NewPath("spec"), "challenge spec is immutable after creation"))
 	}
 	return el, nil

internal/apis/acme/validation/order.go

@@ -27,96 +27,96 @@ import (
 )
 
 func ValidateOrderUpdate(a *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, []string) {
-	old, ok := oldObj.(*cmacme.Order)
-	new := newObj.(*cmacme.Order)
+	oldOrder, ok := oldObj.(*cmacme.Order)
+	newOrder := newObj.(*cmacme.Order)
 	// if oldObj is not set, the Update operation is always valid.
-	if !ok || old == nil {
+	if !ok || oldOrder == nil {
 		return nil, nil
 	}
 
 	el := field.ErrorList{}
-	el = append(el, ValidateOrderSpecUpdate(old.Spec, new.Spec, field.NewPath("spec"))...)
-	el = append(el, ValidateOrderStatusUpdate(old.Status, new.Status, field.NewPath("status"))...)
+	el = append(el, ValidateOrderSpecUpdate(oldOrder.Spec, newOrder.Spec, field.NewPath("spec"))...)
+	el = append(el, ValidateOrderStatusUpdate(oldOrder.Status, newOrder.Status, field.NewPath("status"))...)
 	return el, nil
 }
 
 func ValidateOrder(a *admissionv1.AdmissionRequest, obj runtime.Object) (field.ErrorList, []string) {
 	return nil, nil
 }
 
-func ValidateOrderSpecUpdate(old, new cmacme.OrderSpec, fldPath *field.Path) field.ErrorList {
+func ValidateOrderSpecUpdate(oldOrder, newOrder cmacme.OrderSpec, fldPath *field.Path) field.ErrorList {
 	el := field.ErrorList{}
-	if len(old.Request) > 0 && !bytes.Equal(old.Request, new.Request) {
+	if len(oldOrder.Request) > 0 && !bytes.Equal(oldOrder.Request, newOrder.Request) {
 		el = append(el, field.Forbidden(fldPath.Child("request"), "field is immutable once set"))
 	}
 	return el
 }
 
-func ValidateOrderStatusUpdate(old, new cmacme.OrderStatus, fldPath *field.Path) field.ErrorList {
+func ValidateOrderStatusUpdate(oldStatus, newStatus cmacme.OrderStatus, fldPath *field.Path) field.ErrorList {
 	el := field.ErrorList{}
 	// once the order URL has been set, it cannot be changed
-	if old.URL != "" && old.URL != new.URL {
+	if oldStatus.URL != "" && oldStatus.URL != newStatus.URL {
 		el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
 	}
 	// once the FinalizeURL has been set, it cannot be changed
-	if old.FinalizeURL != "" && old.FinalizeURL != new.FinalizeURL {
+	if oldStatus.FinalizeURL != "" && oldStatus.FinalizeURL != newStatus.FinalizeURL {
 		el = append(el, field.Forbidden(fldPath.Child("finalizeURL"), "field is immutable once set"))
 	}
 	// once the Certificate has been issued, it cannot be changed
-	if len(old.Certificate) > 0 && !bytes.Equal(old.Certificate, new.Certificate) {
+	if len(oldStatus.Certificate) > 0 && !bytes.Equal(oldStatus.Certificate, newStatus.Certificate) {
 		el = append(el, field.Forbidden(fldPath.Child("certificate"), "field is immutable once set"))
 	}
 
-	if len(old.Authorizations) > 0 {
+	if len(oldStatus.Authorizations) > 0 {
 		fldPath := fldPath.Child("authorizations")
 
 		// once at least one Authorization has been inserted, no more can be added
 		// or deleted from the Order
-		if len(old.Authorizations) != len(new.Authorizations) {
+		if len(oldStatus.Authorizations) != len(newStatus.Authorizations) {
 			el = append(el, field.Forbidden(fldPath, "field is immutable once set"))
 		}
 
 		// here we know that len(old) == len(new), so we proceed to validate
 		// the updates that the user requested on each Authorization.
 		// fields on Authorization's cannot be changed after being set from
 		// their zero value.
-		for i := range old.Authorizations {
+		for i := range oldStatus.Authorizations {
 			fldPath := fldPath.Index(i)
-			old := old.Authorizations[i]
-			new := new.Authorizations[i]
-			if old.URL != "" && old.URL != new.URL {
+			oldAuthz := oldStatus.Authorizations[i]
+			newAuthz := newStatus.Authorizations[i]
+			if oldAuthz.URL != "" && oldAuthz.URL != newAuthz.URL {
 				el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
 			}
-			if old.Identifier != "" && old.Identifier != new.Identifier {
+			if oldAuthz.Identifier != "" && oldAuthz.Identifier != newAuthz.Identifier {
 				el = append(el, field.Forbidden(fldPath.Child("identifier"), "field is immutable once set"))
 			}
 			// don't allow the value of the Wildcard field to change unless the
 			// old value is nil
-			if old.Wildcard != nil && (new.Wildcard == nil || *old.Wildcard != *new.Wildcard) {
+			if oldAuthz.Wildcard != nil && (newAuthz.Wildcard == nil || *oldAuthz.Wildcard != *newAuthz.Wildcard) {
 				el = append(el, field.Forbidden(fldPath.Child("wildcard"), "field is immutable once set"))
 			}
-			if old.InitialState != "" && (old.InitialState != new.InitialState) {
+			if oldAuthz.InitialState != "" && (oldAuthz.InitialState != newAuthz.InitialState) {
 				el = append(el, field.Forbidden(fldPath.Child("initialState"), "field is immutable once set"))
 			}
 
-			if len(old.Challenges) > 0 {
+			if len(oldAuthz.Challenges) > 0 {
 				fldPath := fldPath.Child("challenges")
-				if len(old.Challenges) != len(new.Challenges) {
+				if len(oldAuthz.Challenges) != len(newAuthz.Challenges) {
 					el = append(el, field.Forbidden(fldPath, "field is immutable once set"))
 				}
 
-				for i := range old.Challenges {
+				for i := range oldAuthz.Challenges {
 					fldPath := fldPath.Index(i)
-					old := old.Challenges[i]
-					new := new.Challenges[i]
+					oldChallenge := oldAuthz.Challenges[i]
+					newChallenge := newAuthz.Challenges[i]
 
-					if old.URL != "" && old.URL != new.URL {
+					if oldChallenge.URL != "" && oldChallenge.URL != newChallenge.URL {
 						el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
 					}
-					if old.Type != "" && old.Type != new.Type {
+					if oldChallenge.Type != "" && oldChallenge.Type != newChallenge.Type {
 						el = append(el, field.Forbidden(fldPath.Child("type"), "field is immutable once set"))
 					}
-					if old.Token != "" && old.Token != new.Token {
+					if oldChallenge.Token != "" && oldChallenge.Token != newChallenge.Token {
 						el = append(el, field.Forbidden(fldPath.Child("token"), "field is immutable once set"))
 					}
 				}

internal/apis/acme/validation/order_test.go

@@ -55,11 +55,11 @@ func testImmutableOrderField(t *testing.T, fldPath *field.Path, setter func(*cma
 			field.Forbidden(fldPath, "field is immutable once set"),
 		}
 		var expectedWarnings []string
-		old := &cmacme.Order{}
-		new := &cmacme.Order{}
-		setter(old, testValueOptionOne)
-		setter(new, testValueOptionTwo)
-		errs, warnings := ValidateOrderUpdate(someAdmissionRequest, old, new)
+		oldOrder := &cmacme.Order{}
+		newOrder := &cmacme.Order{}
+		setter(oldOrder, testValueOptionOne)
+		setter(newOrder, testValueOptionTwo)
+		errs, warnings := ValidateOrderUpdate(someAdmissionRequest, oldOrder, newOrder)
 		if len(errs) != len(expectedErrs) {
 			t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
 			return
@@ -77,11 +77,11 @@ func testImmutableOrderField(t *testing.T, fldPath *field.Path, setter func(*cma
 	t.Run("should allow updates to "+fldPath.String()+" if not already set", func(t *testing.T) {
 		expectedErrs := []*field.Error{}
 		var expectedWarnings []string
-		old := &cmacme.Order{}
-		new := &cmacme.Order{}
-		setter(old, testValueNone)
-		setter(new, testValueOptionOne)
-		errs, warnings := ValidateOrderUpdate(someAdmissionRequest, old, new)
+		oldOrder := &cmacme.Order{}
+		newOrder := &cmacme.Order{}
+		setter(oldOrder, testValueNone)
+		setter(newOrder, testValueOptionOne)
+		errs, warnings := ValidateOrderUpdate(someAdmissionRequest, oldOrder, newOrder)
 		if len(errs) != len(expectedErrs) {
 			t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
 			return