diff --git a/CHANGELOG.md b/CHANGELOG.md index cd3329d05..1e631fdf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,9 @@ ## unreleased +* Validate CreateVolume limits with forbidden error response + [[GH-481]](https://github.com/digitalocean/csi-digitalocean/pull/481) * CSI driver can accidentally format existing volume - [[GH-469]](https://github.com/digitalocean/csi-digitalocean/pull/478) + [[GH-478]](https://github.com/digitalocean/csi-digitalocean/pull/478) ## v4.5.0 - 2023.01.11 * Update CSI driver for Kubernetes 1.26 diff --git a/driver/controller.go b/driver/controller.go index 35997eada..13244d8fa 100644 --- a/driver/controller.go +++ b/driver/controller.go @@ -181,20 +181,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) volumeReq.SnapshotID = snapshotID } - log.Info("checking volume limit") - details, err := d.checkLimit(ctx) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to check volume limit: %s", err) - } - if details != nil { - return nil, status.Errorf(codes.ResourceExhausted, - "volume limit (%d) has been reached. Current number of volumes: %d. Please contact support.", - details.limit, details.numVolumes) - } - log.WithField("volume_req", volumeReq).Info("creating volume") - vol, _, err := d.storage.CreateVolume(ctx, volumeReq) + vol, cvResp, err := d.storage.CreateVolume(ctx, volumeReq) if err != nil { + if cvResp != nil && cvResp.StatusCode == http.StatusForbidden && strings.Contains(err.Error(), "capacity limit exceeded") { + return nil, status.Errorf(codes.ResourceExhausted, "volume limit has been reached. Please contact support") + } + return nil, status.Error(codes.Internal, err.Error()) } diff --git a/driver/controller_test.go b/driver/controller_test.go index b1b464df4..116f1101e 100644 --- a/driver/controller_test.go +++ b/driver/controller_test.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strconv" "strings" "testing" @@ -284,10 +285,16 @@ func TestControllerExpandVolume(t *testing.T) { } func TestCreateVolume(t *testing.T) { + snapshotId := "snapshotId" + tests := []struct { - name string - listVolumesErr error - getSnapshotErr error + name string + listVolumesErr error + getSnapshotErr error + snapshots map[string]*godo.Snapshot + wantErr error + createVolumeErr error + createVolumeResponseErr *godo.Response }{ { name: "listing volumes failing", @@ -297,24 +304,75 @@ func TestCreateVolume(t *testing.T) { name: "fetching snapshot failing", getSnapshotErr: errors.New("failed to get snapshot"), }, + { + name: "volume limit has been reached", + snapshots: map[string]*godo.Snapshot{ + snapshotId: { + ID: snapshotId, + }, + }, + createVolumeErr: &godo.ErrorResponse{ + Response: &http.Response{ + Request: &http.Request{ + Method: http.MethodPost, + URL: &url.URL{}, + }, + StatusCode: http.StatusForbidden, + }, + Message: "failed to create volume: volume/snapshot capacity limit exceeded", + }, + createVolumeResponseErr: &godo.Response{ + Response: &http.Response{ + StatusCode: http.StatusForbidden, + }, + }, + wantErr: errors.New("volume limit has been reached. Please contact support"), + }, + { + name: "error occurred when creating a volume", + snapshots: map[string]*godo.Snapshot{ + snapshotId: { + ID: snapshotId, + }, + }, + createVolumeErr: &godo.ErrorResponse{ + Response: &http.Response{ + Request: &http.Request{ + Method: http.MethodPost, + URL: &url.URL{}, + }, + StatusCode: http.StatusInternalServerError, + }, + Message: "internal server error", + }, + createVolumeResponseErr: &godo.Response{ + Response: &http.Response{ + StatusCode: http.StatusInternalServerError, + }, + }, + wantErr: errors.New("internal server error"), + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { d := &Driver{ storage: &fakeStorageDriver{ - listVolumesErr: test.listVolumesErr, + listVolumesErr: test.listVolumesErr, + createVolumeErr: test.createVolumeErr, + createVolumeErrResponse: test.createVolumeResponseErr, }, snapshots: &fakeSnapshotsDriver{ getSnapshotErr: test.getSnapshotErr, + snapshots: test.snapshots, }, - log: logrus.New().WithField("test_enabed", true), + log: logrus.New().WithField("test_enabled", true), } _, err := d.CreateVolume(context.Background(), &csi.CreateVolumeRequest{ Name: "name", VolumeCapabilities: []*csi.VolumeCapability{ - &csi.VolumeCapability{ + { AccessType: &csi.VolumeCapability_Mount{ Mount: &csi.VolumeCapability_MountVolume{}, }, @@ -338,6 +396,8 @@ func TestCreateVolume(t *testing.T) { wantErr = test.listVolumesErr case test.getSnapshotErr != nil: wantErr = test.getSnapshotErr + case test.createVolumeErr != nil: + wantErr = test.wantErr } if wantErr == nil && err != nil { diff --git a/driver/driver_test.go b/driver/driver_test.go index a48ce6eb7..a7666eacd 100644 --- a/driver/driver_test.go +++ b/driver/driver_test.go @@ -155,9 +155,11 @@ func (f *fakeAccountDriver) Get(context.Context) (*godo.Account, *godo.Response, } type fakeStorageDriver struct { - volumes map[string]*godo.Volume - snapshots map[string]*godo.Snapshot - listVolumesErr error + volumes map[string]*godo.Volume + snapshots map[string]*godo.Snapshot + listVolumesErr error + createVolumeErr error + createVolumeErrResponse *godo.Response } func (f *fakeStorageDriver) ListVolumes(ctx context.Context, param *godo.ListVolumeParams) ([]godo.Volume, *godo.Response, error) { @@ -213,6 +215,10 @@ func (f *fakeStorageDriver) GetVolume(ctx context.Context, id string) (*godo.Vol } func (f *fakeStorageDriver) CreateVolume(ctx context.Context, req *godo.VolumeCreateRequest) (*godo.Volume, *godo.Response, error) { + if f.createVolumeErr != nil || f.createVolumeErrResponse != nil { + return nil, f.createVolumeErrResponse, f.createVolumeErr + } + id := randString(10) vol := &godo.Volume{ ID: id,