From e27dcdd41992af60638f85b97219802eb0ded5b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Miguel=20Olmo=20Mart=C3=ADnez?= Date: Mon, 23 Dec 2019 10:01:59 +0100 Subject: [PATCH] ceph: Remove ineffectual assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Description of your changes: Fix security issues identified by TrailOfBits Modifications in osd/status.go: I opted by remove the offending variables because "c.checkNodesCompleted" provides this variables always, so no need to initialize them. var was not used, so i have replaced it by the blank identifier. Modifications in machinelabel/add.go: I have followed the same criteria used in other disruption packages, and return the error if it is produced when we try to "watch" machines. Closes: https://github.com/rook/rook/issues/4565 [test ceph] Signed-off-by: Juan Miguel Olmo Martínez --- pkg/operator/ceph/cluster/osd/status.go | 4 +--- pkg/operator/ceph/disruption/machinelabel/add.go | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/operator/ceph/cluster/osd/status.go b/pkg/operator/ceph/cluster/osd/status.go index e993e15e8..547650463 100644 --- a/pkg/operator/ceph/cluster/osd/status.go +++ b/pkg/operator/ceph/cluster/osd/status.go @@ -199,9 +199,7 @@ func (c *Cluster) completeOSDsForAllNodes(config *provisionConfig, configOSDs bo logger.Infof("orchestration status config map result channel closed, will restart watch.") w.Stop() <-time.After(5 * time.Second) - leftNodes := 0 - leftRemainingNodes := util.NewSet() - leftNodes, leftRemainingNodes, completed, statuses, err = c.checkNodesCompleted(selector, config, configOSDs) + leftNodes, leftRemainingNodes, completed, _, err := c.checkNodesCompleted(selector, config, configOSDs) if err == nil { if completed { logger.Infof("additional %d/%d node(s) completed osd provisioning", leftNodes, originalNodes) diff --git a/pkg/operator/ceph/disruption/machinelabel/add.go b/pkg/operator/ceph/disruption/machinelabel/add.go index f15913b34..ced66cae1 100644 --- a/pkg/operator/ceph/disruption/machinelabel/add.go +++ b/pkg/operator/ceph/disruption/machinelabel/add.go @@ -18,6 +18,7 @@ package machinelabel import ( mapiv1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" + "github.com/pkg/errors" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/operator/ceph/disruption/controllerconfig" corev1 "k8s.io/api/core/v1" @@ -53,7 +54,7 @@ func Add(mgr manager.Manager, context *controllerconfig.Context) error { // create a new controller c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: reconciler}) if err != nil { - return err + return errors.Wrapf(err, "could not create controller %q", controllerName) } // Watch for the machines and enqueue the machineRequests if the machine is occupied by the osd pods @@ -71,6 +72,9 @@ func Add(mgr manager.Manager, context *controllerconfig.Context) error { return []reconcile.Request{req} }), }) + if err != nil { + return errors.Wrap(err, "could not watch machines") + } // Watch for the osd pods and enqueue the CephCluster in the namespace from the pods return c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestsFromMapFunc{