From 445684d44d270c8f6bfd1c7e92bf72228cebf7e2 Mon Sep 17 00:00:00 2001 From: dmpe Date: Sat, 5 Jun 2021 19:17:24 +0200 Subject: [PATCH 1/4] fix(start): minikube start with --image-repository: add new test for URLs with ports, fixing fix validation logic --- cmd/minikube/cmd/start.go | 35 ++++++++++++++++++++++++---------- cmd/minikube/cmd/start_test.go | 24 +++++++++++++---------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 0d4f574e29a8..59d50f1f59a1 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1263,26 +1263,41 @@ func validateRegistryMirror() { // This function validates if the --image-repository // args match the format of registry.cn-hangzhou.aliyuncs.com/google_containers -func validateImageRepository(imagRepo string) (vaildImageRepo string) { +// also "[:]" +func validateImageRepository(imageRepo string) (validImageRepo string) { - if strings.ToLower(imagRepo) == "auto" { - vaildImageRepo = "auto" + if strings.ToLower(imageRepo) == "auto" { + validImageRepo = "auto" } - URL, err := url.Parse(imagRepo) + URL, err := url.Parse(imageRepo) + if err != nil { klog.Errorln("Error Parsing URL: ", err) } - // tips when imagRepo ended with a trailing /. - if strings.HasSuffix(imagRepo, "/") { + + var hasPorts = false + var imageRepoPort string + + if URL.Port() != "" && strings.Contains(imageRepo, ":"+URL.Port()) { + hasPorts = true + imageRepoPort = ":" + URL.Port() + } + // tips when imageRepo ended with a trailing /. + if strings.HasSuffix(imageRepo, "/") { out.Infof("The --image-repository flag your provided ended with a trailing / that could cause conflict in kuberentes, removed automatically") } - // tips when imageRepo started with scheme. + // tips when imageRepo started with scheme "http(s)". if URL.Scheme != "" { - out.Infof("The --image-repository flag your provided contains Scheme: {{.scheme}}, it will be as a domian, removed automatically", out.V{"scheme": URL.Scheme}) + out.Infof("The --image-repository flag your provided contains Scheme: {{.scheme}}, which will be removed automatically", out.V{"scheme": URL.Scheme}) + } + + if hasPorts { + validImageRepo = URL.Hostname() + imageRepoPort + strings.TrimSuffix(URL.Path, "/") + } else { + validImageRepo = URL.Hostname() + strings.TrimSuffix(URL.Path, "/") } - vaildImageRepo = URL.Hostname() + strings.TrimSuffix(URL.Path, "/") - return + return validImageRepo } // This function validates if the --listen-address diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index 5d03d3d95cb7..c7d4b59e0d54 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -320,40 +320,44 @@ func TestBaseImageFlagDriverCombo(t *testing.T) { func TestValidateImageRepository(t *testing.T) { var tests = []struct { imageRepository string - vaildImageRepository string + validImageRepository string }{ { imageRepository: "auto", - vaildImageRepository: "auto", + validImageRepository: "auto", }, { imageRepository: "http://registry.test.com/google_containers/", - vaildImageRepository: "registry.test.com/google_containers", + validImageRepository: "registry.test.com/google_containers", }, { imageRepository: "https://registry.test.com/google_containers/", - vaildImageRepository: "registry.test.com/google_containers", + validImageRepository: "registry.test.com/google_containers", }, { imageRepository: "registry.test.com/google_containers/", - vaildImageRepository: "registry.test.com/google_containers", + validImageRepository: "registry.test.com/google_containers", }, { imageRepository: "http://registry.test.com/google_containers", - vaildImageRepository: "registry.test.com/google_containers", + validImageRepository: "registry.test.com/google_containers", }, { imageRepository: "https://registry.test.com/google_containers", - vaildImageRepository: "registry.test.com/google_containers", + validImageRepository: "registry.test.com/google_containers", + }, + { + imageRepository: "https://registry.test.com:6666/google_containers", + validImageRepository: "registry.test.com:6666/google_containers", }, } for _, test := range tests { t.Run(test.imageRepository, func(t *testing.T) { - vaildImageRepository := validateImageRepository(test.imageRepository) - if vaildImageRepository != test.vaildImageRepository { + validImageRepository := validateImageRepository(test.imageRepository) + if validImageRepository != test.validImageRepository { t.Errorf("validateImageRepository(imageRepo=%v): got %v, expected %v", - test.imageRepository, vaildImageRepository, test.vaildImageRepository) + test.imageRepository, validImageRepository, test.validImageRepository) } }) } From 8badb789e8a62149edb9545acacacce00443c407 Mon Sep 17 00:00:00 2001 From: dmpe Date: Sat, 5 Jun 2021 20:59:52 +0200 Subject: [PATCH 2/4] smaller documentation fix --- cmd/minikube/cmd/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 59d50f1f59a1..8e980e7e7f5d 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1286,7 +1286,7 @@ func validateImageRepository(imageRepo string) (validImageRepo string) { if strings.HasSuffix(imageRepo, "/") { out.Infof("The --image-repository flag your provided ended with a trailing / that could cause conflict in kuberentes, removed automatically") } - // tips when imageRepo started with scheme "http(s)". + // tips when imageRepo started with scheme such as http(s). if URL.Scheme != "" { out.Infof("The --image-repository flag your provided contains Scheme: {{.scheme}}, which will be removed automatically", out.V{"scheme": URL.Scheme}) } From a60b33723739dec0524942a5ff0126ca377cfd8e Mon Sep 17 00:00:00 2001 From: dmpe Date: Mon, 12 Jul 2021 20:15:21 +0200 Subject: [PATCH 3/4] fix an issue with hasPorts variable - further simplification --- cmd/minikube/cmd/start.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 579c24cad8ce..3a6f1ea05256 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1286,13 +1286,12 @@ func validateImageRepository(imageRepo string) (validImageRepo string) { klog.Errorln("Error Parsing URL: ", err) } - var hasPorts = false var imageRepoPort string if URL.Port() != "" && strings.Contains(imageRepo, ":"+URL.Port()) { - hasPorts = true imageRepoPort = ":" + URL.Port() } + // tips when imageRepo ended with a trailing /. if strings.HasSuffix(imageRepo, "/") { out.Infof("The --image-repository flag your provided ended with a trailing / that could cause conflict in kuberentes, removed automatically") @@ -1301,12 +1300,8 @@ func validateImageRepository(imageRepo string) (validImageRepo string) { if URL.Scheme != "" { out.Infof("The --image-repository flag your provided contains Scheme: {{.scheme}}, which will be removed automatically", out.V{"scheme": URL.Scheme}) } - - if hasPorts { - validImageRepo = URL.Hostname() + imageRepoPort + strings.TrimSuffix(URL.Path, "/") - } else { - validImageRepo = URL.Hostname() + strings.TrimSuffix(URL.Path, "/") - } + + validImageRepo = URL.Hostname() + imageRepoPort + strings.TrimSuffix(URL.Path, "/") return validImageRepo } From 0c721cbacd140feae9de84f177924b8862612b6c Mon Sep 17 00:00:00 2001 From: dmpe Date: Mon, 12 Jul 2021 20:23:07 +0200 Subject: [PATCH 4/4] remove whitespace --- cmd/minikube/cmd/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 3a6f1ea05256..d51fec280a0d 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1300,7 +1300,7 @@ func validateImageRepository(imageRepo string) (validImageRepo string) { if URL.Scheme != "" { out.Infof("The --image-repository flag your provided contains Scheme: {{.scheme}}, which will be removed automatically", out.V{"scheme": URL.Scheme}) } - + validImageRepo = URL.Hostname() + imageRepoPort + strings.TrimSuffix(URL.Path, "/") return validImageRepo