From 06eb05570a7213f15f34533ce189e2ac520326bd Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Tue, 14 May 2019 16:22:39 +0100 Subject: [PATCH 1/8] fix a few typos Signed-off-by: Ian Campbell (cherry picked from commit d84e278aac67414ff70356b3b926675648dea664) Signed-off-by: Silvin Lubecki --- cli/command/cli.go | 4 ++-- cli/command/defaultcontextstore.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 161163e00b..9bd9ebc2af 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -210,9 +210,9 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err) - baseContextSore := store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig) + baseContextStore := store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig) cli.contextStore = &ContextStoreWithDefault{ - Store: baseContextSore, + Store: baseContextStore, Resolver: func() (*DefaultContext, error) { return resolveDefaultContext(opts.Common, cli.ConfigFile(), cli.Err()) }, diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index 9da76df75d..6a55b79086 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -20,7 +20,7 @@ const ( DefaultContextName = "default" ) -// DefaultContext contains the default context data for all enpoints +// DefaultContext contains the default context data for all endpoints type DefaultContext struct { Meta store.Metadata TLS store.ContextTLSData From 71e1883ca03e01a96affd8e711ccd4765c271121 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 13:19:30 +0100 Subject: [PATCH 2/8] e2e: add a test for `context ls` I'm about to refactor the code which includes the Kubernetes support in a way which relies on something vendoring `./cli/context/kubernetes/` in order to trigger the inclusion of support for the Kubernetes endpoint in the final binary. In practice anything which is interested in Kubernetes must import that package (e.g. `./cli/command/context.list` does for the `EndpointFromContext` function). However if it was somehow possible to build without that import then the `KUBERNETES ENDPOINT` column would be mysteriously empty. Out of an abundance of caution add a specific check on the final binary. Signed-off-by: Ian Campbell (cherry picked from commit d5d693aa6e3f5c7ae99cdc78e7164bb3916a6f14) Signed-off-by: Silvin Lubecki --- e2e/context/context_test.go | 21 +++++++++++++++++++ e2e/context/main_test.go | 17 +++++++++++++++ e2e/context/testdata/context-ls.golden | 3 +++ .../testdata/test-dockerconfig/config.json | 7 +++++++ .../meta.json | 1 + e2e/context/testdata/test-kubeconfig | 20 ++++++++++++++++++ 6 files changed, 69 insertions(+) create mode 100644 e2e/context/context_test.go create mode 100644 e2e/context/main_test.go create mode 100644 e2e/context/testdata/context-ls.golden create mode 100644 e2e/context/testdata/test-dockerconfig/config.json create mode 100644 e2e/context/testdata/test-dockerconfig/contexts/meta/b71199ebd070b36beab7317920c2c2f1d777df8d05e5527d8458fda57cb17a7a/meta.json create mode 100644 e2e/context/testdata/test-kubeconfig diff --git a/e2e/context/context_test.go b/e2e/context/context_test.go new file mode 100644 index 0000000000..b427217116 --- /dev/null +++ b/e2e/context/context_test.go @@ -0,0 +1,21 @@ +package context + +import ( + "testing" + + "gotest.tools/golden" + "gotest.tools/icmd" +) + +func TestContextList(t *testing.T) { + cmd := icmd.Command("docker", "context", "ls") + cmd.Env = append(cmd.Env, + "DOCKER_CONFIG=./testdata/test-dockerconfig", + "KUBECONFIG=./testdata/test-kubeconfig", + ) + result := icmd.RunCmd(cmd).Assert(t, icmd.Expected{ + Err: icmd.None, + ExitCode: 0, + }) + golden.Assert(t, result.Stdout(), "context-ls.golden") +} diff --git a/e2e/context/main_test.go b/e2e/context/main_test.go new file mode 100644 index 0000000000..a3b1c53016 --- /dev/null +++ b/e2e/context/main_test.go @@ -0,0 +1,17 @@ +package context + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/cli/internal/test/environment" +) + +func TestMain(m *testing.M) { + if err := environment.Setup(); err != nil { + fmt.Println(err.Error()) + os.Exit(3) + } + os.Exit(m.Run()) +} diff --git a/e2e/context/testdata/context-ls.golden b/e2e/context/testdata/context-ls.golden new file mode 100644 index 0000000000..2e2569ce10 --- /dev/null +++ b/e2e/context/testdata/context-ls.golden @@ -0,0 +1,3 @@ +NAME DESCRIPTION DOCKER ENDPOINT KUBERNETES ENDPOINT ORCHESTRATOR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock https://someserver (zoinx) swarm +remote my remote cluster ssh://someserver https://someserver (default) kubernetes diff --git a/e2e/context/testdata/test-dockerconfig/config.json b/e2e/context/testdata/test-dockerconfig/config.json new file mode 100644 index 0000000000..832b5c10b6 --- /dev/null +++ b/e2e/context/testdata/test-dockerconfig/config.json @@ -0,0 +1,7 @@ +{ + "auths": {}, + "HttpHeaders": { + "User-Agent": "Docker-Client/19.09.0-dev (linux)" + }, + "credsStore": "secretservice" +} \ No newline at end of file diff --git a/e2e/context/testdata/test-dockerconfig/contexts/meta/b71199ebd070b36beab7317920c2c2f1d777df8d05e5527d8458fda57cb17a7a/meta.json b/e2e/context/testdata/test-dockerconfig/contexts/meta/b71199ebd070b36beab7317920c2c2f1d777df8d05e5527d8458fda57cb17a7a/meta.json new file mode 100644 index 0000000000..11c9b4126e --- /dev/null +++ b/e2e/context/testdata/test-dockerconfig/contexts/meta/b71199ebd070b36beab7317920c2c2f1d777df8d05e5527d8458fda57cb17a7a/meta.json @@ -0,0 +1 @@ +{"Name":"remote","Metadata":{"Description":"my remote cluster","StackOrchestrator":"kubernetes"},"Endpoints":{"docker":{"Host":"ssh://someserver","SkipTLSVerify":false},"kubernetes":{"Host":"https://someserver","SkipTLSVerify":false,"DefaultNamespace":"default","Exec":{"command":"heptio-authenticator-aws","args":["token","-i","eks-cf"],"env":null,"apiVersion":"client.authentication.k8s.io/v1alpha1"}}}} diff --git a/e2e/context/testdata/test-kubeconfig b/e2e/context/testdata/test-kubeconfig new file mode 100644 index 0000000000..e96df74a50 --- /dev/null +++ b/e2e/context/testdata/test-kubeconfig @@ -0,0 +1,20 @@ +apiVersion: v1 +clusters: +- cluster: + certificate-authority-data: dGhlLWNh + server: https://someserver + name: test-cluster +contexts: +- context: + cluster: test-cluster + user: test-user + namespace: zoinx + name: test +current-context: test +kind: Config +preferences: {} +users: +- name: test-user + user: + client-certificate-data: dGhlLWNlcnQ= + client-key-data: dGhlLWtleQ== From a4f41d94dbd94056e3375ae971ad95ebd6386289 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 14:10:57 +0100 Subject: [PATCH 3/8] Support dynamic registration of context store endpoint types This is a yet unused and the default set remains the same, no expected functional change. Signed-off-by: Ian Campbell (cherry picked from commit 087c3f7d08e00934555e34ee4424ea33fa2672c9) Signed-off-by: Silvin Lubecki --- cli/command/cli.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 9bd9ebc2af..776688691f 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -526,10 +526,22 @@ func resolveContextName(opts *cliflags.CommonOptions, config *configfile.ConfigF return DefaultContextName, nil } +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 +// metadata type with the default context store config, so that +// endpoint will be supported by stores using the config returned by +// DefaultContextStoreConfig. +func RegisterDefaultStoreEndpoints(ep ...store.NamedTypeGetter) { + defaultStoreEndpoints = append(defaultStoreEndpoints, ep...) +} + func defaultContextStoreConfig() store.Config { return store.NewConfig( func() interface{} { return &DockerContext{} }, - store.EndpointTypeGetter(docker.DockerEndpoint, func() interface{} { return &docker.EndpointMeta{} }), - store.EndpointTypeGetter(kubecontext.KubernetesEndpoint, func() interface{} { return &kubecontext.EndpointMeta{} }), + defaultStoreEndpoints..., ) } From a7c10adf4e522dabccf55d240a28a189cd287a63 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 14:11:50 +0100 Subject: [PATCH 4/8] Add a helper to iterate over all endpoint types in a context store Unused for now. Signed-off-by: Ian Campbell (cherry picked from commit 4f14c4995ea5d96d91caeb03bb2e700572c97660) Signed-off-by: Silvin Lubecki --- cli/context/store/storeconfig.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cli/context/store/storeconfig.go b/cli/context/store/storeconfig.go index b282a9d10e..2a4bc57c1c 100644 --- a/cli/context/store/storeconfig.go +++ b/cli/context/store/storeconfig.go @@ -30,6 +30,16 @@ func (c Config) SetEndpoint(name string, getter TypeGetter) { c.endpointTypes[name] = getter } +// ForeachEndpointType calls cb on every endpoint type registered with the Config +func (c Config) ForeachEndpointType(cb func(string, TypeGetter) error) error { + for n, ep := range c.endpointTypes { + if err := cb(n, ep); err != nil { + return err + } + } + return nil +} + // NewConfig creates a config object func NewConfig(contextType TypeGetter, endpoints ...NamedTypeGetter) Config { res := Config{ From d4226d2f738bc7cd170e5ec465ee1bc41782aa8f Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 14:47:07 +0100 Subject: [PATCH 5/8] Allow dynamically registered context endpoint to provide their defaults. Previously an endpoint registered using `RegisterDefaultStoreEndpoints` would not be taken into consideration by `resolveDefaultContext` and so could not provide any details. Resolve this by passing a `store.Config` to `resolveDefaultContext` and using it to iterate over all registered endpoints. Any endpoint can ensure that their type implements the new `EndpointDefaultResolver` in order to provide a default. The Docker and Kubernetes endpoints are special cased, shortly the Kubernetes one will be refactored to be dynamically registered. Signed-off-by: Ian Campbell (cherry picked from commit 1433e274204f6bd239f59d14eaf103dfa859ad8c) Signed-off-by: Silvin Lubecki --- cli/command/cli.go | 7 +++--- cli/command/defaultcontextstore.go | 31 ++++++++++++++++++++++++- cli/command/defaultcontextstore_test.go | 2 +- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 776688691f..5abc54e0fc 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -214,7 +214,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize cli.contextStore = &ContextStoreWithDefault{ Store: baseContextStore, Resolver: func() (*DefaultContext, error) { - return resolveDefaultContext(opts.Common, cli.ConfigFile(), cli.Err()) + return resolveDefaultContext(opts.Common, cli.ConfigFile(), cli.contextStoreConfig, cli.Err()) }, } cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore) @@ -259,10 +259,11 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize // NewAPIClientFromFlags creates a new APIClient from command line flags func NewAPIClientFromFlags(opts *cliflags.CommonOptions, configFile *configfile.ConfigFile) (client.APIClient, error) { + storeConfig := defaultContextStoreConfig() store := &ContextStoreWithDefault{ - Store: store.New(cliconfig.ContextStoreDir(), defaultContextStoreConfig()), + Store: store.New(cliconfig.ContextStoreDir(), storeConfig), Resolver: func() (*DefaultContext, error) { - return resolveDefaultContext(opts, configFile, ioutil.Discard) + return resolveDefaultContext(opts, configFile, storeConfig, ioutil.Discard) }, } contextName, err := resolveContextName(opts, configFile, store) diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index 6a55b79086..1489990947 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -35,8 +35,16 @@ type ContextStoreWithDefault struct { Resolver DefaultContextResolver } +// EndpointDefaultResolver is implemented by any EndpointMeta object +// which wants to be able to populate the store with whatever their default is. +type EndpointDefaultResolver interface { + // ResolveDefault returns values suitable for storing in store.Metadata.Endpoints + // and store.ContextTLSData.Endpoints. If there is no default then returns nil, nil. + ResolveDefault() (interface{}, *store.EndpointTLSData) +} + // resolveDefaultContext creates a Metadata for the current CLI invocation parameters -func resolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.ConfigFile, stderr io.Writer) (*DefaultContext, error) { +func resolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.ConfigFile, storeconfig store.Config, stderr io.Writer) (*DefaultContext, error) { stackOrchestrator, err := GetStackOrchestrator("", "", config.StackOrchestrator, stderr) if err != nil { return nil, err @@ -78,6 +86,27 @@ func resolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.Conf } } + if err := storeconfig.ForeachEndpointType(func(n string, get store.TypeGetter) error { + if n == docker.DockerEndpoint || n == kubernetes.KubernetesEndpoint { // handled above + return nil + } + ep := get() + if i, ok := ep.(EndpointDefaultResolver); ok { + meta, tls := i.ResolveDefault() + if meta == nil { + return nil + } + contextMetadata.Endpoints[n] = meta + if tls != nil { + contextTLSData.Endpoints[n] = *tls + } + } + // Nothing to be done + return nil + }); err != nil { + return nil, err + } + return &DefaultContext{Meta: contextMetadata, TLS: contextTLSData}, nil } diff --git a/cli/command/defaultcontextstore_test.go b/cli/command/defaultcontextstore_test.go index 7acdfd3dbc..493d0f4fe5 100644 --- a/cli/command/defaultcontextstore_test.go +++ b/cli/command/defaultcontextstore_test.go @@ -72,7 +72,7 @@ func TestDefaultContextInitializer(t *testing.T) { TLSOptions: &tlsconfig.Options{ CAFile: "./testdata/ca.pem", }, - }, cli.ConfigFile(), cli.Err()) + }, 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) From 5e413159e5681ed89ba73392d8ac913e48bf286c Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 14:52:37 +0100 Subject: [PATCH 6/8] Export `DefaultContextStoreConfig()` and `ResolveDefaultContext()` These are needed by any dynamically registered (via `RegisterDefaultStoreEndpoints`) endpoint type to write a useful/sensible unit test. Signed-off-by: Ian Campbell (cherry picked from commit f820766f6ac57188d96c9ca377f2b4627e90da28) Signed-off-by: Silvin Lubecki --- cli/command/cli.go | 11 ++++++----- cli/command/defaultcontextstore.go | 4 ++-- cli/command/defaultcontextstore_test.go | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 5abc54e0fc..8c0e690fab 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -214,7 +214,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize cli.contextStore = &ContextStoreWithDefault{ Store: baseContextStore, Resolver: func() (*DefaultContext, error) { - return resolveDefaultContext(opts.Common, cli.ConfigFile(), cli.contextStoreConfig, cli.Err()) + return ResolveDefaultContext(opts.Common, cli.ConfigFile(), cli.contextStoreConfig, cli.Err()) }, } cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore) @@ -259,11 +259,11 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize // NewAPIClientFromFlags creates a new APIClient from command line flags func NewAPIClientFromFlags(opts *cliflags.CommonOptions, configFile *configfile.ConfigFile) (client.APIClient, error) { - storeConfig := defaultContextStoreConfig() + storeConfig := DefaultContextStoreConfig() store := &ContextStoreWithDefault{ Store: store.New(cliconfig.ContextStoreDir(), storeConfig), Resolver: func() (*DefaultContext, error) { - return resolveDefaultContext(opts, configFile, storeConfig, ioutil.Discard) + return ResolveDefaultContext(opts, configFile, storeConfig, ioutil.Discard) }, } contextName, err := resolveContextName(opts, configFile, store) @@ -454,7 +454,7 @@ func NewDockerCli(ops ...DockerCliOption) (*DockerCli, error) { WithContentTrustFromEnv(), WithContainerizedClient(containerizedengine.NewClient), } - cli.contextStoreConfig = defaultContextStoreConfig() + cli.contextStoreConfig = DefaultContextStoreConfig() ops = append(defaultOps, ops...) if err := cli.Apply(ops...); err != nil { return nil, err @@ -540,7 +540,8 @@ func RegisterDefaultStoreEndpoints(ep ...store.NamedTypeGetter) { defaultStoreEndpoints = append(defaultStoreEndpoints, ep...) } -func defaultContextStoreConfig() store.Config { +// DefaultContextStoreConfig returns a new store.Config with the default set of endpoints configured. +func DefaultContextStoreConfig() store.Config { return store.NewConfig( func() interface{} { return &DockerContext{} }, defaultStoreEndpoints..., diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index 1489990947..f270b06c2f 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -43,8 +43,8 @@ type EndpointDefaultResolver interface { ResolveDefault() (interface{}, *store.EndpointTLSData) } -// resolveDefaultContext creates a Metadata for the current CLI invocation parameters -func resolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.ConfigFile, storeconfig store.Config, stderr io.Writer) (*DefaultContext, error) { +// ResolveDefaultContext creates a Metadata for the current CLI invocation parameters +func ResolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.ConfigFile, storeconfig store.Config, stderr io.Writer) (*DefaultContext, error) { stackOrchestrator, err := GetStackOrchestrator("", "", config.StackOrchestrator, stderr) if err != nil { return nil, err diff --git a/cli/command/defaultcontextstore_test.go b/cli/command/defaultcontextstore_test.go index 493d0f4fe5..d50c9a1d07 100644 --- a/cli/command/defaultcontextstore_test.go +++ b/cli/command/defaultcontextstore_test.go @@ -67,12 +67,12 @@ func TestDefaultContextInitializer(t *testing.T) { cli.configFile = &configfile.ConfigFile{ StackOrchestrator: "all", } - ctx, err := resolveDefaultContext(&cliflags.CommonOptions{ + ctx, err := ResolveDefaultContext(&cliflags.CommonOptions{ TLS: true, TLSOptions: &tlsconfig.Options{ CAFile: "./testdata/ca.pem", }, - }, cli.ConfigFile(), defaultContextStoreConfig(), cli.Err()) + }, 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) From ec7a9ad6e4772d71982248f56fbb7e7ce89bec23 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Thu, 16 May 2019 15:06:27 +0100 Subject: [PATCH 7/8] 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 (cherry picked from commit 520be05c494940dfd70f1c43765c52735570f212) Signed-off-by: Silvin Lubecki --- 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 8c0e690fab..186854f866 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 4f48ca4ef2..607cd220a3 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 f270b06c2f..6187cdd89e 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 d50c9a1d07..e1c7c1df9b 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 c218d94dee..94044d8138 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 0000000000..203f5dbb9a --- /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 From a720cf572f9673064f42d27933408970aa6bcb61 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 17 May 2019 15:48:49 +0100 Subject: [PATCH 8/8] Push check for kubernetes requirement down into the endpoint This is less of a layering violation and removes some ugly hardcoded `"kubernetes"` strings which were needed to avoid an import loop. Signed-off-by: Ian Campbell (cherry picked from commit c455193d143327d2bc4b0eaee55b0e5d086300ff) Signed-off-by: Silvin Lubecki --- cli/command/defaultcontextstore.go | 26 +++++++++++--------------- cli/context/kubernetes/load.go | 14 +++++++++++--- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/cli/command/defaultcontextstore.go b/cli/command/defaultcontextstore.go index 6187cdd89e..3140dc5030 100644 --- a/cli/command/defaultcontextstore.go +++ b/cli/command/defaultcontextstore.go @@ -35,8 +35,13 @@ type ContextStoreWithDefault struct { // which wants to be able to populate the store with whatever their default is. type EndpointDefaultResolver interface { // ResolveDefault returns values suitable for storing in store.Metadata.Endpoints - // and store.ContextTLSData.Endpoints. If there is no default then returns nil, nil. - ResolveDefault() (interface{}, *store.EndpointTLSData) + // and store.ContextTLSData.Endpoints. + // + // An error is only returned for something fatal, not simply + // the lack of a default (e.g. because the config file which + // would contain it is missing). If there is no default then + // returns nil, nil, nil. + ResolveDefault(Orchestrator) (interface{}, *store.EndpointTLSData, error) } // ResolveDefaultContext creates a Metadata for the current CLI invocation parameters @@ -66,22 +71,17 @@ func ResolveDefaultContext(opts *cliflags.CommonOptions, config *configfile.Conf contextTLSData.Endpoints[docker.DockerEndpoint] = *dockerEP.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 { // handled above return nil } ep := get() if i, ok := ep.(EndpointDefaultResolver); ok { - meta, tls := i.ResolveDefault() + meta, tls, err := i.ResolveDefault(stackOrchestrator) + if err != nil { + return err + } 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 @@ -95,10 +95,6 @@ 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/context/kubernetes/load.go b/cli/context/kubernetes/load.go index 94044d8138..113ec1ad78 100644 --- a/cli/context/kubernetes/load.go +++ b/cli/context/kubernetes/load.go @@ -9,6 +9,7 @@ import ( "github.com/docker/cli/cli/context/store" api "github.com/docker/compose-on-kubernetes/api" "github.com/docker/docker/pkg/homedir" + "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -76,21 +77,28 @@ func (c *Endpoint) KubernetesConfig() clientcmd.ClientConfig { // ResolveDefault returns endpoint metadata for the default Kubernetes // endpoint, which is derived from the env-based kubeconfig. -func (c *EndpointMeta) ResolveDefault() (interface{}, *store.EndpointTLSData) { +func (c *EndpointMeta) ResolveDefault(stackOrchestrator command.Orchestrator) (interface{}, *store.EndpointTLSData, error) { kubeconfig := os.Getenv("KUBECONFIG") if kubeconfig == "" { kubeconfig = filepath.Join(homedir.Get(), ".kube/config") } kubeEP, err := FromKubeConfig(kubeconfig, "", "") if err != nil { - return nil, nil + if stackOrchestrator == command.OrchestratorKubernetes || stackOrchestrator == command.OrchestratorAll { + return nil, nil, errors.Wrapf(err, "default orchestrator is %s but unable to resolve kubernetes endpoint", stackOrchestrator) + } + + // We deliberately quash the error here, returning nil + // for the first argument is sufficient to indicate we weren't able to + // provide a default + return nil, nil, nil } var tls *store.EndpointTLSData if kubeEP.TLSData != nil { tls = kubeEP.TLSData.ToStoreTLSData() } - return kubeEP.EndpointMeta, tls + return kubeEP.EndpointMeta, tls, nil } // EndpointFromContext extracts kubernetes endpoint info from current context