From 520be05c494940dfd70f1c43765c52735570f212 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 15:06:27 +0100 Subject: [PATCH] Dynamically register kubernetes context store endpoint type. This removes the need for the core context code to import `github.com/docker/cli/cli/context/kubernetes` which in turn reduces the transitive import tree in this file to not pull in all of Kubernetes. Note that this means that any calling code which is interested in the kubernetes endpoint must import `github.com/docker/cli/cli/context/kubernetes` itself somewhere in order to trigger the dynamic registration. In practice anything which is interested in Kubernetes must import that package (e.g. `./cli/command/context.list` does for the `EndpointFromContext` function) to do anything useful, so this restriction is not too onerous. As a special case a small amount of Kubernetes related logic remains in `ResolveDefaultContext` to handle error handling when the stack orchestrator includes Kubernetes. In order to avoid a circular import loop this hardcodes the kube endpoint name. Similarly to avoid an import loop the existing `TestDefaultContextInitializer` cannot continue to unit test for the Kubernetes case, so that aspect of the test is carved off into a very similar test in the kubernetes context package. Lastly, note that the kubernetes endpoint is now modifiable via `WithContextEndpointType`. Signed-off-by: Ian Campbell --- cli/command/cli.go | 2 -- cli/command/cli_options.go | 3 +- cli/command/defaultcontextstore.go | 32 +++++++------------ cli/command/defaultcontextstore_test.go | 7 ++-- cli/context/kubernetes/load.go | 32 +++++++++++++++++++ cli/context/kubernetes/load_test.go | 25 +++++++++++++++ .../kubernetes}/testdata/test-kubeconfig | 0 7 files changed, 72 insertions(+), 29 deletions(-) create mode 100644 cli/context/kubernetes/load_test.go rename cli/{command => context/kubernetes}/testdata/test-kubeconfig (100%) diff --git a/cli/command/cli.go b/cli/command/cli.go index 8c0e690fa..186854f86 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -14,7 +14,6 @@ import ( "github.com/docker/cli/cli/config/configfile" dcontext "github.com/docker/cli/cli/context" "github.com/docker/cli/cli/context/docker" - kubecontext "github.com/docker/cli/cli/context/kubernetes" "github.com/docker/cli/cli/context/store" "github.com/docker/cli/cli/debug" cliflags "github.com/docker/cli/cli/flags" @@ -529,7 +528,6 @@ func resolveContextName(opts *cliflags.CommonOptions, config *configfile.ConfigF var defaultStoreEndpoints = []store.NamedTypeGetter{ store.EndpointTypeGetter(docker.DockerEndpoint, func() interface{} { return &docker.EndpointMeta{} }), - store.EndpointTypeGetter(kubecontext.KubernetesEndpoint, func() interface{} { return &kubecontext.EndpointMeta{} }), } // RegisterDefaultStoreEndpoints registers a new named endpoint diff --git a/cli/command/cli_options.go b/cli/command/cli_options.go index 4f48ca4ef..607cd220a 100644 --- a/cli/command/cli_options.go +++ b/cli/command/cli_options.go @@ -7,7 +7,6 @@ import ( "strconv" "github.com/docker/cli/cli/context/docker" - "github.com/docker/cli/cli/context/kubernetes" "github.com/docker/cli/cli/context/store" "github.com/docker/cli/cli/streams" clitypes "github.com/docker/cli/types" @@ -97,7 +96,7 @@ func WithContainerizedClient(containerizedFn func(string) (clitypes.Containerize func WithContextEndpointType(endpointName string, endpointType store.TypeGetter) DockerCliOption { return func(cli *DockerCli) error { switch endpointName { - case docker.DockerEndpoint, kubernetes.KubernetesEndpoint: + case docker.DockerEndpoint: return fmt.Errorf("cannot change %q endpoint type", endpointName) } cli.contextStoreConfig.SetEndpoint(endpointName, endpointType) diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index f270b06c2..6187cdd89 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -3,15 +3,11 @@ package command import ( "fmt" "io" - "os" - "path/filepath" "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/context/docker" - "github.com/docker/cli/cli/context/kubernetes" "github.com/docker/cli/cli/context/store" cliflags "github.com/docker/cli/cli/flags" - "github.com/docker/docker/pkg/homedir" "github.com/pkg/errors" ) @@ -70,30 +66,22 @@ func ResolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.Conf contextTLSData.Endpoints[docker.DockerEndpoint] = *dockerEP.TLSData.ToStoreTLSData() } - // Default context uses env-based kubeconfig for Kubernetes endpoint configuration - kubeconfig := os.Getenv("KUBECONFIG") - if kubeconfig == "" { - kubeconfig = filepath.Join(homedir.Get(), ".kube/config") - } - kubeEP, err := kubernetes.FromKubeConfig(kubeconfig, "", "") - if (stackOrchestrator == OrchestratorKubernetes || stackOrchestrator == OrchestratorAll) && err != nil { - return nil, errors.Wrapf(err, "default orchestrator is %s but kubernetes endpoint could not be found", stackOrchestrator) - } - if err == nil { - contextMetadata.Endpoints[kubernetes.KubernetesEndpoint] = kubeEP.EndpointMeta - if kubeEP.TLSData != nil { - contextTLSData.Endpoints[kubernetes.KubernetesEndpoint] = *kubeEP.TLSData.ToStoreTLSData() - } - } + // We open code the string "kubernetes" below because we + // cannot import KubernetesEndpoint from the corresponding + // package due to import loops. + wantKubernetesEP := stackOrchestrator == OrchestratorKubernetes || stackOrchestrator == OrchestratorAll if err := storeconfig.ForeachEndpointType(func(n string, get store.TypeGetter) error { - if n == docker.DockerEndpoint || n == kubernetes.KubernetesEndpoint { // handled above + if n == docker.DockerEndpoint { // handled above return nil } ep := get() if i, ok := ep.(EndpointDefaultResolver); ok { meta, tls := i.ResolveDefault() if meta == nil { + if wantKubernetesEP && n == "kubernetes" { + return errors.Errorf("default orchestrator is %s but unable to resolve kubernetes endpoint", stackOrchestrator) + } return nil } contextMetadata.Endpoints[n] = meta @@ -107,6 +95,10 @@ func ResolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.Conf return nil, err } + if _, ok := contextMetadata.Endpoints["kubernetes"]; wantKubernetesEP && !ok { + return nil, errors.Errorf("default orchestrator is %s but kubernetes endpoint could not be found", stackOrchestrator) + } + return &DefaultContext{Meta: contextMetadata, TLS: contextTLSData}, nil } diff --git a/cli/command/defaultcontextstore_test.go b/cli/command/defaultcontextstore_test.go index d50c9a1d0..e1c7c1df9 100644 --- a/cli/command/defaultcontextstore_test.go +++ b/cli/command/defaultcontextstore_test.go @@ -8,7 +8,6 @@ import ( "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/context/docker" - "github.com/docker/cli/cli/context/kubernetes" "github.com/docker/cli/cli/context/store" cliflags "github.com/docker/cli/cli/flags" "github.com/docker/go-connections/tlsconfig" @@ -63,9 +62,8 @@ func TestDefaultContextInitializer(t *testing.T) { cli, err := NewDockerCli() assert.NilError(t, err) defer env.Patch(t, "DOCKER_HOST", "ssh://someswarmserver")() - defer env.Patch(t, "KUBECONFIG", "./testdata/test-kubeconfig")() cli.configFile = &configfile.ConfigFile{ - StackOrchestrator: "all", + StackOrchestrator: "swarm", } ctx, err := ResolveDefaultContext(&cliflags.CommonOptions{ TLS: true, @@ -75,10 +73,9 @@ func TestDefaultContextInitializer(t *testing.T) { }, cli.ConfigFile(), DefaultContextStoreConfig(), cli.Err()) assert.NilError(t, err) assert.Equal(t, "default", ctx.Meta.Name) - assert.Equal(t, OrchestratorAll, ctx.Meta.Metadata.(DockerContext).StackOrchestrator) + assert.Equal(t, OrchestratorSwarm, ctx.Meta.Metadata.(DockerContext).StackOrchestrator) assert.DeepEqual(t, "ssh://someswarmserver", ctx.Meta.Endpoints[docker.DockerEndpoint].(docker.EndpointMeta).Host) golden.Assert(t, string(ctx.TLS.Endpoints[docker.DockerEndpoint].Files["ca.pem"]), "ca.pem") - assert.DeepEqual(t, "zoinx", ctx.Meta.Endpoints[kubernetes.KubernetesEndpoint].(kubernetes.EndpointMeta).DefaultNamespace) } func TestExportDefaultImport(t *testing.T) { diff --git a/cli/context/kubernetes/load.go b/cli/context/kubernetes/load.go index c218d94de..94044d813 100644 --- a/cli/context/kubernetes/load.go +++ b/cli/context/kubernetes/load.go @@ -1,9 +1,14 @@ package kubernetes import ( + "os" + "path/filepath" + + "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/context" "github.com/docker/cli/cli/context/store" api "github.com/docker/compose-on-kubernetes/api" + "github.com/docker/docker/pkg/homedir" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -17,6 +22,8 @@ type EndpointMeta struct { Exec *clientcmdapi.ExecConfig `json:",omitempty"` } +var _ command.EndpointDefaultResolver = &EndpointMeta{} + // Endpoint is a typed wrapper around a context-store generic endpoint describing // a Kubernetes endpoint, with TLS data type Endpoint struct { @@ -24,6 +31,12 @@ type Endpoint struct { TLSData *context.TLSData } +func init() { + command.RegisterDefaultStoreEndpoints( + store.EndpointTypeGetter(KubernetesEndpoint, func() interface{} { return &EndpointMeta{} }), + ) +} + // WithTLSData loads TLS materials for the endpoint func (c *EndpointMeta) WithTLSData(s store.Reader, contextName string) (Endpoint, error) { tlsData, err := context.LoadTLSData(s, contextName, KubernetesEndpoint) @@ -61,6 +74,25 @@ func (c *Endpoint) KubernetesConfig() clientcmd.ClientConfig { return clientcmd.NewDefaultClientConfig(*cfg, &clientcmd.ConfigOverrides{}) } +// ResolveDefault returns endpoint metadata for the default Kubernetes +// endpoint, which is derived from the env-based kubeconfig. +func (c *EndpointMeta) ResolveDefault() (interface{}, *store.EndpointTLSData) { + kubeconfig := os.Getenv("KUBECONFIG") + if kubeconfig == "" { + kubeconfig = filepath.Join(homedir.Get(), ".kube/config") + } + kubeEP, err := FromKubeConfig(kubeconfig, "", "") + if err != nil { + return nil, nil + } + + var tls *store.EndpointTLSData + if kubeEP.TLSData != nil { + tls = kubeEP.TLSData.ToStoreTLSData() + } + return kubeEP.EndpointMeta, tls +} + // EndpointFromContext extracts kubernetes endpoint info from current context func EndpointFromContext(metadata store.Metadata) *EndpointMeta { ep, ok := metadata.Endpoints[KubernetesEndpoint] diff --git a/cli/context/kubernetes/load_test.go b/cli/context/kubernetes/load_test.go new file mode 100644 index 000000000..203f5dbb9 --- /dev/null +++ b/cli/context/kubernetes/load_test.go @@ -0,0 +1,25 @@ +package kubernetes + +import ( + "testing" + + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/config/configfile" + cliflags "github.com/docker/cli/cli/flags" + "gotest.tools/assert" + "gotest.tools/env" +) + +func TestDefaultContextInitializer(t *testing.T) { + cli, err := command.NewDockerCli() + assert.NilError(t, err) + defer env.Patch(t, "KUBECONFIG", "./testdata/test-kubeconfig")() + configFile := &configfile.ConfigFile{ + StackOrchestrator: "all", + } + ctx, err := command.ResolveDefaultContext(&cliflags.CommonOptions{}, configFile, command.DefaultContextStoreConfig(), cli.Err()) + assert.NilError(t, err) + assert.Equal(t, "default", ctx.Meta.Name) + assert.Equal(t, command.OrchestratorAll, ctx.Meta.Metadata.(command.DockerContext).StackOrchestrator) + assert.DeepEqual(t, "zoinx", ctx.Meta.Endpoints[KubernetesEndpoint].(EndpointMeta).DefaultNamespace) +} diff --git a/cli/command/testdata/test-kubeconfig b/cli/context/kubernetes/testdata/test-kubeconfig similarity index 100% rename from cli/command/testdata/test-kubeconfig rename to cli/context/kubernetes/testdata/test-kubeconfig