Set current namespace from kubeconfig by default (#172)

* Set current namespace from kubeconfig by default Currently kn command does not pick up namespace from kubeconfig but hardcorded default namespace. This patch fixes to get namespace from kubeconfig. Fixes #7 * Use NamespaceFactory to get current namespace * Update unit tests * Add nil check for ClientConfig
knative · Jun 24, 2019 · 13ff277 · 13ff277
1 parent 9c865a6
commit 13ff277
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 73 deletions.
diff --git a/cmd/kn/main.go b/cmd/kn/main.go
@@ -21,10 +21,6 @@ import (
 	"github.com/knative/client/pkg/kn/core"
 )
 
-func init() {
-	core.InitializeConfig()
-}
-
 func main() {
 	err := core.NewKnCommand().Execute()
 	if err != nil {

diff --git a/pkg/kn/commands/namespaced.go b/pkg/kn/commands/namespaced.go
@@ -19,9 +19,6 @@ import (
 	"github.com/spf13/pflag"
 )
 
-// TODO: default namespace should be same scope for the request
-const defaultNamespace = "default"
-
 // AddNamespaceFlags adds the namespace-related flags:
 // * --namespace
 // * --all-namespaces
@@ -43,7 +40,7 @@ func AddNamespaceFlags(flags *pflag.FlagSet, allowAll bool) {
 }
 
 // GetNamespace returns namespace from command specified by flag
-func GetNamespace(cmd *cobra.Command) (string, error) {
+func (kn *KnParams) GetNamespace(cmd *cobra.Command) (string, error) {
 	namespace := cmd.Flag("namespace").Value.String()
 	// check value of all-namepace only if its defined
 	if cmd.Flags().Lookup("all-namespaces") != nil {
@@ -57,7 +54,11 @@ func GetNamespace(cmd *cobra.Command) (string, error) {
 	}
 	// if all-namepaces=False or namespace not given, use default namespace
 	if namespace == "" {
-		namespace = defaultNamespace
+		var err error
+		namespace, err = kn.NamespaceFactory()
+		if err != nil {
+			return "", err
+		}
 	}
 	return namespace, nil
 }
diff --git a/pkg/kn/commands/namespaced_test.go b/pkg/kn/commands/namespaced_test.go
@@ -36,7 +36,8 @@ func TestGetNamespaceSample(t *testing.T) {
 	expectedNamespace := "test1"
 	testCmd.SetArgs([]string{"--namespace", expectedNamespace})
 	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
+	kp := &KnParams{NamespaceFactory: func() (string, error) { return FakeNamespace, nil }}
+	actualNamespace, err := kp.GetNamespace(testCmd)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -45,12 +46,13 @@ func TestGetNamespaceSample(t *testing.T) {
 	}
 }
 
-// test without setting any namespace
+// test current namespace without setting any namespace
 func TestGetNamespaceDefault(t *testing.T) {
 	testCmd := testCommandGenerator(true)
-	expectedNamespace := "default"
+	expectedNamespace := "current"
 	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
+	kp := &KnParams{NamespaceFactory: func() (string, error) { return FakeNamespace, nil }}
+	actualNamespace, err := kp.GetNamespace(testCmd)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -67,7 +69,8 @@ func TestGetNamespaceAllNamespacesSet(t *testing.T) {
 	sampleNamespace := "test1"
 	testCmd.SetArgs([]string{"--namespace", sampleNamespace, "--all-namespaces"})
 	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
+	kp := &KnParams{NamespaceFactory: func() (string, error) { return FakeNamespace, nil }}
+	actualNamespace, err := kp.GetNamespace(testCmd)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -83,7 +86,8 @@ func TestGetNamespaceDefaultAllNamespacesUnset(t *testing.T) {
 	expectedNamespace := ""
 	testCmd.SetArgs([]string{"--all-namespaces"})
 	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
+	kp := &KnParams{NamespaceFactory: func() (string, error) { return FakeNamespace, nil }}
+	actualNamespace, err := kp.GetNamespace(testCmd)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -98,21 +102,8 @@ func TestGetNamespaceAllNamespacesNotDefined(t *testing.T) {
 	expectedNamespace := "test1"
 	testCmd.SetArgs([]string{"--namespace", expectedNamespace})
 	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if actualNamespace != expectedNamespace {
-		t.Fatalf("Incorrect namespace retrieved: %v, expected: %v", actualNamespace, expectedNamespace)
-	}
-}
-
-// test with all-namespace flag not defined and no namespace given
-func TestGetNamespaceDefaultAllNamespacesNotDefined(t *testing.T) {
-	testCmd := testCommandGenerator(false)
-	expectedNamespace := "default"
-	testCmd.Execute()
-	actualNamespace, err := GetNamespace(testCmd)
+	kp := &KnParams{NamespaceFactory: func() (string, error) { return FakeNamespace, nil }}
+	actualNamespace, err := kp.GetNamespace(testCmd)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/pkg/kn/commands/revision/revision_describe.go b/pkg/kn/commands/revision/revision_describe.go
@@ -39,7 +39,7 @@ func NewRevisionDescribeCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/revision/revision_list.go b/pkg/kn/commands/revision/revision_list.go
@@ -44,7 +44,7 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/service/service_create.go b/pkg/kn/commands/service/service_create.go
@@ -59,7 +59,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 				return errors.New("requires the image name to run.")
 			}
 
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/service/service_create_test.go b/pkg/kn/commands/service/service_create_test.go
@@ -73,7 +73,7 @@ func TestServiceCreateImage(t *testing.T) {
 	} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
 		t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
 	} else if !strings.Contains(output, "foo") || !strings.Contains(output, "created") ||
-		!strings.Contains(output, "default") {
+		!strings.Contains(output, commands.FakeNamespace) {
 		t.Fatalf("wrong stdout message: %v", output)
 	}
 }
@@ -338,7 +338,7 @@ func TestServiceCreateImageForce(t *testing.T) {
 		t.Fatal(err)
 	} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
 		t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
-	} else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
+	} else if !strings.Contains(output, "foo") || !strings.Contains(output, commands.FakeNamespace) {
 		t.Fatalf("wrong output: %s", output)
 	}
 }
@@ -375,7 +375,7 @@ func TestServiceCreateEnvForce(t *testing.T) {
 		actualEnvVars,
 		expectedEnvVars) {
 		t.Fatalf("wrong env vars:%v", template.Spec.DeprecatedContainer.Env)
-	} else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
+	} else if !strings.Contains(output, "foo") || !strings.Contains(output, commands.FakeNamespace) {
 		t.Fatalf("wrong output: %s", output)
 	}
 }
diff --git a/pkg/kn/commands/service/service_delete.go b/pkg/kn/commands/service/service_delete.go
@@ -43,7 +43,7 @@ func NewServiceDeleteCommand(p *commands.KnParams) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/service/service_describe.go b/pkg/kn/commands/service/service_describe.go
@@ -38,7 +38,7 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/service/service_list.go b/pkg/kn/commands/service/service_list.go
@@ -35,7 +35,7 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/service/service_update.go b/pkg/kn/commands/service/service_update.go
@@ -40,7 +40,7 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command {
 				return errors.New("requires the service name.")
 			}
 
-			namespace, err := commands.GetNamespace(cmd)
+			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
 			}

diff --git a/pkg/kn/commands/test_helper.go b/pkg/kn/commands/test_helper.go
@@ -24,11 +24,14 @@ import (
 	client_testing "k8s.io/client-go/testing"
 )
 
+const FakeNamespace = "current"
+
 func CreateTestKnCommand(cmd *cobra.Command, knParams *KnParams) (*cobra.Command, *fake.FakeServingV1alpha1, *bytes.Buffer) {
 	buf := new(bytes.Buffer)
 	fakeServing := &fake.FakeServingV1alpha1{&client_testing.Fake{}}
 	knParams.Output = buf
 	knParams.ServingFactory = func() (serving.ServingV1alpha1Interface, error) { return fakeServing, nil }
+	knParams.NamespaceFactory = func() (string, error) { return FakeNamespace, nil }
 	knCommand := newKnCommand(cmd, knParams)
 	return knCommand, fakeServing, buf
 }
@@ -56,7 +59,7 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`,
 		rootCmd.SetOutput(params.Output)
 	}
 	rootCmd.PersistentFlags().StringVar(&CfgFile, "config", "", "config file (default is $HOME/.kn.yaml)")
-	rootCmd.PersistentFlags().StringVar(&KubeCfgFile, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
+	rootCmd.PersistentFlags().StringVar(&params.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
 
 	rootCmd.AddCommand(subCommand)
 

diff --git a/pkg/kn/commands/types.go b/pkg/kn/commands/types.go
@@ -24,23 +24,39 @@ import (
 // CfgFile is Kn's config file is the path for the Kubernetes config
 var CfgFile string
 
-// KubeCfgFile is the path for the Kubernetes config
-var KubeCfgFile string
-
 // Parameters for creating commands. Useful for inserting mocks for testing.
 type KnParams struct {
-	Output         io.Writer
-	ServingFactory func() (serving.ServingV1alpha1Interface, error)
+	Output           io.Writer
+	ServingFactory   func() (serving.ServingV1alpha1Interface, error)
+	NamespaceFactory func() (string, error)
+
+	KubeCfgPath  string
+	ClientConfig clientcmd.ClientConfig
 }
 
 func (c *KnParams) Initialize() {
 	if c.ServingFactory == nil {
-		c.ServingFactory = GetConfig
+		c.ServingFactory = c.GetConfig
+	}
+	if c.NamespaceFactory == nil {
+		c.NamespaceFactory = c.CurrentNamespace
+	}
+}
+
+func (c *KnParams) CurrentNamespace() (string, error) {
+	if c.ClientConfig == nil {
+		c.ClientConfig = c.GetClientConfig()
 	}
+	name, _, err := c.ClientConfig.Namespace()
+	return name, err
 }
 
-func GetConfig() (serving.ServingV1alpha1Interface, error) {
-	config, err := clientcmd.BuildConfigFromFlags("", KubeCfgFile)
+func (c *KnParams) GetConfig() (serving.ServingV1alpha1Interface, error) {
+	if c.ClientConfig == nil {
+		c.ClientConfig = c.GetClientConfig()
+	}
+	var err error
+	config, err := c.ClientConfig.ClientConfig()
 	if err != nil {
 		return nil, err
 	}
@@ -50,3 +66,11 @@ func GetConfig() (serving.ServingV1alpha1Interface, error) {
 	}
 	return client, nil
 }
+
+func (c *KnParams) GetClientConfig() clientcmd.ClientConfig {
+	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
+	if len(c.KubeCfgPath) > 0 {
+		loadingRules.ExplicitPath = c.KubeCfgPath
+	}
+	return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{})
+}
diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go
@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"path/filepath"
 
 	"github.com/knative/client/pkg/kn/commands"
 	"github.com/knative/client/pkg/kn/commands/revision"
@@ -65,7 +64,7 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`,
 		rootCmd.SetOutput(p.Output)
 	}
 	rootCmd.PersistentFlags().StringVar(&commands.CfgFile, "config", "", "config file (default is $HOME/.kn/config.yaml)")
-	rootCmd.PersistentFlags().StringVar(&commands.KubeCfgFile, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
+	rootCmd.PersistentFlags().StringVar(&p.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")
 
 	rootCmd.AddCommand(service.NewServiceCommand(p))
 	rootCmd.AddCommand(revision.NewRevisionCommand(p))
@@ -77,28 +76,6 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`,
 	return rootCmd
 }
 
-func InitializeConfig() {
-	cobra.OnInitialize(initConfig)
-	cobra.OnInitialize(initKubeConfig)
-
-}
-
-func initKubeConfig() {
-	if commands.KubeCfgFile != "" {
-		return
-	}
-	if kubeEnvConf, ok := os.LookupEnv("KUBECONFIG"); ok {
-		commands.KubeCfgFile = kubeEnvConf
-	} else {
-		home, err := homedir.Dir()
-		if err != nil {
-			fmt.Fprintln(os.Stderr, err)
-			os.Exit(1)
-		}
-		commands.KubeCfgFile = filepath.Join(home, ".kube", "config")
-	}
-}
-
 // initConfig reads in config file and ENV variables if set.
 func initConfig() {
 	if commands.CfgFile != "" {