Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set current namespace from kubeconfig by default #172

Merged
merged 4 commits into from
Jun 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Contributor Author

@nak3 nak3 Jun 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this test case because it is almost same with TestGetNamespaceDefault. (Please let me know if I should not remove it.)

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 @@ -35,7 +35,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