Skip to content

Commit

Permalink
Set current namespace from kubeconfig by default (knative#172)
Browse files Browse the repository at this point in the history
* 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 knative#7

* Use NamespaceFactory to get current namespace

* Update unit tests

* Add nil check for ClientConfig
  • Loading branch information
nak3 authored and maximilien committed Jul 1, 2019
1 parent 375ecbd commit 8a12c72
Show file tree
Hide file tree
Showing 14 changed files with 65 additions and 73 deletions.
4 changes: 0 additions & 4 deletions cmd/kn/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 6 additions & 5 deletions pkg/kn/commands/namespaced.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
33 changes: 12 additions & 21 deletions pkg/kn/commands/namespaced_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/revision/revision_describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/revision/revision_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kn/commands/service/service_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service_describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/service_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/kn/commands/test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)

Expand Down
40 changes: 32 additions & 8 deletions pkg/kn/commands/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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{})
}
25 changes: 1 addition & 24 deletions pkg/kn/core/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
Expand All @@ -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 != "" {
Expand Down

0 comments on commit 8a12c72

Please sign in to comment.