From 71ddf51544be498eee03734573b765eb04480bb9 Mon Sep 17 00:00:00 2001 From: Yug Date: Tue, 21 Jul 2020 10:40:13 +0530 Subject: [PATCH] cleanup: address gomnd warnings Direct usage of numbers should be avoided. Issue reported: mnd: Magic number: X, in detected (gomnd) Signed-off-by: Yug --- cmd/cephcsi.go | 7 +++++-- internal/cephfs/volumemounter.go | 6 +++++- internal/csi-common/server.go | 3 ++- internal/journal/voljournal.go | 2 +- internal/rbd/clone.go | 10 ++++++---- internal/rbd/nodeserver.go | 1 + internal/rbd/rbd_util.go | 8 +++----- internal/util/topology.go | 3 ++- internal/util/util.go | 9 ++++++--- internal/util/vault.go | 4 ++-- internal/util/volid.go | 7 +++++-- 11 files changed, 38 insertions(+), 22 deletions(-) diff --git a/cmd/cephcsi.go b/cmd/cephcsi.go index c4c73d55c..eaf386c9b 100644 --- a/cmd/cephcsi.go +++ b/cmd/cephcsi.go @@ -39,6 +39,9 @@ const ( rbdDefaultName = "rbd.csi.ceph.com" cephfsDefaultName = "cephfs.csi.ceph.com" livenessDefaultName = "liveness.csi.ceph.com" + + pollTime = 60 // seconds + probeTimeout = 3 // seconds ) var ( @@ -65,8 +68,8 @@ func init() { // liveness/grpc metrics related flags flag.IntVar(&conf.MetricsPort, "metricsport", 8080, "TCP port for liveness/grpc metrics requests") flag.StringVar(&conf.MetricsPath, "metricspath", "/metrics", "path of prometheus endpoint where metrics will be available") - flag.DurationVar(&conf.PollTime, "polltime", time.Second*60, "time interval in seconds between each poll") - flag.DurationVar(&conf.PoolTimeout, "timeout", time.Second*3, "probe timeout in seconds") + flag.DurationVar(&conf.PollTime, "polltime", time.Second*pollTime, "time interval in seconds between each poll") + flag.DurationVar(&conf.PoolTimeout, "timeout", time.Second*probeTimeout, "probe timeout in seconds") flag.BoolVar(&conf.EnableGRPCMetrics, "enablegrpcmetrics", false, "[DEPRECATED] enable grpc metrics") flag.StringVar(&conf.HistogramOption, "histogramoption", "0.5,2,6", diff --git a/internal/cephfs/volumemounter.go b/internal/cephfs/volumemounter.go index 67e811e4a..52039690b 100644 --- a/internal/cephfs/volumemounter.go +++ b/internal/cephfs/volumemounter.go @@ -47,6 +47,7 @@ var ( fusePidRx = regexp.MustCompile(`(?m)^ceph-fuse\[(.+)\]: starting fuse$`) + // nolint:gomnd // numbers specify Kernel versions. quotaSupport = []util.KernelVersion{ { Version: 4, @@ -176,7 +177,10 @@ func mountFuse(ctx context.Context, mountPoint string, cr *util.Credentials, vol // and PID of the ceph-fuse daemon for unmount match := fusePidRx.FindSubmatch(stderr) - if len(match) != 2 { + // validMatchLength is set to 2 as match is expected + // to have 2 items, starting fuse and PID of the fuse daemon + const validMatchLength = 2 + if len(match) != validMatchLength { return fmt.Errorf("ceph-fuse failed: %s", stderr) } diff --git a/internal/csi-common/server.go b/internal/csi-common/server.go index 1bc1baedb..b35852a32 100644 --- a/internal/csi-common/server.go +++ b/internal/csi-common/server.go @@ -118,7 +118,8 @@ func (s *nonBlockingGRPCServer) serve(endpoint, hstOptions string, ids csi.Ident util.DefaultLog("Listening for connections on address: %#v", listener.Addr()) if metrics { ho := strings.Split(hstOptions, ",") - if len(ho) != 3 { + const expectedHo = 3 + if len(ho) != expectedHo { klog.Fatalf("invalid histogram options provided: %v", hstOptions) } start, e := strconv.ParseFloat(ho[0], 32) diff --git a/internal/journal/voljournal.go b/internal/journal/voljournal.go index e79ff65b7..a65599fb8 100644 --- a/internal/journal/voljournal.go +++ b/internal/journal/voljournal.go @@ -401,7 +401,7 @@ func (conn *Connection) UndoReservation(ctx context.Context, cj := conn.config if volName != "" { - if len(volName) < 36 { + if len(volName) < uuidEncodedLength { return fmt.Errorf("unable to parse UUID from %s, too short", volName) } diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index 7e219773b..8f066c0f3 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -234,11 +234,13 @@ func (rv *rbdVolume) flattenCloneImage(ctx context.Context) error { // can flatten the parent images before use to avoid the stale omap entries. hardLimit := rbdHardMaxCloneDepth softLimit := rbdSoftMaxCloneDepth - if rbdHardMaxCloneDepth < 2 { - hardLimit = rbdHardMaxCloneDepth - 2 + // choosing 2 so that we don't need to flatten the image in the request. + const depthToAvoidFlatten = 2 + if rbdHardMaxCloneDepth < depthToAvoidFlatten { + hardLimit = rbdHardMaxCloneDepth - depthToAvoidFlatten } - if rbdSoftMaxCloneDepth < 2 { - softLimit = rbdSoftMaxCloneDepth - 2 + if rbdSoftMaxCloneDepth < depthToAvoidFlatten { + softLimit = rbdSoftMaxCloneDepth - depthToAvoidFlatten } err := tempClone.getImageInfo() if err == nil { diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 3ac55dbf1..148f85d08 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -65,6 +65,7 @@ var ( kernelRelease = "" // deepFlattenSupport holds the list of kernel which support mapping rbd // image with deep-flatten image feature + // nolint:gomnd // numbers specify Kernel versions. deepFlattenSupport = []util.KernelVersion{ { Version: 5, diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index cf734c5cd..b427dd6ae 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -62,9 +62,6 @@ const ( rbdImageRequiresEncryption = "requiresEncryption" // image metadata key for encryption encryptionMetaKey = ".rbd.csi.ceph.com/encrypted" - - // go-ceph will provide rbd.ImageOptionCloneFormat - imageOptionCloneFormat = librbd.RbdImageOption(12) ) // rbdVolume represents a CSI volume and its RBD image specifics. @@ -856,7 +853,7 @@ func (rv *rbdVolume) cloneRbdImageFromSnapshot(ctx context.Context, pSnapOpts *r } } - err = options.SetUint64(imageOptionCloneFormat, 2) + err = options.SetUint64(librbd.ImageOptionCloneFormat, 2) if err != nil { return fmt.Errorf("failed to set image features: %w", err) } @@ -1006,7 +1003,8 @@ func (ri *rbdImageMetadataStash) String() string { // JSON format. func stashRBDImageMetadata(volOptions *rbdVolume, path string) error { var imgMeta = rbdImageMetadataStash{ - Version: 2, // there are no checks for this at present + // there are no checks for this at present + Version: 2, // nolint:gomnd // number specifies version. Pool: volOptions.Pool, ImageName: volOptions.RbdImageName, Encrypted: volOptions.Encrypted, diff --git a/internal/util/topology.go b/internal/util/topology.go index c6abd5e0c..014479aa9 100644 --- a/internal/util/topology.go +++ b/internal/util/topology.go @@ -52,7 +52,8 @@ func GetTopologyFromDomainLabels(domainLabels, nodeName, driverName string) (map // size checks on domain label prefix topologyPrefix := strings.ToLower("topology." + driverName) - if len(topologyPrefix) > 63 { + const lenLimit = 63 + if len(topologyPrefix) > lenLimit { return nil, fmt.Errorf("computed topology label prefix (%s) for node exceeds length limits", topologyPrefix) } // driverName is validated, and we are adding a lowercase "topology." to it, so no validation for conformance diff --git a/internal/util/util.go b/internal/util/util.go index b59971048..33551e538 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -125,7 +125,8 @@ func ValidateDriverName(driverName string) error { return errors.New("driver name is empty") } - if len(driverName) > 63 { + const reqDriverNameLen = 63 + if len(driverName) > reqDriverNameLen { return errors.New("driver name length should be less than 63 chars") } var err error @@ -191,7 +192,8 @@ func CheckKernelSupport(release string, supportedVersions []KernelVersion) bool return false } sublevel := 0 - if len(vers) >= 3 { + const minLenForSublvl = 3 + if len(vers) >= minLenForSublvl { sublevel, err = strconv.Atoi(vers[2]) if err != nil { klog.Errorf("failed to parse sublevel from %s: %v", release, err) @@ -200,7 +202,8 @@ func CheckKernelSupport(release string, supportedVersions []KernelVersion) bool } extra := strings.SplitN(release, "-", 2) extraversion := 0 - if len(extra) == 2 { + const expectedExtraLen = 2 + if len(extra) == expectedExtraLen { // ignore errors, 1st component of extraversion does not need to be an int extraversion, err = strconv.Atoi(strings.Split(extra[1], ".")[0]) if err != nil { diff --git a/internal/util/vault.go b/internal/util/vault.go index c428b01c1..b34509af3 100644 --- a/internal/util/vault.go +++ b/internal/util/vault.go @@ -151,7 +151,7 @@ func (kms *VaultKMS) GetPassphrase(key string) (string, error) { } defer resp.Body.Close() - if resp.StatusCode == 404 { + if resp.StatusCode == http.StatusNotFound { return "", MissingPassphrase{fmt.Errorf("passphrase for %s not found", key)} } err = kms.processError(resp, fmt.Sprintf("get passphrase for %s", key)) @@ -219,7 +219,7 @@ func (kms *VaultKMS) DeletePassphrase(key string) error { return fmt.Errorf("delete passphrase at %s request to vault failed: %s", key, err) } defer resp.Body.Close() - if resp.StatusCode != 404 { + if resp.StatusCode != http.StatusNotFound { err = kms.processError(resp, "delete passphrase") if err != nil { return err diff --git a/internal/util/volid.go b/internal/util/volid.go index 270487b2b..281a79336 100644 --- a/internal/util/volid.go +++ b/internal/util/volid.go @@ -127,9 +127,12 @@ func (ci *CSIIdentifier) DecomposeCSIID(composedCSIID string) (err error) { ci.ClusterID = composedCSIID[10 : 10+clusterIDLength] // additional 1 for '-' separator bytesToProcess -= (clusterIDLength + 1) - nextFieldStartIdx := 10 + clusterIDLength + 1 + nextFieldStartIdx := (10 + clusterIDLength + 1) - if bytesToProcess < 17 { + // minLenToDecode is now 17 as composedCSIID should include + // atleast 16 for poolID encoding and 1 for '-' separator. + const minLenToDecode = 17 + if bytesToProcess < minLenToDecode { return errors.New("failed to decode CSI identifier, string underflow") } buf64, err := hex.DecodeString(composedCSIID[nextFieldStartIdx : nextFieldStartIdx+16])