fix: Invalid "invalid encryption kms configuration" error #3854
Conversation
riya-singhal31
changed the title
Yes, adding an unit test for it. |
| { | ||
| testName: "Valid Encryption Option With KMS ID", | ||
| volOptions: map[string]string{ | ||
| "encrypted": "true", |
There was a problem hiding this comment.
can you add an other test with encrypted set to an invalid string (not true or false)?
There was a problem hiding this comment.
Done, thanks!
|
@riya-singhal31 CI problem, you can run the tests locally by running commands like |
Sure, thanks Madhu. |
e2e/rbd.go
Outdated
| @@ -2872,6 +2872,56 @@ var _ = Describe("RBD", func() { | |||
| } | |||
| }) | |||
|
|
|||
| By("create storageClass with encrypted as false", func() { | |||
| encrypted := "false" | |||
There was a problem hiding this comment.
no need for a new variable just set this in map at line 2886
internal/rbd/encryption.go
Outdated
| @@ -344,6 +344,13 @@ func ParseEncryptionOpts( | |||
| if !ok { | |||
| return "", util.EncryptionTypeNone, nil | |||
| } | |||
| val, er := strconv.ParseBool(encrypted) | |||
| if er != nil { | |||
| return "", util.EncryptionTypeInvalid, fmt.Errorf("strconv.ParseBool: parsing %s: invalid syntax", encrypted) | |||
There was a problem hiding this comment.
Instead of defining our own error just return the error er here?
internal/rbd/encryption_test.go
Outdated
| fallbackType util.EncryptionType | ||
| expectedKMS string | ||
| expectedEnc util.EncryptionType | ||
| expectedErr string |
There was a problem hiding this comment.
Instead of a string, you can just check error is expected or not with a bool value here.
There was a problem hiding this comment.
Thanks for the review, Madhu.
Updated the PR with mentioned changes.
Thanks for the suggestion Niels, updated. |
|
Hi @Madhu-1 , |
|
/test ci/centos/k8s-e2e-external-storage/1.24 |
|
/test ci/centos/k8s-e2e-external-storage/1.25 |
|
/test ci/centos/k8s-e2e-external-storage/1.26 |
|
/test ci/centos/k8s-e2e-external-storage/1.27 |
|
/test ci/centos/mini-e2e-helm/k8s-1.24 |
|
/test ci/centos/mini-e2e/k8s-1.27 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e/k8s-1.26 |
this commit adds the validation for encryption value as false, and sets the type as none Signed-off-by: riya-singhal31 <rsinghal@redhat.com>
Signed-off-by: riya-singhal31 <rsinghal@redhat.com>
Signed-off-by: riya-singhal31 <rsinghal@redhat.com>
Signed-off-by: riya-singhal31 <rsinghal@redhat.com>
|
/test ci/centos/k8s-e2e-external-storage/1.24 |
|
/test ci/centos/k8s-e2e-external-storage/1.25 |
|
/test ci/centos/k8s-e2e-external-storage/1.26 |
|
/test ci/centos/k8s-e2e-external-storage/1.27 |
|
/test ci/centos/mini-e2e-helm/k8s-1.24 |
|
/test ci/centos/mini-e2e-helm/k8s-1.25 |
|
/test ci/centos/mini-e2e-helm/k8s-1.26 |
|
/test ci/centos/mini-e2e-helm/k8s-1.27 |
|
/test ci/centos/mini-e2e/k8s-1.24 |
|
/test ci/centos/mini-e2e/k8s-1.25 |
|
/test ci/centos/mini-e2e/k8s-1.26 |
|
/test ci/centos/mini-e2e/k8s-1.27 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/upgrade-tests-rbd |
|
/cherry-pick release-4.13 |
this commit adds the validation for encryption
value as false, and sets the type as none
Fixes: #3759