From 23ce9408691a544b3297f0458944e37e2b620a13 Mon Sep 17 00:00:00 2001 From: liranmauda Date: Thu, 12 Sep 2024 12:17:42 +0300 Subject: [PATCH 1/4] Fixing the CLI decision error handling to avoid lint errors Fixing the CLI decision error handling to avoid lint errors Signed-off-by: liranmauda (cherry picked from commit 10d81d11363b2f0a5ecd9693e2ad82785f7d6a25) --- pkg/backingstore/backingstore.go | 6 +++--- pkg/install/install.go | 10 ++++++---- pkg/namespacestore/namespacestore.go | 6 +++--- pkg/noobaaaccount/noobaaaccount.go | 4 +++- pkg/obc/obc.go | 4 +++- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pkg/backingstore/backingstore.go b/pkg/backingstore/backingstore.go index fd92b75a1..73c29f9ee 100644 --- a/pkg/backingstore/backingstore.go +++ b/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`) } } diff --git a/pkg/install/install.go b/pkg/install/install.go index 9e9a01bae..5d81f8981 100644 --- a/pkg/install/install.go +++ b/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("") diff --git a/pkg/namespacestore/namespacestore.go b/pkg/namespacestore/namespacestore.go index 27ad9c280..85eb1bdc6 100644 --- a/pkg/namespacestore/namespacestore.go +++ b/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`) } } diff --git a/pkg/noobaaaccount/noobaaaccount.go b/pkg/noobaaaccount/noobaaaccount.go index 97ce88b85..47f97f69f 100644 --- a/pkg/noobaaaccount/noobaaaccount.go +++ b/pkg/noobaaaccount/noobaaaccount.go @@ -364,7 +364,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" { diff --git a/pkg/obc/obc.go b/pkg/obc/obc.go index 5965f0081..f528e26f0 100644 --- a/pkg/obc/obc.go +++ b/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" { From 28f2a2fd21ec0788678ada040a82f47060829f56 Mon Sep 17 00:00:00 2001 From: liranmauda Date: Thu, 12 Sep 2024 12:15:40 +0300 Subject: [PATCH 2/4] Using golangci-lint for lint in the Makefile target - Using `golangci-lint` for lint in the Makefile target - Adding `.golangci.yml` as a configuration file - Fixing some lint errors Signed-off-by: liranmauda (cherry picked from commit 496b2ddc8281b24d0bd142342f27535a3b92ea26) --- .golangci.yml | 9 +++++++++ Makefile | 4 +++- pkg/backingstore/backingstore.go | 2 +- pkg/bucketclass/bucketclass.go | 5 +++-- pkg/cosi/provisioner.go | 4 ++-- pkg/namespacestore/namespacestore.go | 2 +- pkg/nb/rpc_ws.go | 2 +- pkg/obc/provisioner.go | 11 ++++++----- pkg/system/reconciler.go | 4 ++-- pkg/util/kms/test/dev/kms_dev_test.go | 2 +- pkg/util/kms/test/ibm-kp/kms_ibm_kp_test.go | 2 +- 11 files changed, 30 insertions(+), 17 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..8484c07f0 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,9 @@ +run: + timeout: 5m + +issues: + exclude-dirs: + - pkg/apis/noobaa/v1alpha1 + - pkg/bundle + exclude-files: + - zz_generated\.go diff --git a/Makefile b/Makefile index d2798eb5c..95b930b18 100644 --- a/Makefile +++ b/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 diff --git a/pkg/backingstore/backingstore.go b/pkg/backingstore/backingstore.go index 73c29f9ee..a45ac0acd 100644 --- a/pkg/backingstore/backingstore.go +++ b/pkg/backingstore/backingstore.go @@ -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{ diff --git a/pkg/bucketclass/bucketclass.go b/pkg/bucketclass/bucketclass.go index 8d0bf01b3..33b0e4cf2 100644 --- a/pkg/bucketclass/bucketclass.go +++ b/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 diff --git a/pkg/cosi/provisioner.go b/pkg/cosi/provisioner.go index 9208189e3..984ed2a8a 100644 --- a/pkg/cosi/provisioner.go +++ b/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) } } diff --git a/pkg/namespacestore/namespacestore.go b/pkg/namespacestore/namespacestore.go index 85eb1bdc6..87b7038a2 100644 --- a/pkg/namespacestore/namespacestore.go +++ b/pkg/namespacestore/namespacestore.go @@ -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{ diff --git a/pkg/nb/rpc_ws.go b/pkg/nb/rpc_ws.go index d737fa580..574028e83 100644 --- a/pkg/nb/rpc_ws.go +++ b/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 { diff --git a/pkg/obc/provisioner.go b/pkg/obc/provisioner.go index 23a8027a0..a6aaebe97 100644 --- a/pkg/obc/provisioner.go +++ b/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 diff --git a/pkg/system/reconciler.go b/pkg/system/reconciler.go index b94018a6a..159421d80 100644 --- a/pkg/system/reconciler.go +++ b/pkg/system/reconciler.go @@ -519,8 +519,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) diff --git a/pkg/util/kms/test/dev/kms_dev_test.go b/pkg/util/kms/test/dev/kms_dev_test.go index 09b2258a9..c1fc4921c 100644 --- a/pkg/util/kms/test/dev/kms_dev_test.go +++ b/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) diff --git a/pkg/util/kms/test/ibm-kp/kms_ibm_kp_test.go b/pkg/util/kms/test/ibm-kp/kms_ibm_kp_test.go index 1b9ed1b97..aee7a0dfa 100644 --- a/pkg/util/kms/test/ibm-kp/kms_ibm_kp_test.go +++ b/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) From 272d99ac5e4248d3301bfdeb6c9610f4e30000ee Mon Sep 17 00:00:00 2001 From: Kaustav Majumder Date: Mon, 7 Oct 2024 12:10:35 +0530 Subject: [PATCH 3/4] Minor lint fixes Signed-off-by: Kaustav Majumder (cherry picked from commit 11392ab55a13877cc7198003e7aaaa0829c7d35a) --- pkg/obc/provisioner.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/obc/provisioner.go b/pkg/obc/provisioner.go index a6aaebe97..679affb64 100644 --- a/pkg/obc/provisioner.go +++ b/pkg/obc/provisioner.go @@ -577,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 @@ -587,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, From 9f845db304ba296e9e123c754788e4a146b2878d Mon Sep 17 00:00:00 2001 From: liranmauda Date: Sun, 27 Oct 2024 12:23:42 +0200 Subject: [PATCH 4/4] Removing golangci-lint-action in favor or make lint Removing golangci-lint-action in favor or make lint Signed-off-by: liranmauda (cherry picked from commit 88564d2307e53c58e1fe42edb79c0017356c3c25) --- .github/workflows/golangci-lint.yml | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 18b03aa96..0b3846b45 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.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