Merge pull request #2340 from erictune/refactor_kube_auth

Refactor kube auth

cmd/e2e/e2e.go

@@ -27,7 +27,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubecfg"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/golang/glog"
@@ -78,18 +78,13 @@ func loadClientOrDie() *client.Client {
 	config := client.Config{
 		Host: *host,
 	}
-	auth, err := kubecfg.LoadAuthInfo(*authConfig, os.Stdin)
+	auth, err := clientauth.LoadFromFile(*authConfig)
 	if err != nil {
 		glog.Fatalf("Error loading auth: %v", err)
 	}
-	config.Username = auth.User
+	config, err = auth.MergeWithConfig(config)
-	config.Password = auth.Password
+	if err != nil {
-	config.CAFile = auth.CAFile
+		glog.Fatalf("Error creating client")
-	config.CertFile = auth.CertFile
-	config.KeyFile = auth.KeyFile
-	config.BearerToken = auth.BearerToken
-	if auth.Insecure != nil {
-		config.Insecure = *auth.Insecure
 	}
 	c, err := client.New(&config)
 	if err != nil {

cmd/kubecfg/kubecfg.go

@@ -199,10 +199,11 @@ func main() {
 
 	if clientConfig.Host == "" {
 		// TODO: eventually apiserver should start on 443 and be secure by default
+		// TODO: don't specify http or https in Host, and infer that from auth options.
 		clientConfig.Host = "http://localhost:8080"
 	}
 	if client.IsConfigTransportTLS(clientConfig) {
-		auth, err := kubecfg.LoadAuthInfo(*authConfig, os.Stdin)
+		auth, err := kubecfg.LoadClientAuthInfoOrPrompt(*authConfig, os.Stdin)
 		if err != nil {
 			glog.Fatalf("Error loading auth: %v", err)
 		}

pkg/clientauth/clientauth.go

@@ -0,0 +1,119 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/*
+Package authcfg defines a file format for holding authentication
+information needed by clients of Kubernetes.  Typically,
+a Kubernetes cluster will put auth info for the admin in a known
+location when it is created, and will (soon) put it in a known
+location within a Container's file tree for Containers that
+need access to the Kubernetes API.
+
+Having a defined format allows:
+  - clients to be implmented in multiple languages
+  - applications which link clients to be portable across
+    clusters with different authentication styles (e.g.
+    some may use SSL Client certs, others may not, etc)
+  - when the format changes, applications only
+    need to update this code.
+
+The file format is json, marshalled from a struct authcfg.Info.
+
+Clinet libraries in other languages should use the same format.
+
+It is not intended to store general preferences, such as default
+namespace, output options, etc.  CLIs (such as kubectl) and UIs should
+develop their own format and may wish to inline the authcfg.Info type.
+
+The authcfg.Info is just a file format.  It is distinct from
+client.Config which holds options for creating a client.Client.
+Helper functions are provided in this package to fill in a
+client.Client from an authcfg.Info.
+
+Example:
+
+    import (
+        "pkg/client"
+        "pkg/clientauth"
+    )
+
+    info, err := clientauth.LoadFromFile(filename)
+    if err != nil {
+      // handle error
+    }
+    clientConfig = client.Config{}
+    clientConfig.Host = "example.com:4901"
+    clientConfig = info.MergeWithConfig()
+    client := client.New(clientConfig)
+    client.ListPods()
+*/
+package clientauth
+
+// TODO: need a way to rotate Tokens.  Therefore, need a way for client object to be reset when the authcfg is updated.
+import (
+	"encoding/json"
+	"io/ioutil"
+	"os"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+)
+
+// Info holds Kubernetes API authorization config.  It is intended
+// to be read/written from a file as a JSON object.
+type Info struct {
+	User        string
+	Password    string
+	CAFile      string
+	CertFile    string
+	KeyFile     string
+	BearerToken string
+	Insecure    *bool
+}
+
+// LoadFromFile parses an Info object from a file path.
+// If the file does not exist, then os.IsNotExist(err) == true
+func LoadFromFile(path string) (*Info, error) {
+	var info Info
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		return nil, err
+	}
+	data, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	err = json.Unmarshal(data, &info)
+	if err != nil {
+		return nil, err
+	}
+	return &info, err
+}
+
+// MergeWithConfig returns a copy of a client.Config with values from the Info.
+// The fields of client.Config with a corresponding field in the Info are set
+// with the value from the Info.
+func (info Info) MergeWithConfig(c client.Config) (client.Config, error) {
+	var config client.Config = c
+	config.Username = info.User
+	config.Password = info.Password
+	config.CAFile = info.CAFile
+	config.CertFile = info.CertFile
+	config.KeyFile = info.KeyFile
+	config.BearerToken = info.BearerToken
+	if info.Insecure != nil {
+		config.Insecure = *info.Insecure
+	}
+	return config, nil
+}

pkg/clientauth/clientauth_test.go

@@ -0,0 +1,69 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package clientauth_test
+
+import (
+	"io/ioutil"
+	"os"
+	"reflect"
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
+)
+
+func TestLoadFromFile(t *testing.T) {
+	loadAuthInfoTests := []struct {
+		authData  string
+		authInfo  *clientauth.Info
+		expectErr bool
+	}{
+		{
+			`{"user": "user", "password": "pass"}`,
+			&clientauth.Info{User: "user", Password: "pass"},
+			false,
+		},
+		{
+			"", nil, true,
+		},
+	}
+	for _, loadAuthInfoTest := range loadAuthInfoTests {
+		tt := loadAuthInfoTest
+		aifile, err := ioutil.TempFile("", "testAuthInfo")
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if tt.authData != "missing" {
+			defer os.Remove(aifile.Name())
+			defer aifile.Close()
+			_, err = aifile.WriteString(tt.authData)
+			if err != nil {
+				t.Errorf("Unexpected error: %v", err)
+			}
+		} else {
+			aifile.Close()
+			os.Remove(aifile.Name())
+		}
+		authInfo, err := clientauth.LoadFromFile(aifile.Name())
+		gotErr := err != nil
+		if gotErr != tt.expectErr {
+			t.Errorf("expected errorness: %v, actual errorness: %v", tt.expectErr, gotErr)
+		}
+		if !reflect.DeepEqual(authInfo, tt.authInfo) {
+			t.Errorf("Expected %v, got %v", tt.authInfo, authInfo)
+		}
+	}
+}

pkg/kubecfg/kubecfg.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util/wait"
@@ -51,23 +52,15 @@ func promptForString(field string, r io.Reader) string {
 	return result
 }
 
-type AuthInfo struct {
-	User        string
-	Password    string
-	CAFile      string
-	CertFile    string
-	KeyFile     string
-	BearerToken string
-	Insecure    *bool
-}
-
 type NamespaceInfo struct {
 	Namespace string
 }
 
-// LoadAuthInfo parses an AuthInfo object from a file path. It prompts user and creates file if it doesn't exist.
+// LoadClientAuthInfoOrPrompt parses a clientauth.Info object from a file path. It prompts user and creates file if it doesn't exist.
-func LoadAuthInfo(path string, r io.Reader) (*AuthInfo, error) {
+// Oddly, it returns a clientauth.Info even if there is an error.
-	var auth AuthInfo
+func LoadClientAuthInfoOrPrompt(path string, r io.Reader) (*clientauth.Info, error) {
+	var auth clientauth.Info
+	// Prompt for user/pass and write a file if none exists.
 	if _, err := os.Stat(path); os.IsNotExist(err) {
 		auth.User = promptForString("Username", r)
 		auth.Password = promptForString("Password", r)
@@ -78,15 +71,11 @@ func LoadAuthInfo(path string, r io.Reader) (*AuthInfo, error) {
 		err = ioutil.WriteFile(path, data, 0600)
 		return &auth, err
 	}
-	data, err := ioutil.ReadFile(path)
+	authPtr, err := clientauth.LoadFromFile(path)
 	if err != nil {
 		return nil, err
 	}
-	err = json.Unmarshal(data, &auth)
+	return authPtr, nil
-	if err != nil {
-		return nil, err
-	}
-	return &auth, err
 }
 
 // LoadNamespaceInfo parses a NamespaceInfo object from a file path. It creates a file at the specified path if it doesn't exist with the default namespace.

pkg/kubecfg/kubecfg_test.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
 )
 
 func validateAction(expectedAction, actualAction client.FakeAction, t *testing.T) {
@@ -290,15 +291,15 @@ func TestLoadNamespaceInfo(t *testing.T) {
 	}
 }
 
-func TestLoadAuthInfo(t *testing.T) {
+func TestLoadClientAuthInfoOrPrompt(t *testing.T) {
 	loadAuthInfoTests := []struct {
 		authData string
-		authInfo *AuthInfo
+		authInfo *clientauth.Info
 		r        io.Reader
 	}{
 		{
 			`{"user": "user", "password": "pass"}`,
-			&AuthInfo{User: "user", Password: "pass"},
+			&clientauth.Info{User: "user", Password: "pass"},
 			nil,
 		},
 		{
@@ -306,7 +307,7 @@ func TestLoadAuthInfo(t *testing.T) {
 		},
 		{
 			"missing",
-			&AuthInfo{User: "user", Password: "pass"},
+			&clientauth.Info{User: "user", Password: "pass"},
 			bytes.NewBufferString("user\npass"),
 		},
 	}
@@ -327,10 +328,10 @@ func TestLoadAuthInfo(t *testing.T) {
 			aifile.Close()
 			os.Remove(aifile.Name())
 		}
-		authInfo, err := LoadAuthInfo(aifile.Name(), tt.r)
+		authInfo, err := LoadClientAuthInfoOrPrompt(aifile.Name(), tt.r)
 		if len(tt.authData) == 0 && tt.authData != "missing" {
 			if err == nil {
-				t.Error("LoadAuthInfo didn't fail on empty file")
+				t.Error("LoadClientAuthInfoOrPrompt didn't fail on empty file")
 			}
 			continue
 		}

pkg/kubectl/cmd/cmd.go

@@ -175,7 +175,9 @@ func GetKubeConfig(cmd *cobra.Command) *client.Config {
 		// command line). Override them with the command line parameters, if
 		// provided.
 		authPath := GetFlagString(cmd, "auth-path")
-		authInfo, err := kubectl.LoadAuthInfo(authPath, os.Stdin)
+		authInfo, err := kubectl.LoadClientAuthInfoOrPrompt(authPath, os.Stdin)
+		// TODO: handle the case where the file could not be written but
+		// we still got a user/pass from prompting.
 		if err != nil {
 			glog.Fatalf("Error loading auth: %v", err)
 		}

pkg/kubectl/kubectl.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
@@ -56,16 +57,6 @@ func GetKubeClient(config *client.Config, matchVersion bool) (*client.Client, er
 	return c, nil
 }
 
-type AuthInfo struct {
-	User        string
-	Password    string
-	CAFile      string
-	CertFile    string
-	KeyFile     string
-	BearerToken string
-	Insecure    *bool
-}
-
 type NamespaceInfo struct {
 	Namespace string
 }
@@ -99,9 +90,10 @@ func SaveNamespaceInfo(path string, ns *NamespaceInfo) error {
 	return err
 }
 
-// LoadAuthInfo parses an AuthInfo object from a file path. It prompts user and creates file if it doesn't exist.
+// LoadClientAuthInfoOrPrompt parses an AuthInfo object from a file path. It prompts user and creates file if it doesn't exist.
-func LoadAuthInfo(path string, r io.Reader) (*AuthInfo, error) {
+func LoadClientAuthInfoOrPrompt(path string, r io.Reader) (*clientauth.Info, error) {
-	var auth AuthInfo
+	var auth clientauth.Info
+	// Prompt for user/pass and write a file if none exists.
 	if _, err := os.Stat(path); os.IsNotExist(err) {
 		auth.User = promptForString("Username", r)
 		auth.Password = promptForString("Password", r)
@@ -112,15 +104,11 @@ func LoadAuthInfo(path string, r io.Reader) (*AuthInfo, error) {
 		err = ioutil.WriteFile(path, data, 0600)
 		return &auth, err
 	}
-	data, err := ioutil.ReadFile(path)
+	authPtr, err := clientauth.LoadFromFile(path)
 	if err != nil {
 		return nil, err
 	}
-	err = json.Unmarshal(data, &auth)
+	return authPtr, nil
-	if err != nil {
-		return nil, err
-	}
-	return &auth, err
 }
 
 func promptForString(field string, r io.Reader) string {

pkg/kubectl/kubectl_test.go

@@ -25,6 +25,7 @@ import (
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth"
 )
 
 func validateAction(expectedAction, actualAction client.FakeAction, t *testing.T) {
@@ -85,15 +86,15 @@ func TestLoadNamespaceInfo(t *testing.T) {
 	}
 }
 
-func TestLoadAuthInfo(t *testing.T) {
+func TestLoadClientAuthInfoOrPrompt(t *testing.T) {
 	loadAuthInfoTests := []struct {
 		authData string
-		authInfo *AuthInfo
+		authInfo *clientauth.Info
 		r        io.Reader
 	}{
 		{
 			`{"user": "user", "password": "pass"}`,
-			&AuthInfo{User: "user", Password: "pass"},
+			&clientauth.Info{User: "user", Password: "pass"},
 			nil,
 		},
 		{
@@ -101,7 +102,7 @@ func TestLoadAuthInfo(t *testing.T) {
 		},
 		{
 			"missing",
-			&AuthInfo{User: "user", Password: "pass"},
+			&clientauth.Info{User: "user", Password: "pass"},
 			bytes.NewBufferString("user\npass"),
 		},
 	}
@@ -122,10 +123,10 @@ func TestLoadAuthInfo(t *testing.T) {
 			aifile.Close()
 			os.Remove(aifile.Name())
 		}
-		authInfo, err := LoadAuthInfo(aifile.Name(), tt.r)
+		authInfo, err := LoadClientAuthInfoOrPrompt(aifile.Name(), tt.r)
 		if len(tt.authData) == 0 && tt.authData != "missing" {
 			if err == nil {
-				t.Error("LoadAuthInfo didn't fail on empty file")
+				t.Error("LoadClientAuthInfoOrPrompt didn't fail on empty file")
 			}
 			continue
 		}
@@ -133,7 +134,7 @@ func TestLoadAuthInfo(t *testing.T) {
 			t.Errorf("Unexpected error: %v", err)
 		}
 		if !reflect.DeepEqual(authInfo, tt.authInfo) {
-			t.Errorf("Expected %v, got %v", tt.authInfo, authInfo)
+			t.Errorf("Expected %#v, got %#v", tt.authInfo, authInfo)
 		}
 	}
 }