Merge pull request #48808 from kad/safe-docker-tags

Automatic merge from submit-queue (batch tested with PRs 49017, 45440, 48384, 45894, 48808)

Make sure that image tags contain only allowed characters.

**What this PR does / why we need it**:
Version strings can contain symbols that are not allowed
as part of image tags. Replace those with underscores.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:
Part of work related to usage of CI images which has versions like "v1.8.0-alpha.1.910+5ca03d674e1495" but image tags in registry are "v1.8.0-alpha.1.910_5ca03d674e1495"

Attn @luxas 

**Release note**:
```release-note
NONE
```

cmd/kubeadm/app/images/BUILD

@@ -12,7 +12,10 @@ go_library(
     name = "go_default_library",
     srcs = ["images.go"],
     tags = ["automanaged"],
-    deps = ["//cmd/kubeadm/app/apis/kubeadm:go_default_library"],
+    deps = [
+        "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//cmd/kubeadm/app/util:go_default_library",
+    ],
 )
 
 go_test(

cmd/kubeadm/app/images/images.go

@@ -21,6 +21,7 @@ import (
 	"runtime"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 )
 
 const (
@@ -38,11 +39,12 @@ func GetCoreImage(image string, cfg *kubeadmapi.MasterConfiguration, overrideIma
 		return overrideImage
 	}
 	repoPrefix := cfg.ImageRepository
+	kubernetesImageTag := kubeadmutil.KubernetesVersionToImageTag(cfg.KubernetesVersion)
 	return map[string]string{
 		KubeEtcdImage:              fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "etcd", runtime.GOARCH, etcdVersion),
-		KubeAPIServerImage:         fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-apiserver", runtime.GOARCH, cfg.KubernetesVersion),
+		KubeAPIServerImage:         fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-apiserver", runtime.GOARCH, kubernetesImageTag),
-		KubeControllerManagerImage: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-controller-manager", runtime.GOARCH, cfg.KubernetesVersion),
+		KubeControllerManagerImage: fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-controller-manager", runtime.GOARCH, kubernetesImageTag),
-		KubeSchedulerImage:         fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-scheduler", runtime.GOARCH, cfg.KubernetesVersion),
+		KubeSchedulerImage:         fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-scheduler", runtime.GOARCH, kubernetesImageTag),
-		KubeProxyImage:             fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-proxy", runtime.GOARCH, cfg.KubernetesVersion),
+		KubeProxyImage:             fmt.Sprintf("%s/%s-%s:%s", repoPrefix, "kube-proxy", runtime.GOARCH, kubernetesImageTag),
 	}[image]
 }

cmd/kubeadm/app/images/images_test.go

@@ -31,7 +31,8 @@ type getCoreImageTest struct {
 }
 
 const (
-	testversion = "1"
+	testversion = "v10.1.2-alpha.1.100+0123456789abcdef+SOMETHING"
+	expected    = "v10.1.2-alpha.1.100_0123456789abcdef_SOMETHING"
 	gcrPrefix   = "gcr.io/google_containers"
 )
 
@@ -49,22 +50,22 @@ func TestGetCoreImage(t *testing.T) {
 		{getCoreImageTest{
 			i: KubeAPIServerImage,
 			c: &kubeadmapi.MasterConfiguration{ImageRepository: gcrPrefix, KubernetesVersion: testversion}},
-			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-apiserver", runtime.GOARCH, testversion),
+			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-apiserver", runtime.GOARCH, expected),
 		},
 		{getCoreImageTest{
 			i: KubeControllerManagerImage,
 			c: &kubeadmapi.MasterConfiguration{ImageRepository: gcrPrefix, KubernetesVersion: testversion}},
-			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-controller-manager", runtime.GOARCH, testversion),
+			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-controller-manager", runtime.GOARCH, expected),
 		},
 		{getCoreImageTest{
 			i: KubeSchedulerImage,
 			c: &kubeadmapi.MasterConfiguration{ImageRepository: gcrPrefix, KubernetesVersion: testversion}},
-			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-scheduler", runtime.GOARCH, testversion),
+			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-scheduler", runtime.GOARCH, expected),
 		},
 		{getCoreImageTest{
 			i: KubeProxyImage,
 			c: &kubeadmapi.MasterConfiguration{ImageRepository: gcrPrefix, KubernetesVersion: testversion}},
-			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-proxy", runtime.GOARCH, testversion),
+			fmt.Sprintf("%s/%s-%s:%s", gcrPrefix, "kube-proxy", runtime.GOARCH, expected),
 		},
 	}
 	for _, it := range imageTest {

cmd/kubeadm/app/util/version.go

@@ -72,3 +72,14 @@ func KubernetesReleaseVersion(version string) (string, error) {
 	}
 	return "", fmt.Errorf("version %q doesn't match patterns for neither semantic version nor labels (stable, latest, ...)", version)
 }
+
+// KubernetesVersionToImageTag is helper function that replaces all
+// non-allowed symbols in tag strings with underscores.
+// Image tag can only contain lowercase and uppercase letters, digits,
+// underscores, periods and dashes.
+// Current usage is for CI images where all of symbols except '+' are valid,
+// but function is for generic usage where input can't be always pre-validated.
+func KubernetesVersionToImageTag(version string) string {
+	allowed := regexp.MustCompile(`[^-a-zA-Z0-9_\.]`)
+	return allowed.ReplaceAllString(version, "_")
+}

cmd/kubeadm/app/util/version_test.go

@@ -140,3 +140,28 @@ func TestVersionFromNetwork(t *testing.T) {
 		}
 	}
 }
+
+func TestVersionToTag(t *testing.T) {
+	type T struct {
+		input    string
+		expected string
+	}
+	cases := []T{
+		// NOP
+		{"", ""},
+		// Official releases
+		{"v1.0.0", "v1.0.0"},
+		// CI or custom builds
+		{"v10.1.2-alpha.1.100+0123456789abcdef+SOMETHING", "v10.1.2-alpha.1.100_0123456789abcdef_SOMETHING"},
+		// random and invalid input: should return safe value
+		{"v1,0!0+üñµ", "v1_0_0____"},
+	}
+
+	for _, tc := range cases {
+		tag := KubernetesVersionToImageTag(tc.input)
+		t.Logf("KubernetesVersionToImageTag: Input: %q. Result: %q. Expected: %q", tc.input, tag, tc.expected)
+		if tag != tc.expected {
+			t.Errorf("failed KubernetesVersionToImageTag: Input: %q. Result: %q. Expected: %q", tc.input, tag, tc.expected)
+		}
+	}
+}