Skip to content

Commit

Permalink
fixed various sonarCloud high severity issues
Browse files Browse the repository at this point in the history 
Signed-off-by: Nathaniel Graham <[email protected]>
  • Loading branch information
@ngraham20
ngraham20 committed Nov 19, 2024
1 parent 342d6fa commit 0849489
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
testbin/*
.DS_STORE
.vscode/*
venv/

bin/

Expand Down Expand Up @@ -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/
4 changes: 2 additions & 2 deletions Dockerfile.rhtap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.test.prow
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 8 additions & 7 deletions controllers/backplaneconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ const (
trustBundleNameEnvVar = "TRUSTED_CA_BUNDLE"
defaultTrustBundleName = "trusted-ca-bundle"

controlPlane = "backplane-operator"
controlPlane = "backplane-operator"
backplaneConfigName = "backplaneconfig.name"
)

var (
Expand Down Expand Up @@ -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{}))
Expand All @@ -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,
}})
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 2 additions & 1 deletion controllers/mcewebhook/webhook_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down
58 changes: 33 additions & 25 deletions controllers/toggle_components.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))
}
12 changes: 8 additions & 4 deletions controllers/uninstall.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
15 changes: 10 additions & 5 deletions pkg/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}

0 comments on commit 0849489

Please sign in to comment.