Browse code

Issue 2358 - better handling of server url parsing in osc login

fabianofranz authored on 2015/05/21 02:35:23
Showing 4 changed files
... ...
@@ -169,17 +169,27 @@ if [[ "${API_SCHEME}" == "https" ]]; then
169 169
 fi
170 170
 
171 171
 # login and logout tests
172
+# --token and --username are mutually exclusive
172 173
 [ "$(osc login ${KUBERNETES_MASTER} -u test-user --token=tmp --insecure-skip-tls-verify 2>&1 | grep 'mutually exclusive')" ]
174
+# must only accept one arg (server)
173 175
 [ "$(osc login https://server1 https://server2.com 2>&1 | grep 'Only the server URL may be specified')" ]
176
+# logs in with a valid certificate authority
174 177
 osc login ${KUBERNETES_MASTER} --certificate-authority="${MASTER_CONFIG_DIR}/ca.crt" -u test-user -p anything
175 178
 osc logout
179
+# logs in skipping certificate check
176 180
 osc login ${KUBERNETES_MASTER} --insecure-skip-tls-verify -u test-user -p anything
181
+# logs in by an existing and valid token
177 182
 temp_token=$(osc config view -o template --template='{{range .users}}{{ index .user.token }}{{end}}')
178 183
 [ "$(osc login --token=${temp_token} 2>&1 | grep 'using the token provided')" ]
179 184
 osc logout
180
-osc login --server=${KUBERNETES_MASTER} --certificate-authority="${MASTER_CONFIG_DIR}/ca.crt" -u test-user -p anything
185
+# properly parse server port
186
+[ "$(osc login https://server1:844333 2>&1 | grep 'Not a valid port')" ]
187
+# properly handle trailing slash
188
+osc login --server=${KUBERNETES_MASTER}/ --certificate-authority="${MASTER_CONFIG_DIR}/ca.crt" -u test-user -p anything
189
+# create a new project
181 190
 osc new-project project-foo --display-name="my project" --description="boring project description"
182 191
 [ "$(osc project | grep 'Using project "project-foo"')" ]
192
+# denies access after logging out
183 193
 osc logout
184 194
 [ -z "$(osc get pods | grep 'system:anonymous')" ]
185 195
 
... ...
@@ -142,11 +142,15 @@ func (o *LoginOptions) Complete(f *osclientcmd.Factory, cmd *cobra.Command, args
142 142
 		o.StartingKubeConfig = kclientcmdapi.NewConfig()
143 143
 	}
144 144
 
145
+	addr := flagtypes.Addr{Value: "localhost:8443", DefaultScheme: "https", DefaultPort: 8443, AllowPrefix: true}.Default()
146
+
145 147
 	if serverFlag := kcmdutil.GetFlagString(cmd, "server"); len(serverFlag) > 0 {
146
-		o.Server = serverFlag
148
+		if err := addr.Set(serverFlag); err != nil {
149
+			return err
150
+		}
151
+		o.Server = addr.String()
147 152
 
148 153
 	} else if len(args) == 1 {
149
-		addr := flagtypes.Addr{Value: "localhost:8443", DefaultScheme: "https", DefaultPort: 8443, AllowPrefix: true}.Default()
150 154
 		if err := addr.Set(args[0]); err != nil {
151 155
 			return err
152 156
 		}
... ...
@@ -158,7 +162,6 @@ func (o *LoginOptions) Complete(f *osclientcmd.Factory, cmd *cobra.Command, args
158 158
 				o.Server = cluster.Server
159 159
 			}
160 160
 		}
161
-
162 161
 	}
163 162
 
164 163
 	if certFile := kcmdutil.GetFlagString(cmd, "client-certificate"); len(certFile) > 0 {
... ...
@@ -46,11 +46,7 @@ func (o *LoginOptions) getClientConfig() (*kclient.Config, error) {
46 46
 
47 47
 	clientConfig := &kclient.Config{}
48 48
 
49
-	// if someone specified a server, use it as the default
50
-	if len(o.Server) > 0 {
51
-		clientConfig.Host = o.Server
52
-
53
-	} else {
49
+	if len(o.Server) == 0 {
54 50
 		// we need to have a server to talk to
55 51
 		if cmdutil.IsTerminal(o.Reader) {
56 52
 			for !o.serverProvided() {
... ...
@@ -58,11 +54,18 @@ func (o *LoginOptions) getClientConfig() (*kclient.Config, error) {
58 58
 				promptMsg := fmt.Sprintf("OpenShift server [%s]: ", defaultServer)
59 59
 
60 60
 				o.Server = cmdutil.PromptForStringWithDefault(o.Reader, defaultServer, promptMsg)
61
-				clientConfig.Host = o.Server
62 61
 			}
63 62
 		}
64 63
 	}
65 64
 
65
+	// normalize the provided server to a format expected by config
66
+	serverNormalized, err := config.NormalizeServerURL(o.Server)
67
+	if err != nil {
68
+		return nil, err
69
+	}
70
+	o.Server = serverNormalized
71
+	clientConfig.Host = o.Server
72
+
66 73
 	if len(o.CAFile) > 0 {
67 74
 		clientConfig.CAFile = o.CAFile
68 75
 
... ...
@@ -1,11 +1,17 @@
1 1
 package config
2 2
 
3 3
 import (
4
+	"fmt"
5
+	"net"
6
+	"net/url"
7
+	"strconv"
8
+	"strings"
9
+
4 10
 	clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api"
5 11
 	"github.com/openshift/origin/pkg/cmd/util"
6 12
 )
7 13
 
8
-// TODO should me moved upstream
14
+// TODO should be moved upstream
9 15
 func RelativizeClientConfigPaths(cfg *clientcmdapi.Config, base string) (err error) {
10 16
 	for k, cluster := range cfg.Clusters {
11 17
 		if len(cluster.CertificateAuthority) > 0 {
... ...
@@ -39,3 +45,59 @@ func RelativizeClientConfigPaths(cfg *clientcmdapi.Config, base string) (err err
39 39
 	}
40 40
 	return nil
41 41
 }
42
+
43
+var validURLSchemes = []string{"https://", "http://", "tcp://"}
44
+
45
+// Opinionated normalization of a string that represents a URL. Returns the URL provided matching the format
46
+// expected when storing a URL in a config. Sets a scheme and port if not present, removes unnecessary trailing
47
+// slashes, etc. Can be used to normalize a URL provided by user input.
48
+func NormalizeServerURL(s string) (string, error) {
49
+	// normalize scheme
50
+	if !hasScheme(s) {
51
+		s = validURLSchemes[0] + s
52
+	}
53
+
54
+	addr, err := url.Parse(s)
55
+	if err != nil {
56
+		return "", fmt.Errorf("Not a valid URL: %v.", err)
57
+	}
58
+
59
+	// normalize host:port
60
+	if strings.Contains(addr.Host, ":") {
61
+		_, port, err := net.SplitHostPort(addr.Host)
62
+		if err != nil {
63
+			return "", fmt.Errorf("Not a valid host:port: %v.", err)
64
+		}
65
+		_, err = strconv.ParseUint(port, 10, 16)
66
+		if err != nil {
67
+			return "", fmt.Errorf("Not a valid port: %v. Port numbers must be between 0 and 65535.", port)
68
+		}
69
+	} else {
70
+		port := 0
71
+		switch addr.Scheme {
72
+		case "http":
73
+			port = 80
74
+		case "https":
75
+			port = 443
76
+		default:
77
+			return "", fmt.Errorf("No port specified.")
78
+		}
79
+		addr.Host = net.JoinHostPort(addr.Host, strconv.FormatInt(int64(port), 10))
80
+	}
81
+
82
+	// remove trailing slash if that's the only path we have
83
+	if addr.Path == "/" {
84
+		addr.Path = ""
85
+	}
86
+
87
+	return addr.String(), nil
88
+}
89
+
90
+func hasScheme(s string) bool {
91
+	for _, p := range validURLSchemes {
92
+		if strings.HasPrefix(s, p) {
93
+			return true
94
+		}
95
+	}
96
+	return false
97
+}