Enable revive linter

.golangci.yml

@@ -11,12 +11,20 @@ linters:
   - govet
   - misspell
   - nakedret
+  - revive
   - unconvert
   - whitespace
 
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
+  exclude:
+    # Allow dot imports for test files
+    - 'dot-imports: should not use dot import'
+    # Allow unused contexts
+    - "unused-parameter: parameter 'ctx' seems to be unused"
+    # Allow unused variables (necessary sometimes to match interfaces)
+    - "unused-parameter: parameter '.*' seems to be unused, consider removing or renaming it as _"
 
 linters-settings:
   errorlint:

hack/build/bazel-build-builder.sh

@@ -34,23 +34,23 @@ fi
 # we should re-build the builder. Separating out this logic from the test for clarity
 
 git diff-index --quiet HEAD~1 hack/build/docker
-POST_SUBMIT=$((1-$?))
+POST_SUBMIT=$((1 - $?))
 
 # The other circumstance in which we need to build the builder image is
-# in the course of test and development of the builder image itself. 
+# in the course of test and development of the builder image itself.
 # we'll signal this circumstance by setting the env variable ADHOC_BUILDER
 #
 # also instead of hard-coding the UNTAGGED_BUILDER_IMAGE, we're going
-# to use DOCKER_PREFIX as it is set in config.sh and used elsewhere in 
-# the build system, to introduce a little more consistency  
+# to use DOCKER_PREFIX as it is set in config.sh and used elsewhere in
+# the build system, to introduce a little more consistency
 
-if ((POST_SUBMIT)) || [ x"${ADHOC_BUILDER}" != "x" ] ; then
+if ((POST_SUBMIT)) || [ x"${ADHOC_BUILDER}" != "x" ]; then
     BUILDER_SPEC="${BUILD_DIR}/docker/builder"
     UNTAGGED_BUILDER_IMAGE=$(DOCKER_PREFIX)/kubevirt-cdi-bazel-builder
     BUILDER_TAG=$(date +"%y%m%d%H%M")-$(git rev-parse --short HEAD)
     BUILDER_MANIFEST=${UNTAGGED_BUILDER_IMAGE}:${BUILDER_TAG}
     echo "export BUILDER_IMAGE=$BUILDER_MANIFEST"
-    
+
     #Build the encapsulated compile and test container
     if [ "${CDI_CONTAINER_BUILDCMD}" = "buildah" ]; then
         (cd ${BUILDER_SPEC} && buildah build ${BUILDAH_PLATFORM_FLAG} --manifest ${BUILDER_MANIFEST} .)

hack/update-codegen.sh

@@ -53,7 +53,6 @@ swagger-doc -in ${SCRIPT_ROOT}/staging/src/kubevirt.io/containerized-data-import
 
 swagger-doc -in ${SCRIPT_ROOT}/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/forklift/v1beta1/types.go
 
-
 (go install ${CODEGEN_PKG}/cmd/openapi-gen)
 
 (

pkg/apiserver/apiserver.go

@@ -157,7 +157,7 @@ func NewCdiAPIServer(bindAddress string,
 
 	err = app.getKeysAndCerts()
 	if err != nil {
-		return nil, errors.Errorf("Unable to get self signed cert: %v\n", errors.WithStack(err))
+		return nil, errors.Errorf("unable to get self signed cert: %v", errors.WithStack(err))
 	}
 
 	app.composeUploadTokenAPI()

pkg/controller/common/util.go

@@ -1125,7 +1125,7 @@ func GetEndpoint(pvc *corev1.PersistentVolumeClaim) (string, error) {
 		if !found {
 			verb = "missing"
 		}
-		return ep, errors.Errorf("annotation %q in pvc \"%s/%s\" is %s\n", AnnEndpoint, pvc.Namespace, pvc.Name, verb)
+		return ep, errors.Errorf("annotation %q in pvc \"%s/%s\" is %s", AnnEndpoint, pvc.Namespace, pvc.Name, verb)
 	}
 	return ep, nil
 }

pkg/controller/config-controller.go

@@ -660,10 +660,10 @@ func getClusterWideProxy(r client.Client) (*ocpconfigv1.Proxy, error) {
 // GetImportProxyConfig attempts to import proxy URLs if configured in the CDIConfig.
 func GetImportProxyConfig(config *cdiv1.CDIConfig, field string) (string, error) {
 	if config == nil {
-		return "", errors.Errorf("failed to get field, the CDIConfig is nil\n")
+		return "", errors.New("failed to get field, the CDIConfig is nil")
 	}
 	if config.Status.ImportProxy == nil {
-		return "", errors.Errorf("failed to get field, the CDIConfig ImportProxy is nil\n")
+		return "", errors.New("failed to get field, the CDIConfig ImportProxy is nil")
 	}
 
 	switch field {
@@ -684,7 +684,7 @@ func GetImportProxyConfig(config *cdiv1.CDIConfig, field string) (string, error)
 			return *config.Status.ImportProxy.TrustedCAProxy, nil
 		}
 	default:
-		return "", errors.Errorf("CDIConfig ImportProxy does not have the field: %s\n", field)
+		return "", errors.Errorf("CDIConfig ImportProxy does not have the field: %s", field)
 	}
 
 	// If everything fails, return blank

pkg/controller/config-controller_test.go

@@ -854,14 +854,14 @@ var _ = Describe("GetImportProxyConfig", func() {
 		cdiConfig := MakeEmptyCDIConfigSpec("cdiconfig")
 		cdiConfig.Status.ImportProxy = createImportProxy("", "", "", "")
 		_, err := GetImportProxyConfig(cdiConfig, "nonExistingField")
-		Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("CDIConfig ImportProxy does not have the field: %s\n", "nonExistingField")))
+		Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("CDIConfig ImportProxy does not have the field: %s", "nonExistingField")))
 	})
 
 	It("should return error if the ImportProxy field is nil", func() {
 		cdiConfig := MakeEmptyCDIConfigSpec("cdiconfig")
 		cdiConfig.Status.ImportProxy = nil
 		_, err := GetImportProxyConfig(cdiConfig, common.ImportProxyHTTP)
-		Expect(err.Error()).To(ContainSubstring("failed to get field, the CDIConfig ImportProxy is nil\n"))
+		Expect(err.Error()).To(ContainSubstring("failed to get field, the CDIConfig ImportProxy is nil"))
 	})
 })
 

pkg/controller/import-controller.go

@@ -793,7 +793,7 @@ func (r *ImportReconciler) getVddkImageName() (*string, error) {
 		return &image, nil
 	}
 
-	return nil, errors.Errorf("Found %s ConfigMap in namespace %s, but it does not contain a '%s' entry.", common.VddkConfigMap, namespace, common.VddkConfigDataKey)
+	return nil, errors.Errorf("found %s ConfigMap in namespace %s, but it does not contain a '%s' entry", common.VddkConfigMap, namespace, common.VddkConfigDataKey)
 }
 
 // returns the import image part of the endpoint string

pkg/image/qemu.go

@@ -257,7 +257,7 @@ func checkIfURLIsValid(info *ImgInfo, availableSize int64, image string) error {
 	}
 
 	if availableSize < info.VirtualSize {
-		return errors.Errorf("Virtual image size %d is larger than the reported available storage %d. A larger PVC is required.", info.VirtualSize, availableSize)
+		return errors.Errorf("virtual image size %d is larger than the reported available storage %d. A larger PVC is required", info.VirtualSize, availableSize)
 	}
 	return nil
 }

pkg/image/qemu_test.go

@@ -280,7 +280,7 @@ var _ = Describe("Validate", func() {
 		Entry("should return error on bad json", mockExecFunction(badValidateJSON, "", expectedLimits), "unexpected end of JSON input", imageName),
 		Entry("should return error on bad format", mockExecFunction(badFormatValidateJSON, "", expectedLimits), fmt.Sprintf("Invalid format raw2 for image %s", imageName), imageName),
 		Entry("should return error on invalid backing file", mockExecFunction(backingFileValidateJSON, "", expectedLimits), fmt.Sprintf("Image %s is invalid because it has invalid backing file backing-file.qcow2", imageName), imageName),
-		Entry("should return error when PVC is too small", mockExecFunction(hugeValidateJSON, "", expectedLimits), fmt.Sprintf("Virtual image size %d is larger than the reported available storage %d. A larger PVC is required.", 52949672960, 42949672960), imageName),
+		Entry("should return error when PVC is too small", mockExecFunction(hugeValidateJSON, "", expectedLimits), fmt.Sprintf("virtual image size %d is larger than the reported available storage %d. A larger PVC is required", 52949672960, 42949672960), imageName),
 	)
 
 })

pkg/importer/imageio-datasource.go

@@ -487,7 +487,7 @@ func (reader *extentReader) GetRange(start, end int64) (io.ReadCloser, error) {
 	expectedLength := end - start + 1
 	if length != expectedLength {
 		response.Body.Close()
-		return nil, errors.New(fmt.Sprintf("wrong length returned: %d vs expected %d", length, expectedLength))
+		return nil, errors.Errorf("wrong length returned: %d vs expected %d", length, expectedLength)
 	}
 
 	return response.Body, nil
@@ -513,13 +513,13 @@ func createImageioReader(ctx context.Context, ep string, accessKey string, secKe
 		if snapshot == nil { // Snapshot not found, check for a disk with a matching image ID
 			imageID, available := disk.ImageId()
 			if !available {
-				return nil, uint64(0), nil, conn, errors.Wrap(snapshotErr, "Could not get disk's image ID!")
+				return nil, uint64(0), nil, conn, errors.Wrap(snapshotErr, "could not get disk's image ID")
 			}
 			if imageID != currentCheckpoint {
-				return nil, uint64(0), nil, conn, errors.Wrapf(snapshotErr, "Snapshot %s not found!", currentCheckpoint)
+				return nil, uint64(0), nil, conn, errors.Wrapf(snapshotErr, "snapshot %s not found", currentCheckpoint)
 			}
 			// Matching ID: use disk as checkpoint
-			klog.Infof("Snapshot ID %s found on disk %s, transferring active disk as checkpoint.", currentCheckpoint, diskID)
+			klog.Infof("Snapshot ID %s found on disk %s, transferring active disk as checkpoint", currentCheckpoint, diskID)
 		}
 	}
 

pkg/importer/vddk-datasource_amd64.go

@@ -502,7 +502,7 @@ const MaxPreadLengthVC = (2 << 20)
 // MaxPreadLength is the maxmimum read size to request from VMware. Default to
 // the larger option, and reduce it in createVddkDataSource when connecting to
 // vCenter endpoints.
-var MaxPreadLength int = MaxPreadLengthESX
+var MaxPreadLength = MaxPreadLengthESX
 
 // NbdOperations provides a mockable interface for the things needed from libnbd.
 type NbdOperations interface {

pkg/importer/vddk-datasource_test.go

@@ -666,14 +666,11 @@ var _ = Describe("VDDK get block status", func() {
 		flagSetting := true
 		currentMockNbdFunctions.BlockStatus = func(length uint64, offset uint64, callback libnbd.ExtentCallback, optargs *libnbd.BlockStatusOptargs) error {
 			err := 0
-			len := uint32(MaxBlockStatusLength)
-			if length < MaxBlockStatusLength {
-				len = uint32(length)
-			}
+			ln := uint32(min(length, MaxBlockStatusLength))
 			if flagSetting {
-				callback("base:allocation", offset, []uint32{len, 0}, &err)
+				callback("base:allocation", offset, []uint32{ln, 0}, &err)
 			} else {
-				callback("base:allocation", offset, []uint32{len, 3}, &err)
+				callback("base:allocation", offset, []uint32{ln, 3}, &err)
 			}
 			flagSetting = !flagSetting
 			return nil

pkg/system/prlimit_test.go

@@ -210,7 +210,7 @@ func doFaker(args []string) {
 }
 
 func doSpinner(args []string) {
-	for { //nolint:staticcheck
+	for { //nolint:staticcheck,revive
 
 	}
 }

tests/apiserver_test.go

@@ -22,7 +22,7 @@ import (
 
 const (
 	cdiDeploymentPodPrefix  = "cdi-deployment-"
-	cdiApiServerPodPrefix   = "cdi-apiserver-"
+	cdiAPIServerPodPrefix   = "cdi-apiserver-"
 	cdiUploadProxyPodPrefix = "cdi-uploadproxy-"
 
 	pollingInterval = 2 * time.Second

tests/cdiconfig_test.go

@@ -27,12 +27,12 @@ import (
 )
 
 const (
-	ingressUrl = "www.super-duper-test.ingress.tt.org"
+	ingressURL = "www.super-duper-test.ingress.tt.org"
 	routeName  = "cdi-uploadproxy"
 )
 
 var (
-	defaultUrl = ""
+	defaultURL = ""
 )
 
 var _ = Describe("CDI storage class config tests", Serial, func() {
@@ -245,7 +245,7 @@ var _ = Describe("CDI ingress config tests, using manifests", Serial, func() {
 			}, time.Second*30, time.Second).Should(BeTrue())
 			config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			defaultUrl = *config.Status.UploadProxyURL
+			defaultURL = *config.Status.UploadProxyURL
 		}
 		By("Making sure no url is set")
 		config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
@@ -264,14 +264,14 @@ var _ = Describe("CDI ingress config tests, using manifests", Serial, func() {
 				return ""
 			}
 			return *config.Status.UploadProxyURL
-		}, time.Second*30, time.Second).Should(Equal(defaultUrl))
+		}, time.Second*30, time.Second).Should(Equal(defaultURL))
 	})
 
 	AfterEach(func() {
 		_, err := f.RunKubectlCommand("delete", "-f", manifestFile, "-n", f.CdiInstallNs)
 		Expect(err).ShouldNot(HaveOccurred())
 
-		matchingVals := []string{defaultUrl}
+		matchingVals := []string{defaultURL}
 		if origUploadProxyOverride != nil {
 			matchingVals = append(matchingVals, *origUploadProxyOverride)
 		}
@@ -321,7 +321,7 @@ var _ = Describe("CDI ingress config tests, using manifests", Serial, func() {
 				return *config.Status.UploadProxyURL
 			}
 			return ""
-		}, time.Second*30, time.Second).Should(Equal(defaultUrl))
+		}, time.Second*30, time.Second).Should(Equal(defaultURL))
 		for i := 0; i < 10; i++ {
 			// Check for 20 seconds if the deployment pod crashed.
 			time.Sleep(2 * time.Second)
@@ -360,7 +360,7 @@ var _ = Describe("CDI ingress config tests", Serial, func() {
 			}, time.Second*30, time.Second).Should(BeTrue())
 			config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			defaultUrl = *config.Status.UploadProxyURL
+			defaultURL = *config.Status.UploadProxyURL
 		}
 		By("Making sure no url is set")
 		config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
@@ -379,11 +379,11 @@ var _ = Describe("CDI ingress config tests", Serial, func() {
 				return ""
 			}
 			return *config.Status.UploadProxyURL
-		}, time.Second*30, time.Second).Should(Equal(defaultUrl))
+		}, time.Second*30, time.Second).Should(Equal(defaultURL))
 	})
 
 	AfterEach(func() {
-		matchingVals := []string{defaultUrl}
+		matchingVals := []string{defaultURL}
 		if origUploadProxyOverride != nil {
 			matchingVals = append(matchingVals, *origUploadProxyOverride)
 		}
@@ -410,18 +410,18 @@ var _ = Describe("CDI ingress config tests", Serial, func() {
 
 	It("[test_id:3960]Should set uploadProxyURL if override is not defined", func() {
 		// TODO, don't hard code "cdi-uploadproxy", read it from container env of cdi-deployment deployment.
-		ingress = createIngress("test-ingress", f.CdiInstallNs, "cdi-uploadproxy", ingressUrl)
+		ingress = createIngress("test-ingress", f.CdiInstallNs, "cdi-uploadproxy", ingressURL)
 		_, err := f.K8sClient.NetworkingV1().Ingresses(f.CdiInstallNs).Create(context.TODO(), ingress, metav1.CreateOptions{})
 		Expect(err).ToNot(HaveOccurred())
-		By("Expecting uploadproxy url to be " + ingressUrl)
+		By("Expecting uploadproxy url to be " + ingressURL)
 		Eventually(func() string {
 			config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
 			if config.Status.UploadProxyURL == nil {
 				return ""
 			}
 			return *config.Status.UploadProxyURL
-		}, time.Second*30, time.Second).Should(Equal(ingressUrl))
+		}, time.Second*30, time.Second).Should(Equal(ingressURL))
 	})
 
 	It("[test_id:3961]Should keep override uploadProxyURL if override is defined", func() {
@@ -431,7 +431,7 @@ var _ = Describe("CDI ingress config tests", Serial, func() {
 		})
 		Expect(err).ToNot(HaveOccurred())
 		// TODO, don't hard code "cdi-uploadproxy", read it from container env of cdi-deployment deployment.
-		ingress = createIngress("test-ingress", f.CdiInstallNs, "cdi-uploadproxy", ingressUrl)
+		ingress = createIngress("test-ingress", f.CdiInstallNs, "cdi-uploadproxy", ingressURL)
 		_, err = f.K8sClient.NetworkingV1().Ingresses(f.CdiInstallNs).Create(context.TODO(), ingress, metav1.CreateOptions{})
 		Expect(err).ToNot(HaveOccurred())
 		By("Expecting uploadproxy url to be " + override)
@@ -454,12 +454,12 @@ var _ = Describe("CDI ingress config tests", Serial, func() {
 		Eventually(func() string {
 			config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			proxyUrl := ""
+			proxyURL := ""
 			if config.Status.UploadProxyURL != nil {
-				proxyUrl = *config.Status.UploadProxyURL
+				proxyURL = *config.Status.UploadProxyURL
 			}
-			return proxyUrl
-		}, time.Second*30, time.Second).Should(Equal(defaultUrl))
+			return proxyURL
+		}, time.Second*30, time.Second).Should(Equal(defaultURL))
 	})
 })
 
@@ -645,7 +645,7 @@ var _ = Describe("Modifying CDIConfig spec tests", Serial, func() {
 
 })
 
-func createIngress(name, ns, service, hostUrl string) *networkingv1.Ingress {
+func createIngress(name, ns, service, hostURL string) *networkingv1.Ingress {
 	return &networkingv1.Ingress{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "networking/v1",
@@ -659,7 +659,7 @@ func createIngress(name, ns, service, hostUrl string) *networkingv1.Ingress {
 				Service: &networkingv1.IngressServiceBackend{Name: service, Port: networkingv1.ServiceBackendPort{Number: 443}},
 			},
 			Rules: []networkingv1.IngressRule{
-				{Host: hostUrl},
+				{Host: hostURL},
 			},
 		},
 	}

tests/framework/framework.go

@@ -615,7 +615,7 @@ func (f *Framework) UpdateCdiConfigResourceLimits(resourceCPU, resourceMemory, l
 		}
 		values := strings.Fields(res)
 		if len(values) != 4 {
-			return false, errors.New(fmt.Sprintf("length is not 4: %d", len(values)))
+			return false, errors.Errorf("length is not 4: %d", len(values))
 		}
 		reqCPU, err := strconv.ParseInt(values[0], 10, 64)
 		if err != nil {

tests/monitoring_test.go

@@ -60,15 +60,15 @@ var _ = Describe("[Destructive] Monitoring Tests", Serial, func() {
 		Eventually(func() bool {
 			scs, err := f.K8sClient.StorageV1().StorageClasses().List(context.TODO(), metav1.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			sc_count := len(scs.Items)
+			scCount := len(scs.Items)
 			sps, err := f.CdiClient.CdiV1beta1().StorageProfiles().List(context.TODO(), metav1.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())
-			sp_count := len(sps.Items)
+			spCount := len(sps.Items)
 
 			complete := countMetricLabelValue(f, "kubevirt_cdi_storageprofile_info", "complete", "true")
 			incomplete := countMetricLabelValue(f, "kubevirt_cdi_storageprofile_info", "complete", "false")
 
-			return sc_count == sp_count && sc_count == complete+incomplete
+			return scCount == spCount && scCount == complete+incomplete
 		}, 2*time.Minute, 1*time.Second).Should(BeTrue())
 	}