Skip to content

Commit

Permalink
bug/snapshots - v0.5.3 show snapshots working with all storage api ve…
Browse files Browse the repository at this point in the history 
…rsions, plus snapshot prefix

Signed-off-by: Joe Skazinski <[email protected]>
  • Loading branch information
@jskazinski
jskazinski committed Sep 4, 2021
1 parent e9de7ce commit 284872b
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 41 deletions.
37 changes: 18 additions & 19 deletions pkg/common/system.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,12 @@ import (
"strings"
"unicode"

"github.com/container-storage-interface/spec/lib/go/csi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/klog"
)

// ValidateVolumeName verifies that the string only includes spaces and printable UTF-8 characters except: " , < \
func ValidateVolumeName(s string) bool {
klog.V(2).Infof("ValidateVolumeName %q", s)
// ValidateName verifies that the string only includes spaces and printable UTF-8 characters except: " , < \
func ValidateName(s string) bool {
klog.V(2).Infof("ValidateName %q", s)
for i := 0; i < len(s); i++ {
if s[i] == '"' || s[i] == ',' || s[i] == '<' || s[i] == '\\' {
return false
Expand All @@ -41,33 +38,35 @@ func ValidateVolumeName(s string) bool {
return true
}

// TranslateVolumeName converts the passed in volume name to the translated volume name
func TranslateVolumeName(req *csi.CreateVolumeRequest) (string, error) {
// TranslateName converts the passed in volume name to the translated volume name
func TranslateName(name, prefix string) (string, error) {

if req.GetName() == "" {
return "", status.Error(codes.InvalidArgument, "cannot create volume with an empty name")
}

originalVolumeID := req.GetName()
volumeID := originalVolumeID
parameters := req.GetParameters()
prefix := parameters[VolumePrefixKey]
volumeID := name

if len(prefix) == 0 {
// If string is greater than max, truncate it, otherwise return original string
if len(volumeID) > VolumeNameMaxLength {
// Skip over 'pvc-'
if volumeID[0:4] == "pvc-" {
if len(volumeID) >= 4 && volumeID[0:4] == "pvc-" {
volumeID = volumeID[4:]
}
// Skip over 'snapshot-'
if len(volumeID) >= 9 && volumeID[0:9] == "snapshot-" {
volumeID = volumeID[9:]
}
volumeID = strings.ReplaceAll(volumeID, "-", "")
volumeID = volumeID[:VolumeNameMaxLength]
}
} else {
// Skip over 'pvc-' and remove all dashes
uuid := volumeID
if volumeID[0:4] == "pvc-" {
if len(volumeID) >= 4 && volumeID[0:4] == "pvc-" {
uuid = volumeID[4:]
klog.Infof("TranslateName(pvc): uuid=%q", uuid)
}
if len(volumeID) >= 9 && volumeID[0:9] == "snapshot-" {
uuid = volumeID[9:]
klog.Infof("TranslateName(snapshot): uuid=%q", uuid)
}
uuid = strings.ReplaceAll(uuid, "-", "")

Expand All @@ -85,7 +84,7 @@ func TranslateVolumeName(req *csi.CreateVolumeRequest) (string, error) {
}
}

klog.Infof("TranslateVolumeName %q[%d], prefix %q[%d], result %q[%d]", originalVolumeID, len(originalVolumeID), prefix, len(prefix), volumeID, len(volumeID))
klog.Infof("TranslateName %q[%d], prefix %q[%d], result %q[%d]", name, len(name), prefix, len(prefix), volumeID, len(volumeID))

return volumeID, nil
}
29 changes: 12 additions & 17 deletions pkg/common/system_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ func createRequestVolume(name string, prefix string) (csi.CreateVolumeRequest, e
}

func runTest(t *testing.T, idin string, idout string, prefix string) {
req, _ := createRequestVolume(idin, prefix)
id, err := TranslateVolumeName(&req)

id, err := TranslateName(idin, prefix)
g := NewWithT(t)
g.Expect(err).To(BeNil())
g.Expect(id).To(Equal(idout))
Expand All @@ -56,13 +54,7 @@ func runTest(t *testing.T, idin string, idout string, prefix string) {
func TestTranslate(t *testing.T) {

// Test empty name
req := csi.CreateVolumeRequest{
Name: "",
Parameters: map[string]string{"volPrefix": "csi"},
}
_, err := TranslateVolumeName(&req)
g := NewWithT(t)
g.Expect(err).ToNot(BeNil())
runTest(t, "", "csi_", "csi")

// Test with no prefix
runTest(t, "pvc-03c551d9-7e77-43ff-993e-c2308d2f09a1", "03c551d97e7743ff993ec2308d2f09a1", "")
Expand All @@ -78,17 +70,20 @@ func TestTranslate(t *testing.T) {
runTest(t, "51d97e7743ff993ec2308d2f09a1", "csi_51d97e7743ff993ec2308d2f09a1", "csi_123")
runTest(t, "pvc-51d9-7e77-43ff-993e-c2308d2f09a1", "cd_51d97e7743ff993ec2308d2f09a1", "cd")
runTest(t, "pvc-51d9-7e77-43ff-993e-c2308d2f09a1", "c_51d97e7743ff993ec2308d2f09a1", "c")

// Test with prefix
runTest(t, "snapshot-03c551d9-7e77-43ff-993e-c2308d2f09a1", "csi_51d97e7743ff993ec2308d2f09a1", "csi")
}

func TestValidate(t *testing.T) {
g := NewWithT(t)
g.Expect(ValidateVolumeName("abcdefghijklmnopqrstuvwxyz")).To(BeTrue())
g.Expect(ValidateVolumeName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")).To(BeTrue())
g.Expect(ValidateVolumeName("a b _ . - c")).To(BeTrue())
g.Expect(ValidateName("abcdefghijklmnopqrstuvwxyz")).To(BeTrue())
g.Expect(ValidateName("ABCDEFGHIJKLMNOPQRSTUVWXYZ")).To(BeTrue())
g.Expect(ValidateName("a b _ . - c")).To(BeTrue())

// Test unaccepable characters: " , < \
g.Expect(ValidateVolumeName("\"abc")).To(BeFalse())
g.Expect(ValidateVolumeName("abc,")).To(BeFalse())
g.Expect(ValidateVolumeName("abc<def")).To(BeFalse())
g.Expect(ValidateVolumeName("abc\\def")).To(BeFalse())
g.Expect(ValidateName("\"abc")).To(BeFalse())
g.Expect(ValidateName("abc,")).To(BeFalse())
g.Expect(ValidateName("abc<def")).To(BeFalse())
g.Expect(ValidateName("abc\\def")).To(BeFalse())
}
6 changes: 3 additions & 3 deletions pkg/controller/provisioner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ import (
// CreateVolume creates a new volume from the given request. The function is idempotent.
func (controller *Controller) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) {

volumeID, err := common.TranslateVolumeName(req)
parameters := req.GetParameters()
volumeID, err := common.TranslateName(req.GetName(), parameters[common.VolumePrefixKey])
if err != nil {
return nil, err
}

if common.ValidateVolumeName(volumeID) == false {
if common.ValidateName(volumeID) == false {
return nil, status.Error(codes.InvalidArgument, "volume name contains invalid characters")
}

size := req.GetCapacityRange().GetRequiredBytes()
sizeStr := getSizeStr(size)
parameters := req.GetParameters()
pool := parameters[common.PoolConfigKey]
poolType, _ := controller.client.Info.GetPoolType(pool)

Expand Down
13 changes: 11 additions & 2 deletions pkg/controller/snapshotter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"errors"
"fmt"
"strconv"
"strings"
"time"

storageapi "github.com/Seagate/seagate-exos-x-api-go"
"github.com/Seagate/seagate-exos-x-csi/pkg/common"
"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/timestamp"
Expand All @@ -19,7 +19,16 @@ import (

// CreateSnapshot creates a snapshot of the given volume
func (controller *Controller) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) {
name := strings.Replace(req.Name[9:], "-", "", -1)

parameters := req.GetParameters()
name, err := common.TranslateName(req.GetName(), parameters[common.VolumePrefixKey])
if err != nil {
return nil, err
}

if common.ValidateName(name) == false {
return nil, status.Error(codes.InvalidArgument, "snapshot name contains invalid characters")
}

_, respStatus, err := controller.client.CreateSnapshot(req.SourceVolumeId, name)
if err != nil && respStatus.ReturnCode != snapshotAlreadyExists {
Expand Down

0 comments on commit 284872b

Please sign in to comment.