From 6ff69a4e7217bb8483871461c23575b38cd03baa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kristoffer=20Gr=C3=B6nlund?= <kgronlund@suse.com>
Date: Thu, 11 Jul 2019 18:05:36 +0200
Subject: [PATCH] k8sutil: Ensure rook version label has valid syntax
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace any invalid characters and ensure length
and format for the rook version label, to avoid
problems with version strings that contain
illegal characters (common characters that aren't
valid in labels would be + and ~, for example).

Kubernetes label syntax:
https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set

Fixes: #3434
Signed-off-by: Kristoffer Grönlund <kgronlund@suse.com>
---
 pkg/operator/k8sutil/k8sutil.go      | 27 ++++++++++++++++++++++++++-
 pkg/operator/k8sutil/k8sutil_test.go | 15 +++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/pkg/operator/k8sutil/k8sutil.go b/pkg/operator/k8sutil/k8sutil.go
index 831bc8ee2..cb6a5e05c 100644
--- a/pkg/operator/k8sutil/k8sutil.go
+++ b/pkg/operator/k8sutil/k8sutil.go
@@ -20,6 +20,7 @@ package k8sutil
 import (
 	"crypto/md5"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -138,6 +139,30 @@ func deleteResourceAndWait(namespace, name, resourceType string,
 // will result in the deployment/daemonset/ect. recreating all of its pods even if an update
 // wouldn't otherwise be required. Upgrading unnecessarily increases risk for loss of data
 // reliability, even if only briefly.
+//
+// Note that the label may not match the version string exactly, since some characters used
+// in version strings are illegal in pod labels.
 func addRookVersionLabel(labels map[string]string) {
-	labels[RookVersionLabelKey] = rookversion.Version
+	label := rookversion.Version
+	labels[RookVersionLabelKey] = validateLabelValue(label)
+}
+
+// validateLabelValue replaces any invalid characters
+// in the input string with a replacement character,
+// and enforces other limitations for k8s label values.
+//
+// See https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
+func validateLabelValue(value string) string {
+	repl := "-"
+	maxlen := 63
+	re := regexp.MustCompile("[^a-z0-9A-Z._-]")
+	// restrict label value to valid character set
+	sanitized := re.ReplaceAllLiteralString(value, repl)
+	// ensure value begins and ends with a valid character
+	sanitized = strings.TrimRight(strings.TrimLeft(sanitized, ".-_"), ".-_")
+	// limit length
+	if len(sanitized) > maxlen {
+		sanitized = sanitized[:maxlen]
+	}
+	return sanitized
 }
diff --git a/pkg/operator/k8sutil/k8sutil_test.go b/pkg/operator/k8sutil/k8sutil_test.go
index 138c032c1..bca85c483 100644
--- a/pkg/operator/k8sutil/k8sutil_test.go
+++ b/pkg/operator/k8sutil/k8sutil_test.go
@@ -51,3 +51,18 @@ func TestTruncateNodeName(t *testing.T) {
 		assert.Equal(t, result, TruncateNodeName(params[0], params[1]))
 	}
 }
+
+func TestValidateLabelValue(t *testing.T) {
+	// The key is the result, and the value is the input.
+	tests := map[string]string{
+		"":                        "",
+		"1.0":                     "1.0",
+		"1.0.0-git1697.ga265cdfd": "1.0.0+git1697.ga265cdfd",
+		"this-entry-is-more-than-63-characters-long-so-it-will-be-trunca": "this-entry-is-more-than-63-characters-long-so-it-will-be-truncated",
+		"1": ".1.",
+	}
+
+	for result, input := range tests {
+		assert.Equal(t, result, validateLabelValue(input))
+	}
+}