Support whitespaces in opaqueports annotation (#8355)

Opaque ports may be configured through annotations, where the value may
be either as a range (e.g 0 - 255), or as a comma delimited string
("121,122"). When configured as a comma delimited string, our parsing
logic will trim any leading and trailing commas and split the value;
ports will be converted from string to an int counterpart.

If whitespaces are used, such that the value looks similar to "121,
122", the parsing logic will fail -- when attempting to convert the
string into an integer -- with the following error "\" 122\" is not a
valid lower-bound". This can lead to confusion from users whose services
and endpoints function with undefined behaviour.

This change introduces logic to strip any leading or trailing
whitespaces from strings when tokenizing the annotation value; this way,
we are guaranteed not to experience parsing errors when converting
strings. To validate the behaviour, a new (unit) test has been added to
the opaque ports watcher with a multi-opaque-port annotation on a
service, where the value contains a space.

Signed-off-by: Matei David <matei@buoyant.io>

controller/api/destination/watcher/opaque_ports_watcher_test.go

@@ -53,6 +53,24 @@ metadata:
 			Ports: []corev1.ServicePort{{Port: 3306}},
 		},
 	}
+	opaqueServiceMultiPort = `
+apiVersion: v1
+kind: Service
+metadata:
+  name: svc
+  namespace: ns
+  annotations:
+    config.linkerd.io/opaque-ports: "3306, 665"`
+	opaqueServiceMultiPortObject = corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "svc",
+			Namespace:   "ns",
+			Annotations: map[string]string{"config.linkerd.io/opaque-ports": "3306, 665"},
+		},
+		Spec: corev1.ServiceSpec{
+			Ports: []corev1.ServicePort{{Port: 3306}, {Port: 665}},
+		},
+	}
 	explicitlyNotOpaqueService = `
 apiVersion: v1
 kind: Service
@@ -135,6 +153,21 @@ func TestOpaquePortsWatcher(t *testing.T) {
 			// 3. svc created: update with port 3306
 			expectedOpaquePorts: []map[uint32]struct{}{{3306: {}}, {11211: {}, 25: {}, 3306: {}, 443: {}, 5432: {}, 587: {}}, {3306: {}}},
 		},
+		{
+			name:         "namespace with multi port opaque service",
+			initialState: []string{testNS, opaqueServiceMultiPort},
+			nsObject:     &testNSObject,
+			svcObject:    &opaqueServiceMultiPortObject,
+			service: ServiceID{
+				Name:      "svc",
+				Namespace: "ns",
+			},
+			// 1: svc annotation 3306, 665 (with whitespace)
+			// 2: svc updated: no update
+			// 2: svc deleted: update with default ports
+			// 3. svc created: update with port 3306, 665
+			expectedOpaquePorts: []map[uint32]struct{}{{3306: {}, 665: {}}, {11211: {}, 25: {}, 3306: {}, 443: {}, 5432: {}, 587: {}}, {3306: {}, 665: {}}},
+		},
 		{
 			name:         "namespace and service, create opaque service",
 			initialState: []string{testNS, baseService},

pkg/util/parsing.go

@@ -56,7 +56,12 @@ func ParseContainerOpaquePorts(override string, containers []corev1.Container) [
 
 // GetPortRanges gets port ranges from an override annotation
 func GetPortRanges(override string) []string {
-	return strings.Split(strings.TrimSuffix(override, ","), ",")
+	var ports []string
+	for _, port := range strings.Split(strings.TrimSuffix(override, ","), ",") {
+		ports = append(ports, strings.TrimSpace(port))
+	}
+
+	return ports
 }
 
 // isNamed checks if a port range is actually a container named port (e.g.