fix: --format outputs any string, --https only subsitute http URL scheme

cmd/minikube service
--format: outputs arbitrarily formatted strings
--https: https only substituted when the URL scheme is http

Author: doodlesbykumbi

pkg/minikube/service/service.go

@@ -184,12 +184,7 @@ func printURLsForService(c corev1.CoreV1Interface, ip, service, namespace string
 			return nil, err
 		}
 
-		u, err := url.Parse(doc.String())
-		if err != nil {
-			return nil, err
-		}
-
-		urls = append(urls, u.String())
+		urls = append(urls, doc.String())
 	}
 	return urls, nil
 }
@@ -236,6 +231,21 @@ func checkEndpointReady(endpoints corev1.EndpointsInterface, service string) err
 	return nil
 }
 
+func OptionallyHttpsFormattedUrlString(bareUrlString string, https bool) (string, bool) {
+	httpsFormattedString := bareUrlString
+	isHttpSchemedURL := false
+
+	if u, parseErr := url.Parse(bareUrlString); parseErr == nil {
+		isHttpSchemedURL = u.Scheme == "http"
+	}
+
+	if isHttpSchemedURL && https {
+		httpsFormattedString = strings.Replace(bareUrlString, "http", "https", 1)
+	}
+
+	return httpsFormattedString, isHttpSchemedURL
+}
+
 func WaitAndMaybeOpenService(api libmachine.API, namespace string, service string, urlTemplate *template.Template, urlMode bool, https bool,
 	wait int, interval int) error {
 	if err := util.RetryAfter(wait, func() error { return CheckService(namespace, service) }, time.Duration(interval)*time.Second); err != nil {
@@ -246,15 +256,14 @@ func WaitAndMaybeOpenService(api libmachine.API, namespace string, service strin
 	if err != nil {
 		return errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace")
 	}
-	for _, url := range urls {
-		if https {
-			url = strings.Replace(url, "http", "https", 1)
-		}
-		if urlMode || !strings.HasPrefix(url, "http") {
-			fmt.Fprintln(os.Stdout, url)
+	for _, bareUrlString := range urls {
+		urlString, isHttpSchemedURL := OptionallyHttpsFormattedUrlString(bareUrlString, https)
+
+		if urlMode || !isHttpSchemedURL {
+			fmt.Fprintln(os.Stdout, urlString)
 		} else {
 			fmt.Fprintln(os.Stderr, "Opening kubernetes service "+namespace+"/"+service+" in default browser...")
-			browser.OpenURL(url)
+			browser.OpenURL(urlString)
 		}
 	}
 	return nil

pkg/minikube/service/service_test.go

@@ -249,6 +249,13 @@ func TestPrintURLsForService(t *testing.T) {
 			tmpl:           defaultTemplate,
 			expectedOutput: []string{"http://127.0.0.1:1111", "http://127.0.0.1:2222"},
 		},
+		{
+			description:    "should get all node ports with arbitrary format",
+			serviceName:    "mock-dashboard",
+			namespace:      "default",
+			tmpl:           template.Must(template.New("svc-arbitrary-template").Parse("{{.IP}}:{{.Port}}")),
+			expectedOutput: []string{"127.0.0.1:1111", "127.0.0.1:2222"},
+		},
 		{
 			description:    "empty slice for no node ports",
 			serviceName:    "mock-dashboard-no-ports",
@@ -279,6 +286,63 @@ func TestPrintURLsForService(t *testing.T) {
 	}
 }
 
+func TestOptionallyHttpsFormattedUrlString(t *testing.T) {
+
+	var tests = []struct {
+		description                     string
+		bareUrlString                   string
+		https                           bool
+		expectedHttpsFormattedUrlString string
+		expectedIsHttpSchemedURL        bool
+	}{
+		{
+			description:   "no https for http schemed with no https option",
+			bareUrlString: "http://192.168.99.100:30563",
+			https:         false,
+			expectedHttpsFormattedUrlString: "http://192.168.99.100:30563",
+			expectedIsHttpSchemedURL:        true,
+		},
+		{
+			description:   "no https for non-http schemed with no https option",
+			bareUrlString: "xyz.http.myservice:30563",
+			https:         false,
+			expectedHttpsFormattedUrlString: "xyz.http.myservice:30563",
+			expectedIsHttpSchemedURL:        false,
+		},
+		{
+			description:   "https for http schemed with https option",
+			bareUrlString: "http://192.168.99.100:30563",
+			https:         true,
+			expectedHttpsFormattedUrlString: "https://192.168.99.100:30563",
+			expectedIsHttpSchemedURL:        true,
+		},
+		{
+			description:   "no https for non-http schemed with https option and http substring",
+			bareUrlString: "xyz.http.myservice:30563",
+			https:         true,
+			expectedHttpsFormattedUrlString: "xyz.http.myservice:30563",
+			expectedIsHttpSchemedURL:        false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.description, func(t *testing.T) {
+			t.Parallel()
+			httpsFormattedUrlString, isHttpSchemedURL := OptionallyHttpsFormattedUrlString(test.bareUrlString, test.https)
+
+			if httpsFormattedUrlString != test.expectedHttpsFormattedUrlString {
+				t.Errorf("\nhttpsFormattedUrlString, Expected %v \nActual: %v \n\n", test.expectedHttpsFormattedUrlString, httpsFormattedUrlString)
+			}
+
+			if isHttpSchemedURL != test.expectedIsHttpSchemedURL {
+				t.Errorf("\nisHttpSchemedURL, Expected %v \nActual: %v \n\n",
+					test.expectedHttpsFormattedUrlString, httpsFormattedUrlString)
+			}
+		})
+	}
+}
+
 func TestGetServiceURLs(t *testing.T) {
 	defaultAPI := &tests.MockAPI{
 		Hosts: map[string]*host.Host{