From 232855b1f4433eab38bc87627eeec92bbd04cd22 Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 14:45:16 +0900 Subject: [PATCH 01/14] Add nolint for gosec. ignore G404: Use of weak random number generator (math/rand instead of crypto/rand) --- pkg/cloud/cloud.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index f4ea44c..53b34d7 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -150,13 +150,13 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * createType = VolumeTypeMapping[diskOptions.VolumeType] case VolumeTypeHighSpeed: types := []string{VolumeTypeHighSpeedA, VolumeTypeHighSpeedB} - createType = VolumeTypeMapping[types[rand.Intn(len(types))]] + createType = VolumeTypeMapping[types[rand.Intn(len(types))]] //nolint:gosec // Strong random is unnecessary case VolumeTypeStandardFlash: types := []string{VolumeTypeStandardFlashA, VolumeTypeStandardFlashB} - createType = VolumeTypeMapping[types[rand.Intn(len(types))]] + createType = VolumeTypeMapping[types[rand.Intn(len(types))]] //nolint:gosec // Strong random is unnecessary case VolumeTypeHighSpeedFlash: types := []string{VolumeTypeHighSpeedFlashA, VolumeTypeHighSpeedFlashB} - createType = VolumeTypeMapping[types[rand.Intn(len(types))]] + createType = VolumeTypeMapping[types[rand.Intn(len(types))]] //nolint:gosec // Strong random is unnecessary case "": createType = VolumeTypeMapping[DefaultVolumeType] default: From 476dd9a28c4dd2294c11b35a0860312c0ba6a206 Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 14:50:13 +0900 Subject: [PATCH 02/14] Fix gosec G601: Implicit memory aliasing in for loop. --- pkg/cloud/cloud.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 53b34d7..869ca13 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -357,7 +357,7 @@ func (c *cloud) ListDisks(ctx context.Context) ([]*Disk, error) { } disks := []*Disk{} - for _, volume := range resp.VolumeSet { + for i, volume := range resp.VolumeSet { // Volume name was setted in volume description. // So use description to check this volume was created by Kubernetes CSI driver. if !strings.HasPrefix(nifcloud.ToString(volume.Description), "pvc-") { @@ -374,7 +374,7 @@ func (c *cloud) ListDisks(ctx context.Context) ([]*Disk, error) { VolumeID: nifcloud.ToString(volume.VolumeId), CapacityGiB: int64(volSize), AvailabilityZone: nifcloud.ToString(volume.AvailabilityZone), - AttachedInstanceID: getVolumeAttachedInstanceID(&volume), + AttachedInstanceID: getVolumeAttachedInstanceID(&resp.VolumeSet[i]), }) } @@ -426,9 +426,9 @@ func (c *cloud) GetDiskByName(ctx context.Context, name string, capacityBytes in } var volume *types.VolumeSet - for _, vol := range resp.VolumeSet { + for i, vol := range resp.VolumeSet { if nifcloud.ToString(vol.Description) == name { - volume = &vol + volume = &resp.VolumeSet[i] } } if volume == nil { From 818c00fe21a1c5e5c122cd820c451370f1d27a54 Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 14:53:21 +0900 Subject: [PATCH 03/14] Fix errorlint non-wrapping format verb for fmt.Errorf. Use %w to format errors --- pkg/cloud/cloud.go | 46 +++++++++++++++++++++++----------------------- pkg/driver/node.go | 8 ++++---- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 869ca13..2eaaf2f 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -185,7 +185,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * } resp, err := c.computing.CreateVolume(ctx, input) if err != nil { - return nil, fmt.Errorf("could not create NIFCLOUD additional storage: %v", err) + return nil, fmt.Errorf("could not create NIFCLOUD additional storage: %w", err) } volumeID := nifcloud.ToString(resp.VolumeId) @@ -204,7 +204,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * } if err := c.waitForVolume(ctx, volumeID, "in-use"); err != nil { - return nil, fmt.Errorf("failed to get an in-use volume: %v", err) + return nil, fmt.Errorf("failed to get an in-use volume: %w", err) } detachVolumeInput := &computing.DetachVolumeInput{ @@ -213,11 +213,11 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions * VolumeId: nifcloud.String(volumeID), } if _, err = c.computing.DetachVolume(ctx, detachVolumeInput); err != nil { - return nil, fmt.Errorf("could not detach additional storage %q from %q: %v", volumeID, instanceID, err) + return nil, fmt.Errorf("could not detach additional storage %q from %q: %w", volumeID, instanceID, err) } if err := c.waitForVolume(ctx, volumeID, "available"); err != nil { - return nil, fmt.Errorf("failed to get an available volume: %v", err) + return nil, fmt.Errorf("failed to get an available volume: %w", err) } return &Disk{ @@ -236,7 +236,7 @@ func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) { if isAWSErrorVolumeNotFound(err) { return false, ErrNotFound } - return false, fmt.Errorf("DeleteDisk could not delete volume: %v", err) + return false, fmt.Errorf("DeleteDisk could not delete volume: %w", err) } return true, nil @@ -252,13 +252,13 @@ func (c *cloud) AttachDisk(ctx context.Context, volumeID, nodeID string) (string if isAWSError(err, "Server.Inoperable.Volume.AlreadyAttached") { deviceName, err := c.getDeviceNameFromVolumeID(ctx, nodeID, volumeID) if err != nil { - return "", fmt.Errorf("could not fetch the device name for already attached volume: %v", err) + return "", fmt.Errorf("could not fetch the device name for already attached volume: %w", err) } return deviceName, nil } - return "", fmt.Errorf("could not attach volume %q to node %q: %v", volumeID, nodeID, err) + return "", fmt.Errorf("could not attach volume %q to node %q: %w", volumeID, nodeID, err) } - klog.V(5).Infof("AttachVolume volume=%q instance=%q request returned %v", volumeID, nodeID, resp) + klog.V(5).Infof("AttachVolume volume=%q instance=%q request returned %w", volumeID, nodeID, resp) // This is the only situation where we taint the device if err := c.WaitForAttachmentState(ctx, volumeID, "attached"); err != nil { @@ -267,7 +267,7 @@ func (c *cloud) AttachDisk(ctx context.Context, volumeID, nodeID string) (string deviceName, err := c.getDeviceNameFromVolumeID(ctx, nodeID, volumeID) if err != nil { - return "", fmt.Errorf("could not fetch the device name after attach volume: %v", err) + return "", fmt.Errorf("could not fetch the device name after attach volume: %w", err) } // TODO: Double check the attachment to be 100% sure we attached the correct volume at the correct mountpoint @@ -287,7 +287,7 @@ func (c *cloud) DetachDisk(ctx context.Context, volumeID, nodeID string) error { if isAWSError(err, "Client.Inoperable.Volume.DetachedFromInstance") { return nil } - return fmt.Errorf("could not detach volume %q from node %q: %v", volumeID, nodeID, err) + return fmt.Errorf("could not detach volume %q from node %q: %w", volumeID, nodeID, err) } if err := c.WaitForAttachmentState(ctx, volumeID, "detached"); err != nil { @@ -323,7 +323,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, size int64) (in VolumeId: nifcloud.String(volumeID), } if _, err := c.computing.ExtendVolumeSize(ctx, input); err != nil { - klog.Errorf("ExtendVolumeSize returns an error: %v", err) + klog.Errorf("ExtendVolumeSize returns an error: %w", err) return 0, err } @@ -336,7 +336,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, size int64) (in volume, err = c.GetDiskByID(ctx, volumeID) if err != nil { - klog.Errorf("could not get the disk info from id: %v", err) + klog.Errorf("could not get the disk info from id: %w", err) return 0, err } klog.V(4).Infof("after extend volume: current=%dGiB, desired=%dGiB", volume.CapacityGiB, desiredSize) @@ -353,7 +353,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, size int64) (in func (c *cloud) ListDisks(ctx context.Context) ([]*Disk, error) { resp, err := c.computing.DescribeVolumes(ctx, nil) if err != nil { - return nil, fmt.Errorf("clould not fetch the additional storages: %v", err) + return nil, fmt.Errorf("clould not fetch the additional storages: %w", err) } disks := []*Disk{} @@ -366,7 +366,7 @@ func (c *cloud) ListDisks(ctx context.Context) ([]*Disk, error) { volSize, err := strconv.Atoi(nifcloud.ToString(volume.Size)) if err != nil { - klog.Warningf("could not convert volume size %q. using 100GiB...: %v", nifcloud.ToString(volume.Size), err) + klog.Warningf("could not convert volume size %q. using 100GiB...: %w", nifcloud.ToString(volume.Size), err) volSize = 100 } @@ -422,7 +422,7 @@ func (c *cloud) WaitForAttachmentState(ctx context.Context, volumeID, state stri func (c *cloud) GetDiskByName(ctx context.Context, name string, capacityBytes int64) (*Disk, error) { resp, err := c.computing.DescribeVolumes(ctx, nil) if err != nil { - return nil, fmt.Errorf("could not list the volumes: %v", err) + return nil, fmt.Errorf("could not list the volumes: %w", err) } var volume *types.VolumeSet @@ -437,7 +437,7 @@ func (c *cloud) GetDiskByName(ctx context.Context, name string, capacityBytes in volSizeGiB, err := strconv.Atoi(nifcloud.ToString(volume.Size)) if err != nil { - return nil, fmt.Errorf("could not convert volume size %q: %v", nifcloud.ToString(volume.Size), err) + return nil, fmt.Errorf("could not convert volume size %q: %w", nifcloud.ToString(volume.Size), err) } if int64(volSizeGiB) != roundUpCapacity(util.BytesToGiB(capacityBytes)) { @@ -467,7 +467,7 @@ func (c *cloud) GetDiskByID(ctx context.Context, volumeID string) (*Disk, error) volSize, err := strconv.Atoi(nifcloud.ToString(volume.Size)) if err != nil { - return nil, fmt.Errorf("could not convert volume size %q: %v", nifcloud.ToString(volume.Size), err) + return nil, fmt.Errorf("could not convert volume size %q: %w", nifcloud.ToString(volume.Size), err) } return &Disk{ @@ -489,7 +489,7 @@ func (c *cloud) IsExistInstance(ctx context.Context, nodeID string) bool { func (c *cloud) GetInstanceByName(ctx context.Context, name string) (*Instance, error) { res, err := c.getInstance(ctx, name) if err != nil { - return nil, fmt.Errorf("could not found instance %q: %v", name, err) + return nil, fmt.Errorf("could not found instance %q: %w", name, err) } return &Instance{ @@ -551,7 +551,7 @@ func (c *cloud) listInstancesByZone(ctx context.Context, zone string) ([]Instanc func (c *cloud) listInstances(ctx context.Context) ([]Instance, error) { resp, err := c.computing.DescribeInstances(ctx, nil) if err != nil { - return nil, fmt.Errorf("error listing NIFCLOUD instances: %v", err) + return nil, fmt.Errorf("error listing NIFCLOUD instances: %w", err) } instances := []Instance{} @@ -572,7 +572,7 @@ func (c *cloud) getInstance(ctx context.Context, nodeID string) (*types.Instance } resp, err := c.computing.DescribeInstances(ctx, input) if err != nil { - return nil, fmt.Errorf("error listing NIFCLOUD instances: %v", err) + return nil, fmt.Errorf("error listing NIFCLOUD instances: %w", err) } instances := []types.InstancesSet{} @@ -591,7 +591,7 @@ func (c *cloud) getInstance(ctx context.Context, nodeID string) (*types.Instance // So call DescribeInstanceAttribute API and set blockDeviceMapping to instance info. instance := &instances[0] if err := c.setBlockDeviceMapping(ctx, instance); err != nil { - return nil, fmt.Errorf("error setting block device mapping: %v", err) + return nil, fmt.Errorf("error setting block device mapping: %w", err) } return instance, nil @@ -604,7 +604,7 @@ func (c *cloud) setBlockDeviceMapping(ctx context.Context, instance *types.Insta } resp, err := c.computing.DescribeInstanceAttribute(ctx, input) if err != nil { - return fmt.Errorf("error getting block device mapping: %v", err) + return fmt.Errorf("error getting block device mapping: %w", err) } blockDeviceInfo := resp.BlockDeviceMapping @@ -651,7 +651,7 @@ func (c *cloud) getDeviceNameFromVolumeID(ctx context.Context, instanceID, volum } resp, err := c.computing.DescribeInstanceAttribute(ctx, input) if err != nil { - return "", fmt.Errorf("error getting block device mapping: %v", err) + return "", fmt.Errorf("error getting block device mapping: %w", err) } for _, blockDevice := range resp.BlockDeviceMapping { diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 14ee61b..b4758f3 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -525,7 +525,7 @@ func (n *nodeService) getBlockSizeBytes(devicePath string) (int64, error) { cmd := n.mounter.(*NodeMounter).Exec.Command("blockdev", "--getsize64", devicePath) output, err := cmd.Output() if err != nil { - return -1, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %v", devicePath, string(output), err) + return -1, fmt.Errorf("error when getting size of block volume at path %s: output: %s, err: %w", devicePath, string(output), err) } strOut := strings.TrimSpace(string(output)) @@ -551,7 +551,7 @@ func (n *nodeService) findDevicePath(scsiID string) (string, error) { deviceFileDir := "/dev/disk/by-path" files, err := ioutil.ReadDir(deviceFileDir) if err != nil { - return "", fmt.Errorf("could not list the files in /dev/disk/by-path/: %v", err) + return "", fmt.Errorf("could not list the files in /dev/disk/by-path/: %w", err) } devicePath := "" @@ -560,7 +560,7 @@ func (n *nodeService) findDevicePath(scsiID string) (string, error) { if deviceFileRegexp.MatchString(f.Name()) { devicePath, err = filepath.EvalSymlinks(filepath.Join(deviceFileDir, f.Name())) if err != nil { - return "", fmt.Errorf("could not eval symlynk for %q: %v", f.Name(), err) + return "", fmt.Errorf("could not eval symlynk for %q: %w", f.Name(), err) } } } @@ -613,7 +613,7 @@ func (n *nodeService) scanStorageDevices() error { return nil }) if err != nil { - return fmt.Errorf("failed to scan devices: %v", err) + return fmt.Errorf("failed to scan devices: %w", err) } } From f403929ca6145e779cef47e2a2fe7ad927db566d Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 15:07:55 +0900 Subject: [PATCH 04/14] Fix errorlint comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error --- pkg/driver/controller.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 8335a3c..5794678 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -2,6 +2,7 @@ package driver import ( "context" + "errors" "strings" csi "github.com/container-storage-interface/spec/lib/go/csi" @@ -76,11 +77,11 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol disk, err := d.cloud.GetDiskByName(ctx, volName, volSizeBytes) if err != nil { - switch err { - case cloud.ErrNotFound: - case cloud.ErrMultiDisks: + switch { + case errors.Is(err, cloud.ErrNotFound): + case errors.Is(err, cloud.ErrMultiDisks): return nil, status.Error(codes.Internal, err.Error()) - case cloud.ErrDiskExistsDiffSize: + case errors.Is(err, cloud.ErrDiskExistsDiffSize): return nil, status.Error(codes.AlreadyExists, err.Error()) default: return nil, status.Error(codes.Internal, err.Error()) @@ -125,7 +126,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol disk, err = d.cloud.CreateDisk(ctx, volName, opts) if err != nil { errCode := codes.Internal - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { errCode = codes.NotFound } return nil, status.Errorf(errCode, "Could not create volume %q: %v", volName, err) @@ -147,7 +148,7 @@ func (d *controllerService) DeleteVolume(ctx context.Context, req *csi.DeleteVol defer d.inFlight.Delete(volumeID) if _, err := d.cloud.DeleteDisk(ctx, volumeID); err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { klog.V(4).Info("DeleteVolume: volume not found, returning with success") return &csi.DeleteVolumeResponse{}, nil } @@ -184,7 +185,7 @@ func (d *controllerService) ControllerPublishVolume(ctx context.Context, req *cs } if _, err := d.cloud.GetDiskByID(ctx, volumeID); err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Error(codes.NotFound, "Volume not found") } return nil, status.Errorf(codes.Internal, "Could not get volume with ID %q: %v", volumeID, err) @@ -285,7 +286,7 @@ func (d *controllerService) ControllerGetVolume(ctx context.Context, req *csi.Co disk, err := d.cloud.GetDiskByID(ctx, volumeID) if err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Error(codes.NotFound, "Volume not found") } return nil, status.Errorf(codes.Internal, "Could not get volume with ID %q: %v", volumeID, err) @@ -315,7 +316,7 @@ func (d *controllerService) ValidateVolumeCapabilities(ctx context.Context, req } if _, err := d.cloud.GetDiskByID(ctx, volumeID); err != nil { - if err == cloud.ErrNotFound { + if errors.Is(err, cloud.ErrNotFound) { return nil, status.Error(codes.NotFound, "Volume not found") } return nil, status.Errorf(codes.Internal, "Could not get volume with ID %q: %v", volumeID, err) From 5f386bb67f0b8b69d46b8dba95c58e15fbb06c4c Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 15:47:05 +0900 Subject: [PATCH 05/14] Fix staticcheck SA1019: rand.Seed has been deprecated since Go 1.20 --- cmd/main.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 3136b5f..f61cc1b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -3,18 +3,12 @@ package main import ( "flag" "fmt" - "math/rand" "os" - "time" "github.com/nifcloud/nifcloud-additional-storage-csi-driver/pkg/driver" "k8s.io/klog/v2" ) -func init() { - rand.Seed(time.Now().Unix()) -} - func main() { var ( version bool From a3fc65dec81fb9c40a849cf5185b7096615780a0 Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 15:58:15 +0900 Subject: [PATCH 06/14] Fix staticcheck SA1019: wait.Poll is deprecated --- pkg/cloud/cloud.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 2eaaf2f..1896ed0 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -511,16 +511,17 @@ func (c *cloud) waitForVolume(ctx context.Context, volumeID, status string) erro VolumeId: []string{volumeID}, } - err := wait.Poll(checkInterval, checkTimeout, func() (done bool, err error) { - vol, err := c.getVolume(ctx, input) - if err != nil { - return true, err - } - if vol.Status != nil { - return *vol.Status == status, nil - } - return false, nil - }) + err := wait.PollUntilContextTimeout(context.Background(), checkInterval, checkTimeout, false, + func(context.Context) (done bool, err error) { + vol, err := c.getVolume(ctx, input) + if err != nil { + return true, err + } + if vol.Status != nil { + return *vol.Status == status, nil + } + return false, nil + }) return err } From f8179f900702edcaf0c88a22ee5f9a8e17ac14ff Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 16:05:42 +0900 Subject: [PATCH 07/14] Fix prealloc Consider pre-allocating --- pkg/driver/controller.go | 2 +- pkg/driver/node.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 5794678..ab2d983 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -233,7 +233,7 @@ func (d *controllerService) ControllerUnpublishVolume(ctx context.Context, req * func (d *controllerService) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { klog.V(4).Infof("ControllerGetCapabilities: called with args %+v", *req) - var caps []*csi.ControllerServiceCapability + caps := make([]*csi.ControllerServiceCapability, len(controllerCaps)) for _, cap := range controllerCaps { c := &csi.ControllerServiceCapability{ Type: &csi.ControllerServiceCapability_Rpc{ diff --git a/pkg/driver/node.go b/pkg/driver/node.go index b4758f3..fc3db78 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -395,7 +395,7 @@ func (n *nodeService) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVo func (n *nodeService) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) { klog.V(4).Infof("NodeGetCapabilities: called with args %+v", *req) - var caps []*csi.NodeServiceCapability + caps := make([]*csi.NodeServiceCapability, len(nodeCaps)) for _, cap := range nodeCaps { c := &csi.NodeServiceCapability{ Type: &csi.NodeServiceCapability_Rpc{ From 6fc5b468611296628f14d3544cadc1b5701c8085 Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 16:09:26 +0900 Subject: [PATCH 08/14] Fux staticcheck SA1019: io/ioutil has been deprecated since Go 1.19 --- pkg/driver/node.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index fc3db78..0534663 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -3,7 +3,6 @@ package driver import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "regexp" @@ -549,7 +548,7 @@ func (n *nodeService) findDevicePath(scsiID string) (string, error) { deviceNumber := string(match[1]) deviceFileDir := "/dev/disk/by-path" - files, err := ioutil.ReadDir(deviceFileDir) + files, err := os.ReadDir(deviceFileDir) if err != nil { return "", fmt.Errorf("could not list the files in /dev/disk/by-path/: %w", err) } From 4f1662a1fcae105b38dc247391a8dd97ee90dd1a Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 16:09:53 +0900 Subject: [PATCH 09/14] Add golangci-lint --- .golangci.yml | 33 +++++++++++++++++++++++++++++++++ Makefile | 3 +++ 2 files changed, 36 insertions(+) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..7948956 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,33 @@ +run: + tests: true + timeout: 10m + +linters: + disable-all: true + enable: + - errcheck + - errorlint + - goimports + - revive + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - lll + - misspell + - nolintlint + - prealloc + - staticcheck + - typecheck + - unparam + - unused + +linters-settings: + revive: + rules: + - name: package-comments + disabled: true + lll: + line-length: 200 diff --git a/Makefile b/Makefile index 75f1f25..70a4a65 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,9 @@ GIT_COMMIT?=$(shell git rev-parse HEAD) BUILD_DATE?=$(shell date -u +"%Y-%m-%dT%H:%M:%SZ") LDFLAGS?="-X ${PKG}/pkg/driver.driverVersion=${VERSION} -X ${PKG}/pkg/driver.gitCommit=${GIT_COMMIT} -X ${PKG}/pkg/driver.buildDate=${BUILD_DATE} -s -w" +lint: + golangci-lint run + build: mkdir -p bin CGO_ENABLED=0 GOOS=linux go build -ldflags ${LDFLAGS} -o bin/nifcloud-additional-storage-csi-driver ./cmd/ From 833b06c3193fff15f3ac26db36c27e07ffe5dcde Mon Sep 17 00:00:00 2001 From: heriet Date: Mon, 4 Dec 2023 16:11:46 +0900 Subject: [PATCH 10/14] Add golangci-lint action --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c126e42..da16747 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,11 @@ jobs: - name: Diff mod run: go mod tidy && git diff --exit-code go.mod go.sum + - name: Lint + uses: golangci/golangci-lint-action@v3 + with: + version: latest + - name: Build run: make build From 38a28b5a917d2c0117f4ef32d7c9794ae54b4fa9 Mon Sep 17 00:00:00 2001 From: heriet Date: Thu, 7 Dec 2023 15:55:56 +0900 Subject: [PATCH 11/14] Fix context.Background() -> ctx Co-authored-by: Kouta Asai <8992675+aokumasan@users.noreply.github.com> --- pkg/cloud/cloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 1896ed0..52568ff 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -511,7 +511,7 @@ func (c *cloud) waitForVolume(ctx context.Context, volumeID, status string) erro VolumeId: []string{volumeID}, } - err := wait.PollUntilContextTimeout(context.Background(), checkInterval, checkTimeout, false, + err := wait.PollUntilContextTimeout(ctx, checkInterval, checkTimeout, false, func(context.Context) (done bool, err error) { vol, err := c.getVolume(ctx, input) if err != nil { From cab7cd728dbcc265bd39fde3072c0b769aa578a4 Mon Sep 17 00:00:00 2001 From: heriet Date: Thu, 7 Dec 2023 16:06:58 +0900 Subject: [PATCH 12/14] Fix context parameter Co-authored-by: Kouta Asai <8992675+aokumasan@users.noreply.github.com> --- pkg/cloud/cloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 52568ff..8ed5287 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -512,7 +512,7 @@ func (c *cloud) waitForVolume(ctx context.Context, volumeID, status string) erro } err := wait.PollUntilContextTimeout(ctx, checkInterval, checkTimeout, false, - func(context.Context) (done bool, err error) { + func(ctx context.Context) (done bool, err error) { vol, err := c.getVolume(ctx, input) if err != nil { return true, err From ebda43cdcfe84e0382383fd4a57c6035ebf4f10f Mon Sep 17 00:00:00 2001 From: heriet Date: Thu, 7 Dec 2023 16:27:27 +0900 Subject: [PATCH 13/14] Fix format specifiers --- pkg/cloud/cloud.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 8ed5287..d43a55a 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -258,7 +258,7 @@ func (c *cloud) AttachDisk(ctx context.Context, volumeID, nodeID string) (string } return "", fmt.Errorf("could not attach volume %q to node %q: %w", volumeID, nodeID, err) } - klog.V(5).Infof("AttachVolume volume=%q instance=%q request returned %w", volumeID, nodeID, resp) + klog.V(5).Infof("AttachVolume volume=%q instance=%q request returned %v", volumeID, nodeID, resp) // This is the only situation where we taint the device if err := c.WaitForAttachmentState(ctx, volumeID, "attached"); err != nil { From 4dbd4e352c7c84c7fd84493bb88e43ea8f6a264d Mon Sep 17 00:00:00 2001 From: heriet Date: Thu, 7 Dec 2023 17:27:54 +0900 Subject: [PATCH 14/14] Fix format specifier. %w -> %v without fmt.Errorf --- pkg/cloud/cloud.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index d43a55a..f54580a 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -323,7 +323,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, size int64) (in VolumeId: nifcloud.String(volumeID), } if _, err := c.computing.ExtendVolumeSize(ctx, input); err != nil { - klog.Errorf("ExtendVolumeSize returns an error: %w", err) + klog.Errorf("ExtendVolumeSize returns an error: %v", err) return 0, err } @@ -336,7 +336,7 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, size int64) (in volume, err = c.GetDiskByID(ctx, volumeID) if err != nil { - klog.Errorf("could not get the disk info from id: %w", err) + klog.Errorf("could not get the disk info from id: %v", err) return 0, err } klog.V(4).Infof("after extend volume: current=%dGiB, desired=%dGiB", volume.CapacityGiB, desiredSize) @@ -366,7 +366,7 @@ func (c *cloud) ListDisks(ctx context.Context) ([]*Disk, error) { volSize, err := strconv.Atoi(nifcloud.ToString(volume.Size)) if err != nil { - klog.Warningf("could not convert volume size %q. using 100GiB...: %w", nifcloud.ToString(volume.Size), err) + klog.Warningf("could not convert volume size %q. using 100GiB...: %v", nifcloud.ToString(volume.Size), err) volSize = 100 }