Merge pull request #1471 from liranmauda/liran-backport-into-5_16

[Backport into 5.16] CI | Fixing deprecated linter

.github/workflows/golangci-lint.yml

@@ -11,20 +11,9 @@ jobs:
       cancel-in-progress: true
     steps:
       - uses: actions/checkout@v4
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+      - uses: actions/setup-go@v5
         with:
-          # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.55
-
-          # Optional: if set to true then the all caching functionality will be complete disabled,
-          #           takes precedence over all other caching options.
-          # skip-cache: true
-
-          # Optional: working directory, useful for monorepos
-          # working-directory: somedir
-
-          # Optional: golangci-lint command line arguments.
-          args: --disable-all --print-issued-lines -E typecheck,errcheck,gosimple,unused,ineffassign,staticcheck --timeout=4m
-          # Optional: show only new issues if it's a pull request. The default value is `false`.
-          # only-new-issues: true
+          go-version: "1.21"     
+      - name: run lint
+        id: run-build
+        run: make lint || exit 1

.golangci.yml

@@ -0,0 +1,9 @@
+run:
+  timeout: 5m
+
+issues:
+  exclude-dirs:
+    - pkg/apis/noobaa/v1alpha1
+    - pkg/bundle
+  exclude-files:
+    - zz_generated\.go

Makefile

@@ -177,7 +177,9 @@ golangci-lint: gen
 .PHONY: golangci-lint
 
 lint: gen
-	@echo "Lint is deprecated and failing due to a dependency. Disabling it as a quick fix to release the CI flow."
+	@echo ""
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+	golangci-lint run --config .golangci.yml
 	@echo "✅ lint"
 .PHONY: lint
 

pkg/backingstore/backingstore.go

@@ -395,7 +395,9 @@ func createCommon(cmd *cobra.Command, args []string, storeType nbv1.StoreType, p
 		log.Printf("Found a Secret in the system with the same credentials (%s)", suggestedSecret.Name)
 		log.Printf("Note that using more then one secret with the same credentials is not supported")
 		log.Printf("do you want to use it for this Backingstore? y/n")
-		fmt.Scanln(&decision)
+		if _, err := fmt.Scanln(&decision); err != nil {
+			log.Fatalf(`❌ Invalid input, please select y/n`)
+		}
 		if strings.ToLower(decision) == "y" {
 			log.Printf("Will use %s as the Backingstore %s Secret", suggestedSecret.Name, backStore.Name)
 			err := util.SetBackingStoreSecretRef(backStore, &corev1.SecretReference{
@@ -407,8 +409,6 @@ func createCommon(cmd *cobra.Command, args []string, storeType nbv1.StoreType, p
 			}
 		} else if strings.ToLower(decision) == "n" {
 			log.Fatalf("Not creating Backingstore")
-		} else {
-			log.Fatalf(`❌ Invalid input, please select y/n`)
 		}
 	}
 
@@ -1076,7 +1076,7 @@ func MapSecretToBackingStores(secret types.NamespacedName) []reconcile.Request {
 	for _, bs := range bsList.Items {
 		bsSecret, err := util.GetBackingStoreSecret(&bs)
 		if err != nil {
-			log.Errorf(err.Error())
+			log.Error(err)
 		}
 		if bsSecret != nil && bsSecret.Name == secret.Name {
 			reqs = append(reqs, reconcile.Request{

pkg/bucketclass/bucketclass.go

@@ -3,6 +3,7 @@ package bucketclass
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"time"
 
@@ -787,12 +788,12 @@ func GetDefaultBucketClass(Namespace string) (*nbv1.BucketClass, error) {
 
 	if !util.KubeCheck(bucketClass) {
 		msg := fmt.Sprintf("GetDefaultBucketClass BucketClass %q not found in provisioner namespace %q", bucketClassName, Namespace)
-		return nil, fmt.Errorf(msg)
+		return nil, errors.New(msg)
 	}
 
 	if bucketClass.Status.Phase != nbv1.BucketClassPhaseReady {
 		msg := fmt.Sprintf("GetDefaultBucketClass BucketClass %q is %v", bucketClassName, bucketClass.Status.Phase)
-		return nil, fmt.Errorf(msg)
+		return nil, errors.New(msg)
 	}
 
 	return bucketClass, nil

pkg/cosi/provisioner.go

@@ -119,7 +119,7 @@ func (p *Provisioner) DriverCreateBucket(ctx context.Context,
 		finalizersArray := r.SysClient.NooBaa.GetFinalizers()
 		if util.Contains(finalizersArray, nbv1.GracefulFinalizer) {
 			msg := "NooBaa is in deleting state, new requests will be ignored"
-			log.Errorf(msg)
+			log.Error(msg)
 			return nil, status.Error(codes.Internal, msg)
 		}
 	}
@@ -185,7 +185,7 @@ func (p *Provisioner) DriverGrantBucketAccess(ctx context.Context,
 		finalizersArray := r.SysClient.NooBaa.GetFinalizers()
 		if util.Contains(finalizersArray, nbv1.GracefulFinalizer) {
 			msg := "NooBaa is in deleting state, new requests will be ignored"
-			log.Errorf(msg)
+			log.Error(msg)
 			return nil, status.Error(codes.Internal, msg)
 		}
 	}

pkg/install/install.go

@@ -151,19 +151,21 @@ func RunUninstall(cmd *cobra.Command, args []string) {
 	if cleanup {
 		var decision string
 
+		log.Printf("--cleanup removes the CRDs and affecting all noobaa instances, are you sure? y/n ")
 		for {
-			log.Printf("--cleanup removes the CRDs and affecting all noobaa instances, are you sure? y/n ")
-			fmt.Scanln(&decision)
+			if _, err := fmt.Scanln(&decision); err != nil {
+				log.Printf(`are you sure? y/n`)
+			}
 
 			if decision == "y" {
 				log.Printf("Will remove CRD (cluster scope)")
 				break
 			} else if decision == "n" {
-				return
+				log.Printf("Will not uninstall as remove CRD (cluster scope) was declined.")
+				log.Fatalf("In order to uninstall agree to remove CRD or remove the --cleanup flag.")
 			}
 		}
 	}
-
 	system.RunSystemVersionsStatus(cmd, args)
 	log.Printf("Namespace: %s", options.Namespace)
 	log.Printf("")

pkg/namespacestore/namespacestore.go

@@ -370,7 +370,9 @@ func createCommon(cmd *cobra.Command, args []string, storeType nbv1.NSType, popu
 		log.Printf("Found a Secret in the system with the same credentials (%s)", suggestedSecret.Name)
 		log.Printf("Note that using more then one secret with the same credentials is not supported")
 		log.Printf("do you want to use it for this Namespacestore? y/n")
-		fmt.Scanln(&decision)
+		if _, err := fmt.Scanln(&decision); err != nil {
+			log.Fatalf(`❌ Invalid input, please select y/n`)
+		}
 		if strings.ToLower(decision) == "y" {
 			log.Printf("Will use %s as the Namespacestore Secret", suggestedSecret.Name)
 			err := util.SetNamespaceStoreSecretRef(namespaceStore, &corev1.SecretReference{
@@ -382,8 +384,6 @@ func createCommon(cmd *cobra.Command, args []string, storeType nbv1.NSType, popu
 			}
 		} else if strings.ToLower(decision) == "n" {
 			log.Fatalf("Not creating Namespacestore")
-		} else {
-			log.Fatalf(`❌ Invalid input, please select y/n`)
 		}
 	}
 
@@ -899,7 +899,7 @@ func MapSecretToNamespaceStores(secret types.NamespacedName) []reconcile.Request
 	for _, ns := range nsList.Items {
 		nsSecret, err := util.GetNamespaceStoreSecret(&ns)
 		if err != nil {
-			log.Errorf(err.Error())
+			log.Error(err)
 		}
 		if nsSecret != nil && nsSecret.Name == secret.Name {
 			reqs = append(reqs, reconcile.Request{

pkg/nb/rpc_ws.go

@@ -318,7 +318,7 @@ func (c *RPCConnWS) ReadMessage() (*RPCMessage, error) {
 	msg.RawBytes = msgBytes
 
 	// Read message buffers if any
-	if msg.Buffers != nil && len(msg.Buffers) > 0 {
+	if len(msg.Buffers) > 0 {
 		buffers, err := io.ReadAll(reader)
 		// if err != nil && err.Error() != "failed to read: cannot use EOFed reader" {
 		if err != nil {

pkg/noobaaaccount/noobaaaccount.go

@@ -362,7 +362,9 @@ func RunRegenerate(cmd *cobra.Command, args []string) {
 	log.Printf("are you sure? y/n")
 
 	for {
-		fmt.Scanln(&decision)
+		if _, err := fmt.Scanln(&decision); err != nil {
+			log.Printf(`are you sure? y/n`)
+		}
 		if decision == "y" {
 			break
 		} else if decision == "n" {

pkg/obc/obc.go

@@ -265,7 +265,9 @@ func RunRegenerate(cmd *cobra.Command, args []string) {
 	log.Printf("are you sure? y/n")
 
 	for {
-		fmt.Scanln(&decision)
+		if _, err := fmt.Scanln(&decision); err != nil {
+			log.Printf(`are you sure? y/n`)
+		}
 		if decision == "y" {
 			break
 		} else if decision == "n" {

pkg/obc/provisioner.go

@@ -2,6 +2,7 @@ package obc
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"strconv"
@@ -112,7 +113,7 @@ func (p *Provisioner) Provision(bucketOptions *obAPI.BucketOptions) (*nbv1.Objec
 		finalizersArray := r.SysClient.NooBaa.GetFinalizers()
 		if util.Contains(finalizersArray, nbv1.GracefulFinalizer) {
 			msg := "NooBaa is in deleting state, new requests will be ignored"
-			log.Errorf(msg)
+			log.Error(msg)
 			return nil, obErrors.NewBucketExistsError(msg)
 		}
 	}
@@ -151,7 +152,7 @@ func (p *Provisioner) Grant(bucketOptions *obAPI.BucketOptions) (*nbv1.ObjectBuc
 		finalizersArray := r.SysClient.NooBaa.GetFinalizers()
 		if util.Contains(finalizersArray, nbv1.GracefulFinalizer) {
 			msg := "NooBaa is in deleting state, new requests will be ignored"
-			log.Errorf(msg)
+			log.Error(msg)
 			return nil, obErrors.NewBucketExistsError(msg)
 		}
 	}
@@ -308,12 +309,12 @@ func NewBucketRequest(
 			}
 
 			p.recorder.Event(r.OBC, "Warning", "MissingBucketClass", msg)
-			return nil, fmt.Errorf(msg)
+			return nil, errors.New(msg)
 		}
 		if r.BucketClass.Status.Phase != nbv1.BucketClassPhaseReady {
 			msg := fmt.Sprintf("BucketClass %q is not ready", bucketClassName)
 			p.recorder.Event(r.OBC, "Warning", "BucketClassNotReady", msg)
-			return nil, fmt.Errorf(msg)
+			return nil, errors.New(msg)
 		}
 		additionalConfig := r.OBC.Spec.AdditionalConfig
 		if additionalConfig == nil {
@@ -554,7 +555,7 @@ func (r *BucketRequest) LogAndGetError(format string, a ...interface{}) error {
 	log := r.Provisioner.Logger
 	msg := fmt.Sprintf(format, a...)
 	log.Error(msg)
-	return fmt.Errorf(msg)
+	return errors.New(msg)
 }
 
 // CreateAccount creates the obc account
@@ -576,8 +577,8 @@ func (r *BucketRequest) CreateAccount() error {
 			return fmt.Errorf("failed to parse NSFS config %q: %w", r.OBC.Spec.AdditionalConfig["nsfsAccountConfig"], err)
 		}
 		// We prefer to make sure this account is only used for its appropriate NSFS operations
-		nsfsAccountConfig.NewBucketsPath = "";
-		nsfsAccountConfig.NsfsOnly = true;
+		nsfsAccountConfig.NewBucketsPath = ""
+		nsfsAccountConfig.NsfsOnly = true
 		// -1 is the default CLI value which we use to indicate that the UID/GID should not be set
 		// 0 cannot be used since it is a valid GID/UID value
 		var IDNullifier = -1
@@ -586,14 +587,14 @@ func (r *BucketRequest) CreateAccount() error {
 			nsfsAccountConfig.GID = nil
 		}
 	}
-	
+
 	accountInfo, err := r.SysClient.NBClient.CreateAccountAPI(nb.CreateAccountParams{
-		Name:              r.AccountName,
-		Email:             r.AccountName,
-		// defaultResource is left as-is only because AllowBucketCreate is false 
-		DefaultResource:   defaultResource,
-		HasLogin:          false,
-		S3Access:          true,
+		Name:  r.AccountName,
+		Email: r.AccountName,
+		// defaultResource is left as-is only because AllowBucketCreate is false
+		DefaultResource: defaultResource,
+		HasLogin:        false,
+		S3Access:        true,
 		// If this field is to be changed, DefaultResource above will need to be modified as well
 		AllowBucketCreate: false,
 		BucketClaimOwner:  r.BucketName,

pkg/system/reconciler.go

@@ -499,8 +499,8 @@ func (r *Reconciler) VerifyObjectBucketCleanup() error {
 		}
 		msg := fmt.Sprintf("Failed to delete NooBaa. object buckets in namespace %q are not cleaned up. remaining buckets: %+v",
 			r.NooBaa.Namespace, bucketNames)
-		log.Errorf(msg)
-		return fmt.Errorf(msg)
+		log.Error(msg)
+		return errors.New(msg)
 	}
 
 	log.Infof("All object buckets deleted in namespace %q", r.NooBaa.Namespace)

pkg/util/kms/test/dev/kms_dev_test.go

@@ -41,7 +41,7 @@ func simpleKmsSpec(token, apiAddress string) nbv1.KeyManagementServiceSpec {
 func checkExternalSecret(noobaa *nbv1.NooBaa, expectedNil bool) {
 	k := noobaa.Spec.Security.KeyManagementService
 	uid := string(noobaa.UID)
-	driver := &kms.Vault{uid}
+	driver := &kms.Vault{UID: uid}
 	path := k.ConnectionDetails[vault.VaultBackendPathKey] + driver.Path()
 	cmd := exec.Command("kubectl", "exec", "vault-0", "--", "vault", "kv", "get", path)
 	logger.Printf("Running command: path %v args %v ", cmd.Path, cmd.Args)

pkg/util/kms/test/ibm-kp/kms_ibm_kp_test.go

@@ -52,7 +52,7 @@ func checkExternalSecret(tokenSecretName string, instanceID string, noobaa *nbv1
 
 	k := noobaa.Spec.Security.KeyManagementService
 	uid := string(noobaa.UID)
-	driver := &kms.IBM{uid}
+	driver := &kms.IBM{UID: uid}
 
 	// Generate backend configuration using backend driver instance
 	c, err := driver.Config(k.ConnectionDetails, k.TokenSecretName, noobaa.Namespace)