From 37bbb4877359a55b7ceffdd2331e8cc307b99563 Mon Sep 17 00:00:00 2001 From: riya-singhal31 Date: Wed, 24 May 2023 16:22:52 +0530 Subject: [PATCH 1/4] rbd: add check for EncryptionTypeNone this commit adds the validation for encryption value as false, and sets the type as none Signed-off-by: riya-singhal31 --- internal/rbd/encryption.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go index ea65f14aa87..aadf3f9babd 100644 --- a/internal/rbd/encryption.go +++ b/internal/rbd/encryption.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "strings" + "strconv" kmsapi "github.com/ceph/ceph-csi/internal/kms" "github.com/ceph/ceph-csi/internal/util" @@ -341,7 +342,8 @@ func ParseEncryptionOpts( encrypted, kmsID string ) encrypted, ok = volOptions["encrypted"] - if !ok { + val, _ := strconv.ParseBool(encrypted) + if !ok || !val{ return "", util.EncryptionTypeNone, nil } kmsID, err = util.FetchEncryptionKMSID(encrypted, volOptions["encryptionKMSID"]) From 7d70f3a5a42b53adc9b372296418b2fdad3ad32c Mon Sep 17 00:00:00 2001 From: riya-singhal31 Date: Wed, 24 May 2023 19:13:15 +0530 Subject: [PATCH 2/4] rbd: remove context where its not being used Signed-off-by: riya-singhal31 --- internal/rbd/controllerserver.go | 2 +- internal/rbd/encryption.go | 7 +++---- internal/rbd/nodeserver.go | 2 +- internal/rbd/rbd_journal.go | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 56061b2273b..bd00d148f6b 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -191,7 +191,7 @@ func (cs *ControllerServer) parseVolCreateRequest( // get the owner of the PVC which is required for few encryption related operations rbdVol.Owner = k8s.GetOwner(req.GetParameters()) - err = rbdVol.initKMS(ctx, req.GetParameters(), req.GetSecrets()) + err = rbdVol.initKMS(req.GetParameters(), req.GetSecrets()) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go index aadf3f9babd..bd5ced9988c 100644 --- a/internal/rbd/encryption.go +++ b/internal/rbd/encryption.go @@ -110,7 +110,7 @@ func (ri *rbdImage) isFileEncrypted() bool { } func IsFileEncrypted(ctx context.Context, volOptions map[string]string) (bool, error) { - _, encType, err := ParseEncryptionOpts(ctx, volOptions, util.EncryptionTypeInvalid) + _, encType, err := ParseEncryptionOpts(volOptions, util.EncryptionTypeInvalid) if err != nil { return false, err } @@ -306,8 +306,8 @@ func (rv *rbdVolume) openEncryptedDevice(ctx context.Context, devicePath string) return mapperFilePath, nil } -func (ri *rbdImage) initKMS(ctx context.Context, volOptions, credentials map[string]string) error { - kmsID, encType, err := ParseEncryptionOpts(ctx, volOptions, rbdDefaultEncryptionType) +func (ri *rbdImage) initKMS(volOptions, credentials map[string]string) error { + kmsID, encType, err := ParseEncryptionOpts(volOptions, rbdDefaultEncryptionType) if err != nil { return err } @@ -332,7 +332,6 @@ func (ri *rbdImage) initKMS(ctx context.Context, volOptions, credentials map[str // ParseEncryptionOpts returns kmsID and sets Owner attribute. func ParseEncryptionOpts( - ctx context.Context, volOptions map[string]string, fallbackEncType util.EncryptionType, ) (string, util.EncryptionType, error) { diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index 22efcf90162..825f9bc61a7 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -234,7 +234,7 @@ func (ns *NodeServer) populateRbdVol( return nil, status.Error(codes.Internal, err.Error()) } - err = rv.initKMS(ctx, req.GetVolumeContext(), req.GetSecrets()) + err = rv.initKMS(req.GetVolumeContext(), req.GetSecrets()) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 49bb241b312..55c6a033ebf 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -573,7 +573,7 @@ func RegenerateJournal( rbdVol.Owner = owner - kmsID, encryptionType, err = ParseEncryptionOpts(ctx, volumeAttributes, util.EncryptionTypeNone) + kmsID, encryptionType, err = ParseEncryptionOpts(volumeAttributes, util.EncryptionTypeNone) if err != nil { return "", err } From 374043861c96cd5058bbc76193e81bc80d90dd68 Mon Sep 17 00:00:00 2001 From: riya-singhal31 Date: Thu, 25 May 2023 16:06:01 +0530 Subject: [PATCH 3/4] rbd: add e2e for encryption as false Signed-off-by: riya-singhal31 --- e2e/rbd.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/e2e/rbd.go b/e2e/rbd.go index 37b592942c1..057ccf9f4c8 100644 --- a/e2e/rbd.go +++ b/e2e/rbd.go @@ -2872,6 +2872,56 @@ var _ = Describe("RBD", func() { } }) + By("create storageClass with encrypted as false", func() { + err := deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + framework.Failf("failed to delete storageclass: %v", err) + } + err = createRBDStorageClass( + f.ClientSet, + f, + defaultSCName, + nil, + map[string]string{"encrypted": "false"}, + deletePolicy) + if err != nil { + framework.Failf("failed to create storageclass: %v", err) + } + // set up PVC + pvc, err := loadPVC(pvcPath) + if err != nil { + framework.Failf("failed to load PVC: %v", err) + } + pvc.Namespace = f.UniqueName + err = createPVCAndvalidatePV(f.ClientSet, pvc, deployTimeout) + if err != nil { + framework.Failf("failed to create PVC: %v", err) + } + + // validate created backend rbd images + validateRBDImageCount(f, 1, defaultRBDPool) + validateOmapCount(f, 1, rbdType, defaultRBDPool, volumesType) + + // clean up after ourselves + err = deletePVCAndValidatePV(f.ClientSet, pvc, deployTimeout) + if err != nil { + framework.Failf("failed to delete PVC: %v", err) + } + // validate created backend rbd images + validateRBDImageCount(f, 0, defaultRBDPool) + validateOmapCount(f, 0, rbdType, defaultRBDPool, volumesType) + + err = deleteResource(rbdExamplePath + "storageclass.yaml") + if err != nil { + framework.Failf("failed to delete storageclass: %v", err) + } + err = createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, nil, deletePolicy) + if err != nil { + framework.Failf("failed to create storageclass: %v", err) + } + }) + + By("validate RBD static FileSystem PVC", func() { err := validateRBDStaticPV(f, appPath, false, false) if err != nil { From f275ce143bab837143f1af762691f187fb0b6a1a Mon Sep 17 00:00:00 2001 From: riya-singhal31 Date: Thu, 25 May 2023 18:01:30 +0530 Subject: [PATCH 4/4] rbd: add unit test for ParseEncryptionOpts Signed-off-by: riya-singhal31 --- e2e/rbd.go | 1 - internal/rbd/encryption.go | 12 +++- internal/rbd/encryption_test.go | 99 +++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 internal/rbd/encryption_test.go diff --git a/e2e/rbd.go b/e2e/rbd.go index 057ccf9f4c8..76928341925 100644 --- a/e2e/rbd.go +++ b/e2e/rbd.go @@ -2921,7 +2921,6 @@ var _ = Describe("RBD", func() { } }) - By("validate RBD static FileSystem PVC", func() { err := validateRBDStaticPV(f, appPath, false, false) if err != nil { diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go index bd5ced9988c..3d68e80c898 100644 --- a/internal/rbd/encryption.go +++ b/internal/rbd/encryption.go @@ -20,8 +20,8 @@ import ( "context" "errors" "fmt" - "strings" "strconv" + "strings" kmsapi "github.com/ceph/ceph-csi/internal/kms" "github.com/ceph/ceph-csi/internal/util" @@ -341,8 +341,14 @@ func ParseEncryptionOpts( encrypted, kmsID string ) encrypted, ok = volOptions["encrypted"] - val, _ := strconv.ParseBool(encrypted) - if !ok || !val{ + if !ok { + return "", util.EncryptionTypeNone, nil + } + ok, err = strconv.ParseBool(encrypted) + if err != nil { + return "", util.EncryptionTypeInvalid, err + } + if !ok { return "", util.EncryptionTypeNone, nil } kmsID, err = util.FetchEncryptionKMSID(encrypted, volOptions["encryptionKMSID"]) diff --git a/internal/rbd/encryption_test.go b/internal/rbd/encryption_test.go new file mode 100644 index 00000000000..fb043050ad2 --- /dev/null +++ b/internal/rbd/encryption_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2023 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbd + +import ( + "testing" + + "github.com/ceph/ceph-csi/internal/util" +) + +func TestParseEncryptionOpts(t *testing.T) { + t.Parallel() + tests := []struct { + testName string + volOptions map[string]string + fallbackType util.EncryptionType + expectedKMS string + expectedEnc util.EncryptionType + expectedErr bool + }{ + { + testName: "No Encryption Option", + volOptions: map[string]string{ + "foo": "bar", + }, + fallbackType: util.EncryptionTypeBlock, + expectedKMS: "", + expectedEnc: util.EncryptionTypeNone, + expectedErr: false, + }, + { + testName: "Encrypted as false", + volOptions: map[string]string{ + "encrypted": "false", + }, + fallbackType: util.EncryptionTypeBlock, + expectedKMS: "", + expectedEnc: util.EncryptionTypeNone, + expectedErr: false, + }, + { + testName: "Encrypted as invalid string", + volOptions: map[string]string{ + "encrypted": "notbool", + }, + fallbackType: util.EncryptionTypeBlock, + expectedKMS: "", + expectedEnc: util.EncryptionTypeInvalid, + expectedErr: true, + }, + { + testName: "Valid Encryption Option With KMS ID", + volOptions: map[string]string{ + "encrypted": "true", + "encryptionKMSID": "valid-kms-id", + }, + fallbackType: util.EncryptionTypeBlock, + expectedKMS: "valid-kms-id", + expectedEnc: util.EncryptionTypeBlock, + expectedErr: false, + }, + } + + for _, tt := range tests { + newtt := tt + t.Run(newtt.testName, func(t *testing.T) { + t.Parallel() + actualKMS, actualEnc, actualErr := ParseEncryptionOpts( + newtt.volOptions, + newtt.fallbackType, + ) + if actualKMS != newtt.expectedKMS { + t.Errorf("Expected KMS ID: %s, but got: %s", newtt.expectedKMS, actualKMS) + } + + if actualEnc != newtt.expectedEnc { + t.Errorf("Expected Encryption Type: %v, but got: %v", newtt.expectedEnc, actualEnc) + } + + if (actualErr != nil) != newtt.expectedErr { + t.Errorf("expected error %v but got %v", newtt.expectedErr, actualErr) + } + }) + } +}