From 3eb0fa5e2110b7d7d48fabfb7531da0ad4f4858a Mon Sep 17 00:00:00 2001
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Date: Thu, 10 Mar 2022 16:31:25 +0530
Subject: [PATCH] rbd: fix parsing mapOptions

Currently, we support

mapOption: "krbd:v1,v2,v3;nbd:v1,v2,v3"

- By omitting `krbd:` or `nbd:`, the option(s) apply to
  rbdDefaultMounter which is krbd.
- A user can _override_ the options for a mounter by specifying `krbd:`
  or `nbd:`.
  mapOption: "v1,v2,v3;nbd:v1,v2,v3"
  is effectively the same as the 1st example.
- Sections are split by `;`.
- If users want to specify common options for both `krbd` and `nbd`,
  they should mention them twice.

But in case if the krbd or nbd specifc options contian `:` within them,
then the parsing is failing now.

E0301 10:19:13.615111 7348 utils.go:200] ID: 63 Req-ID:
0001-0009-rook-ceph-0000000000000001-fd37c41b-9948-11ec-ad32-0242ac110004
GRPC error: badly formatted map/unmap options:
"krbd:read_from_replica=localize,crush_location=zone:zone1;"

This patch fix the above case where the options itself contain `:`
delimitor
ex: krbd:v1,v2,v3=v31:v32;nbd:v1,v2,v3"

Please note, if you are using such options which contain `:` delimiter,
then it is mandatory to specify the mounter-type.

Fixes: #2910
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
 internal/rbd/rbd_attach.go      | 18 +++++++-----------
 internal/rbd/rbd_attach_test.go | 17 ++++++++++++-----
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/internal/rbd/rbd_attach.go b/internal/rbd/rbd_attach.go
index 60fce5bbe..7ea8a9466 100644
--- a/internal/rbd/rbd_attach.go
+++ b/internal/rbd/rbd_attach.go
@@ -228,21 +228,19 @@ func setRbdNbdToolFeatures() {
 // returns mounter specific options.
 func parseMapOptions(mapOptions string) (string, string, error) {
 	var krbdMapOptions, nbdMapOptions string
-	const (
-		noKeyLength = 1
-		validLength = 2
-	)
 	for _, item := range strings.Split(mapOptions, ";") {
 		var mounter, options string
 		if item == "" {
 			continue
 		}
-		s := strings.Split(item, ":")
-		switch len(s) {
-		case noKeyLength:
+		s := strings.SplitN(item, ":", 2)
+		if len(s) == 1 {
 			options = strings.TrimSpace(s[0])
 			krbdMapOptions = options
-		case validLength:
+		} else {
+			// options might also contain values delimited with ":", in this
+			// case mounter type MUST be specified.
+			// ex: krbd:read_from_replica=localize,crush_location=zone:zone1;
 			mounter = strings.TrimSpace(s[0])
 			options = strings.TrimSpace(s[1])
 			switch strings.ToLower(mounter) {
@@ -251,10 +249,8 @@ func parseMapOptions(mapOptions string) (string, string, error) {
 			case accessTypeNbd:
 				nbdMapOptions = options
 			default:
-				return "", "", fmt.Errorf("unknown mounter type: %q", mounter)
+				return "", "", fmt.Errorf("unknown mounter type: %q, please specify mounter type", mounter)
 			}
-		default:
-			return "", "", fmt.Errorf("badly formatted map/unmap options: %q", mapOptions)
 		}
 	}
 
diff --git a/internal/rbd/rbd_attach_test.go b/internal/rbd/rbd_attach_test.go
index 07749530d..7b1c6fa0a 100644
--- a/internal/rbd/rbd_attach_test.go
+++ b/internal/rbd/rbd_attach_test.go
@@ -60,18 +60,25 @@ func TestParseMapOptions(t *testing.T) {
 			expectErr:         "",
 		},
 		{
-			name:              "unknown mounter used",
-			mapOption:         "xyz:xOp1,xOp2",
+			name:              "with `:` delimiter used with in the options",
+			mapOption:         "krbd:kOp1,kOp2=kOp21:kOp22;nbd:nOp1,nOp2=nOp21:nOp22",
+			expectKrbdOptions: "kOp1,kOp2=kOp21:kOp22",
+			expectNbdOptions:  "nOp1,nOp2=nOp21:nOp22",
+			expectErr:         "",
+		},
+		{
+			name:              "with `:` delimiter used with in the options, without mounter label",
+			mapOption:         "kOp1,kOp2=kOp21:kOp22;nbd:nOp1,nOp2",
 			expectKrbdOptions: "",
 			expectNbdOptions:  "",
 			expectErr:         "unknown mounter type",
 		},
 		{
-			name:              "bad formatted options",
-			mapOption:         "nbd:nOp1:nOp2;",
+			name:              "unknown mounter used",
+			mapOption:         "xyz:xOp1,xOp2",
 			expectKrbdOptions: "",
 			expectNbdOptions:  "",
-			expectErr:         "badly formatted map/unmap options",
+			expectErr:         "unknown mounter type",
 		},
 	}
 	for _, tt := range tests {