From 13ff27709505e2e8b104402daecb610d5d9bfbda Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 25 Jun 2019 03:23:06 +0900 Subject: [PATCH] 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 https://github.com/knative/client/issues/7 * Use NamespaceFactory to get current namespace * Update unit tests * Add nil check for ClientConfig --- cmd/kn/main.go | 4 -- pkg/kn/commands/namespaced.go | 11 ++--- pkg/kn/commands/namespaced_test.go | 33 ++++++--------- pkg/kn/commands/revision/revision_describe.go | 2 +- pkg/kn/commands/revision/revision_list.go | 2 +- pkg/kn/commands/service/service_create.go | 2 +- .../commands/service/service_create_test.go | 6 +-- pkg/kn/commands/service/service_delete.go | 2 +- pkg/kn/commands/service/service_describe.go | 2 +- pkg/kn/commands/service/service_list.go | 2 +- pkg/kn/commands/service/service_update.go | 2 +- pkg/kn/commands/test_helper.go | 5 ++- pkg/kn/commands/types.go | 40 +++++++++++++++---- pkg/kn/core/root.go | 25 +----------- 14 files changed, 65 insertions(+), 73 deletions(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index a261cdb691..154f510560 100644 --- 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 index e945bf6f64..5c2742d77b 100644 --- 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 index dc0bc22baa..9c99705523 100644 --- 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 index 2ca24eb20a..74ec80ece9 100644 --- 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 index aee81e358f..35e4f7efde 100644 --- 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 index 6339810c6d..e20dc1bc06 100644 --- 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 index 682b22ffa9..3627144787 100644 --- 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 index 6e782d12fb..ed14f2f3c5 100644 --- 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 index ce462c39cb..94e9504f5d 100644 --- 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 index bc73863b1e..88578df811 100644 --- 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 index 97ea76426a..e8d0736ade 100644 --- 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 index 1f7de1398a..a0ec06c7dd 100644 --- 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(¶ms.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 index 41e7f6d600..36138c8d82 100644 --- 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 index 02ac60f7d9..7522af0255 100644 --- 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 != "" {