fixed various sonarCloud high severity issues

.gitignore

@@ -9,6 +9,7 @@
 testbin/*
 .DS_STORE
 .vscode/*
+venv/
 
 bin/
 
@@ -38,4 +39,4 @@ test/function_tests/results/*
 
 test/mock-component-image/bin
 test/mock-component-image/scripts/_pycache_
-hack/bundle-automation/tmp/
+hack/bundle-automation/tmp/

Dockerfile.rhtap

@@ -17,8 +17,8 @@ COPY controllers/ controllers/
 COPY pkg/ pkg/
 
 # Build
-RUN CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go mod vendor
-RUN CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go build -a -o backplane-operator main.go
+RUN CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go mod vendor &&\
+go build -a -o backplane-operator main.go
 
 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details

Dockerfile.test.prow

@@ -8,8 +8,8 @@ COPY test/function_tests/ test/function_tests/
 COPY go.mod go.mod
 COPY go.sum go.sum
 
-RUN go install github.com/onsi/ginkgo/v2/ginkgo@latest
-RUN ginkgo build test/function_tests/backplane_operator_install_test
+RUN go install github.com/onsi/ginkgo/v2/ginkgo@latest &&\
+ginkgo build test/function_tests/backplane_operator_install_test
 
 FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
 

controllers/backplaneconfig_controller.go

@@ -87,7 +87,8 @@ const (
 	trustBundleNameEnvVar  = "TRUSTED_CA_BUNDLE"
 	defaultTrustBundleName = "trusted-ca-bundle"
 
-	controlPlane = "backplane-operator"
+	controlPlane        = "backplane-operator"
+	backplaneConfigName = "backplaneconfig.name"
 )
 
 var (
@@ -529,21 +530,21 @@ func (r *MultiClusterEngineReconciler) SetupWithManager(mgr ctrl.Manager) error
 			DeleteFunc: func(ctx context.Context, e event.DeleteEvent, q workqueue.RateLimitingInterface) {
 				labels := e.Object.GetLabels()
 				q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
-					Name: labels["backplaneconfig.name"],
+					Name: labels[backplaneConfigName],
 				}})
 			},
 		}, builder.WithPredicates(predicate.LabelChangedPredicate{})).
 		Watches(&clustermanager.ClusterManager{}, &handler.Funcs{
 			DeleteFunc: func(ctx context.Context, e event.DeleteEvent, q workqueue.RateLimitingInterface) {
 				labels := e.Object.GetLabels()
 				q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
-					Name: labels["backplaneconfig.name"],
+					Name: labels[backplaneConfigName],
 				}})
 			},
 			UpdateFunc: func(ctx context.Context, e event.UpdateEvent, q workqueue.RateLimitingInterface) {
 				labels := e.ObjectOld.GetLabels()
 				q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
-					Name: labels["backplaneconfig.name"],
+					Name: labels[backplaneConfigName],
 				}})
 			},
 		}, builder.WithPredicates(predicate.LabelChangedPredicate{}))
@@ -552,7 +553,7 @@ func (r *MultiClusterEngineReconciler) SetupWithManager(mgr ctrl.Manager) error
 		mceBuilder.Watches(&monitorv1.ServiceMonitor{}, &handler.Funcs{
 			DeleteFunc: func(ctx context.Context, e event.DeleteEvent, q workqueue.RateLimitingInterface) {
 				labels := e.Object.GetLabels()
-				if label, ok := labels["backplaneconfig.name"]; ok {
+				if label, ok := labels[backplaneConfigName]; ok {
 					q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
 						Name: label,
 					}})
@@ -1315,7 +1316,7 @@ func (r *MultiClusterEngineReconciler) applyTemplate(ctx context.Context,
 		// Apply the object data.
 		force := true
 		err := r.Client.Patch(ctx, template, client.Apply,
-			&client.PatchOptions{Force: &force, FieldManager: "backplane-operator"})
+			&client.PatchOptions{Force: &force, FieldManager: controlPlane})
 
 		if err != nil {
 			errMessage := fmt.Errorf(
@@ -1379,7 +1380,7 @@ func (r *MultiClusterEngineReconciler) ensureCustomResources(ctx context.Context
 			}
 			force := true
 			err := r.Client.Patch(ctx, addonTemplate, client.Apply,
-				&client.PatchOptions{Force: &force, FieldManager: "backplane-operator"})
+				&client.PatchOptions{Force: &force, FieldManager: controlPlane})
 			if err != nil {
 				return ctrl.Result{}, pkgerrors.Wrapf(err, "error applying object Name: %s Kind: %s",
 					addonTemplate.GetName(), addonTemplate.GetKind())

controllers/mcewebhook/webhook_controller.go

@@ -3,6 +3,8 @@ package mcewebhook
 import (
 	"context"
 	"fmt"
+	"time"
+
 	"github.com/go-logr/logr"
 	backplanev1 "github.com/stolostron/backplane-operator/api/v1"
 	"github.com/stolostron/backplane-operator/pkg/utils"
@@ -18,7 +20,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"time"
 )
 
 const (

controllers/toggle_components.go

@@ -33,6 +33,10 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	localCluster = "local-cluster"
+)
+
 var clusterManagementAddOnGVK = schema.GroupVersionKind{
 	Group:   "addon.open-cluster-management.io",
 	Version: "v1alpha1",
@@ -412,7 +416,7 @@ func (r *MultiClusterEngineReconciler) ensureNoDiscovery(ctx context.Context,
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -510,7 +514,7 @@ func (r *MultiClusterEngineReconciler) ensureNoHive(ctx context.Context, mce *ba
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -597,7 +601,7 @@ func (r *MultiClusterEngineReconciler) ensureNoAssistedService(ctx context.Conte
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -691,7 +695,7 @@ func (r *MultiClusterEngineReconciler) ensureNoServerFoundation(ctx context.Cont
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -769,7 +773,7 @@ func (r *MultiClusterEngineReconciler) ensureNoImageBasedInstallOperator(ctx con
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -870,7 +874,7 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterLifecycle(ctx context.Cont
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -923,7 +927,7 @@ func (r *MultiClusterEngineReconciler) ensureClusterManager(ctx context.Context,
 		return ctrl.Result{}, errors.Wrapf(err, "Error setting controller reference on resource %s", cmTemplate.GetName())
 	}
 	force := true
-	err := r.Client.Patch(ctx, cmTemplate, client.Apply, &client.PatchOptions{Force: &force, FieldManager: "backplane-operator"})
+	err := r.Client.Patch(ctx, cmTemplate, client.Apply, &client.PatchOptions{Force: &force, FieldManager: controlPlane})
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "error applying object Name: %s Kind: %s", cmTemplate.GetName(), cmTemplate.GetKind())
 	}
@@ -991,7 +995,7 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterManager(ctx context.Contex
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -1111,7 +1115,7 @@ func (r *MultiClusterEngineReconciler) ensureNoHyperShift(ctx context.Context,
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -1153,7 +1157,7 @@ func (r *MultiClusterEngineReconciler) reconcileHypershiftLocalHosting(ctx conte
 		return r.removeHypershiftLocalHosting(ctx, mce)
 	}
 
-	localNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "local-cluster"}}
+	localNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: localCluster}}
 	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: localNS.GetName()}, localNS)
 	if apierrors.IsNotFound(err) {
 		// wait for local-cluster namespace
@@ -1335,7 +1339,7 @@ func (r *MultiClusterEngineReconciler) ensureNoClusterProxyAddon(ctx context.Con
 	for _, template := range templates {
 		result, err := r.deleteTemplate(ctx, mce, template)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("Failed to delete template: %s", template.GetName()))
+			logTemplateDeletionError(err, template.GetName())
 			return result, err
 		}
 	}
@@ -1387,7 +1391,7 @@ func (r *MultiClusterEngineReconciler) ensureLocalCluster(ctx context.Context, m
 		return ctrl.Result{}, nil
 	}
 
-	nsn := types.NamespacedName{Name: "local-cluster", Namespace: mce.Spec.TargetNamespace}
+	nsn := types.NamespacedName{Name: localCluster, Namespace: mce.Spec.TargetNamespace}
 	lcs := status.LocalClusterStatus{NamespacedName: nsn, Enabled: true}
 	r.StatusManager.RemoveComponent(lcs)
 	r.StatusManager.AddComponent(lcs)
@@ -1415,14 +1419,14 @@ func (r *MultiClusterEngineReconciler) ensureLocalCluster(ctx context.Context, m
 					log.Info("ManagedCluster webhook not available, waiting for controller")
 					r.StatusManager.RemoveComponent(lcs)
 					r.StatusManager.AddComponent(status.StaticStatus{
-						NamespacedName: types.NamespacedName{Name: "local-cluster", Namespace: mce.Spec.TargetNamespace},
-						Kind:           "local-cluster",
+						NamespacedName: types.NamespacedName{Name: localCluster, Namespace: mce.Spec.TargetNamespace},
+						Kind:           localCluster,
 						Condition: backplanev1.ComponentCondition{
 							Type:      "Available",
-							Name:      "local-cluster",
+							Name:      localCluster,
 							Status:    metav1.ConditionFalse,
 							Reason:    status.WaitingForResourceReason,
-							Kind:      "local-cluster",
+							Kind:      localCluster,
 							Available: false,
 							Message:   "Waiting for ManagedCluster webhook",
 						},
@@ -1445,14 +1449,14 @@ func (r *MultiClusterEngineReconciler) ensureLocalCluster(ctx context.Context, m
 		// managedCluster CRD does not yet exist. Replace status.
 		r.StatusManager.RemoveComponent(lcs)
 		r.StatusManager.AddComponent(status.StaticStatus{
-			NamespacedName: types.NamespacedName{Name: "local-cluster", Namespace: mce.Spec.TargetNamespace},
-			Kind:           "local-cluster",
+			NamespacedName: types.NamespacedName{Name: localCluster, Namespace: mce.Spec.TargetNamespace},
+			Kind:           localCluster,
 			Condition: backplanev1.ComponentCondition{
 				Type:      "Available",
-				Name:      "local-cluster",
+				Name:      localCluster,
 				Status:    metav1.ConditionFalse,
 				Reason:    status.WaitingForResourceReason,
-				Kind:      "local-cluster",
+				Kind:      localCluster,
 				Available: false,
 				Message:   "Waiting for ManagedCluster CRD to be available",
 			},
@@ -1463,14 +1467,14 @@ func (r *MultiClusterEngineReconciler) ensureLocalCluster(ctx context.Context, m
 		log.Info("ManagedCluster webhook not available, waiting for controller")
 		r.StatusManager.RemoveComponent(lcs)
 		r.StatusManager.AddComponent(status.StaticStatus{
-			NamespacedName: types.NamespacedName{Name: "local-cluster", Namespace: mce.Spec.TargetNamespace},
-			Kind:           "local-cluster",
+			NamespacedName: types.NamespacedName{Name: localCluster, Namespace: mce.Spec.TargetNamespace},
+			Kind:           localCluster,
 			Condition: backplanev1.ComponentCondition{
 				Type:      "Available",
-				Name:      "local-cluster",
+				Name:      localCluster,
 				Status:    metav1.ConditionFalse,
 				Reason:    status.WaitingForResourceReason,
-				Kind:      "local-cluster",
+				Kind:      localCluster,
 				Available: false,
 				Message:   "Waiting for ManagedCluster webhook",
 			},
@@ -1519,7 +1523,7 @@ func (r *MultiClusterEngineReconciler) ensureNoLocalCluster(ctx context.Context,
 		return ctrl.Result{}, nil
 	}
 
-	nsn := types.NamespacedName{Name: "local-cluster", Namespace: mce.Spec.TargetNamespace}
+	nsn := types.NamespacedName{Name: localCluster, Namespace: mce.Spec.TargetNamespace}
 	lcs := status.LocalClusterStatus{
 		NamespacedName: nsn,
 		Enabled:        false,
@@ -1618,3 +1622,7 @@ func applyReleaseVersionAnnotation(template *unstructured.Unstructured) {
 	annotations[utils.AnnotationReleaseVersion] = version.Version
 	template.SetAnnotations(annotations)
 }
+
+func logTemplateDeletionError(err error, name string) {
+	log.Error(err, fmt.Sprintf("Failed to delete template: %s", name))
+}

controllers/uninstall.go

@@ -18,6 +18,10 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
+const (
+	rbacAuthorization = "rbac.authorization.k8s.io"
+)
+
 var (
 	// The uninstallList is the list of all resources from previous installs to remove. Items can be removed
 	// from this list in future releases if they are sure to not exist prior to the current installer version
@@ -33,19 +37,19 @@ var (
 			),
 			newUnstructured(
 				types.NamespacedName{Name: "open-cluster-management:hypershift-preview:hypershiftDeployment-leader-election", Namespace: backplaneConfig.Spec.TargetNamespace},
-				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Kind: "Role", Version: "v1"},
+				schema.GroupVersionKind{Group: rbacAuthorization, Kind: "Role", Version: "v1"},
 			),
 			newUnstructured(
 				types.NamespacedName{Name: "open-cluster-management:hypershift-preview:hypershiftDeployment-leader-election", Namespace: backplaneConfig.Spec.TargetNamespace},
-				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Kind: "RoleBinding", Version: "v1"},
+				schema.GroupVersionKind{Group: rbacAuthorization, Kind: "RoleBinding", Version: "v1"},
 			),
 			newUnstructured(
 				types.NamespacedName{Name: "open-cluster-management:hypershift-preview:hypershift-deployment-controller"},
-				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Kind: "ClusterRole", Version: "v1"},
+				schema.GroupVersionKind{Group: rbacAuthorization, Kind: "ClusterRole", Version: "v1"},
 			),
 			newUnstructured(
 				types.NamespacedName{Name: "open-cluster-management:hypershift-preview:hypershift-deployment-controller"},
-				schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Kind: "ClusterRoleBinding", Version: "v1"},
+				schema.GroupVersionKind{Group: rbacAuthorization, Kind: "ClusterRoleBinding", Version: "v1"},
 			),
 			// managed-serviceaccount
 			// TODO: Remove this in the next release

pkg/utils/utils.go

@@ -7,12 +7,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"path"
+	"path/filepath"
+
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
-	"path"
-	"path/filepath"
 
 	"os"
 
@@ -388,20 +389,24 @@ func DumpServingCertSecret() error {
 		case os.IsNotExist(err):
 			// create file
 			if err := os.WriteFile(filename, data, 0600); err != nil {
-				return fmt.Errorf("unable to write file %q: %w", filename, err)
+				return fileWriteError(filename, err)
 			}
 		case err != nil:
-			return fmt.Errorf("unable to write file %q: %w", filename, err)
+			return fileWriteError(filename, err)
 		case bytes.Equal(lastData, data):
 			// skip file without any change
 			continue
 		default:
 			// update file
 			if err := os.WriteFile(path.Clean(filename), data, 0600); err != nil {
-				return fmt.Errorf("unable to write file %q: %w", filename, err)
+				return fileWriteError(filename, err)
 			}
 		}
 	}
 
 	return nil
 }
+
+func fileWriteError(filename string, err error) error {
+	return fmt.Errorf("unable to write file %q: %w", filename, err)
+}