From 1bb2909d1a8a2a3988373c91c949e6d1c6baf7f9 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 17 Jun 2020 22:20:23 +0200 Subject: [PATCH 1/4] Do not restrict `docker inspect` to 1 arg, can be zero (inspect current context), one or several. Especially, inspect with zero param is already used by VSCode. Cf https://github.com/docker/desktop-microsoft/issues/19 --- cli/cmd/context/inspect.go | 4 +++- tests/e2e/e2e_test.go | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cli/cmd/context/inspect.go b/cli/cmd/context/inspect.go index 5ce3cdc4a..0a7be9c0d 100644 --- a/cli/cmd/context/inspect.go +++ b/cli/cmd/context/inspect.go @@ -37,10 +37,12 @@ func inspectCommand() *cobra.Command { cmd := &cobra.Command{ Use: "inspect", Short: "Display detailed information on one or more contexts", - Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return mobycli.ExecCmd(cmd) }, } + // flags matching delegated command in moby cli + flags := cmd.Flags() + flags.StringP("format", "f", "", "Format the output using the given Go template") return cmd } diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 62f223b78..2e56bf72a 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -79,6 +79,11 @@ func (s *E2eSuite) TestInspectDefaultContext() { Expect(output).To(ContainSubstring(`"Name": "default"`)) } +func (s *E2eSuite) TestInspectContextNoArgs() { + output := s.NewDockerCommand("context", "inspect").ExecOrDie() + Expect(output).To(ContainSubstring(`"Name": "default"`)) +} + func (s *E2eSuite) TestContextCreateParseErrorDoesNotDelegateToLegacy() { It("should dispay new cli error when parsing context create flags", func() { _, err := s.NewDockerCommand("context", "create", "aci", "--subscription-id", "titi").Exec() From fe57e4c1596193068e9815aecb1b16aed13a3298 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 17 Jun 2020 22:52:49 +0200 Subject: [PATCH 2/4] Fix a bug where calling login, context inspect did not do anything if not on default context. These command will shell out to Moby cli regardless of current context --- cli/mobycli/exec.go | 32 ++++++++++++++++++-------------- tests/e2e/e2e_test.go | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index ce5eed5ab..1fd52fa7d 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -16,7 +16,7 @@ import ( // ComDockerCli name of the classic cli binary const ComDockerCli = "com.docker.cli" -// Exec delegates to com.docker.cli +// Exec delegates to com.docker.cli if on moby context func Exec(ctx context.Context) { currentContext := apicontext.CurrentContext(ctx) s := store.ContextStore(ctx) @@ -25,24 +25,28 @@ func Exec(ctx context.Context) { // Only run original docker command if the current context is not // ours. if err != nil { - cmd := exec.CommandContext(ctx, ComDockerCli, os.Args[1:]...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - if exiterr, ok := err.(*exec.ExitError); ok { - os.Exit(exiterr.ExitCode()) - } - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - os.Exit(0) + shellOut(ctx) } } +func shellOut(ctx context.Context) { + cmd := exec.CommandContext(ctx, ComDockerCli, os.Args[1:]...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Run(); err != nil { + if exiterr, ok := err.(*exec.ExitError); ok { + os.Exit(exiterr.ExitCode()) + } + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) +} + // ExecCmd delegates the cli command to com.docker.cli. The error is never returned (process will exit with docker classic exit code), the return type is to make it easier to use with cobra commands func ExecCmd(command *cobra.Command) error { - Exec(command.Context()) + shellOut(command.Context()) return nil } diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 2e56bf72a..0c1412c3c 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -84,6 +84,13 @@ func (s *E2eSuite) TestInspectContextNoArgs() { Expect(output).To(ContainSubstring(`"Name": "default"`)) } +func (s *E2eSuite) TestInspectContextRegardlessCurrentContext() { + s.NewDockerCommand("context", "create", "local", "localCtx").ExecOrDie() + s.NewDockerCommand("context", "use", "localCtx").ExecOrDie() + output := s.NewDockerCommand("context", "inspect").ExecOrDie() + Expect(output).To(ContainSubstring(`"Name": "localCtx"`)) +} + func (s *E2eSuite) TestContextCreateParseErrorDoesNotDelegateToLegacy() { It("should dispay new cli error when parsing context create flags", func() { _, err := s.NewDockerCommand("context", "create", "aci", "--subscription-id", "titi").Exec() @@ -113,6 +120,14 @@ func (s *E2eSuite) TestClassicLoginWithparameters() { Expect(err).NotTo(BeNil()) } +func (s *E2eSuite) TestClassicLoginRegardlessCurrentContext() { + s.NewDockerCommand("context", "create", "local", "localCtx").ExecOrDie() + s.NewDockerCommand("context", "use", "localCtx").ExecOrDie() + output, err := s.NewDockerCommand("login", "-u", "nouser", "-p", "wrongpasword").Exec() + Expect(output).To(ContainSubstring("Get https://registry-1.docker.io/v2/: unauthorized: incorrect username or password")) + Expect(err).NotTo(BeNil()) +} + func (s *E2eSuite) TestClassicLogin() { output, err := s.NewDockerCommand("login", "someregistry.docker.io").Exec() Expect(output).To(ContainSubstring("Cannot perform an interactive login from a non TTY device")) From 39812447016dfe49db14aef7e8a542b78a9c1440 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 17 Jun 2020 23:12:50 +0200 Subject: [PATCH 3/4] =?UTF-8?q?Fix=20bug=20where=20we=20shell=20out=20to?= =?UTF-8?q?=20Moby=20cli=20only=20if=20context=20=3D=3D=20default.=20We=20?= =?UTF-8?q?must=20shell=20out=20to=20Moby=20cli=20for=20any=20context=20of?= =?UTF-8?q?=20type=20=E2=80=9CMoby=E2=80=9D,=20not=20only=20the=20default?= =?UTF-8?q?=20context.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cli/mobycli/exec.go | 5 +++-- context/store/contextmetadata.go | 2 +- context/store/store.go | 23 +++++++++++------------ context/store/storedefault.go | 4 +--- tests/e2e/e2e_test.go | 7 +++++++ 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index 1fd52fa7d..95631edb0 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -19,12 +19,13 @@ const ComDockerCli = "com.docker.cli" // Exec delegates to com.docker.cli if on moby context func Exec(ctx context.Context) { currentContext := apicontext.CurrentContext(ctx) + s := store.ContextStore(ctx) - _, err := s.Get(currentContext) + currentCtx, err := s.Get(currentContext) // Only run original docker command if the current context is not // ours. - if err != nil { + if err != nil || currentCtx.Type() == store.DefaultContextType { shellOut(ctx) } } diff --git a/context/store/contextmetadata.go b/context/store/contextmetadata.go index a9987dfd8..38aabaa2e 100644 --- a/context/store/contextmetadata.go +++ b/context/store/contextmetadata.go @@ -12,7 +12,7 @@ type DockerContext struct { // Type the context type func (m *DockerContext) Type() string { if m.Metadata.Type == "" { - return defaultContextType + return DefaultContextType } return m.Metadata.Type } diff --git a/context/store/store.go b/context/store/store.go index 25c604ddd..75baa0761 100644 --- a/context/store/store.go +++ b/context/store/store.go @@ -45,6 +45,17 @@ import ( const ( // DefaultContextName is an automatically generated local context DefaultContextName = "default" + // DefaultContextType is the type for all moby contexts (not associated with cli backend) + DefaultContextType = "moby" + // AciContextType is the endpoint key in the context endpoints for an ACI + // backend + AciContextType = "aci" + // LocalContextType is the endpoint key in the context endpoints for a new + // local backend + LocalContextType = "local" + // ExampleContextType is the endpoint key in the context endpoints for an + // example backend + ExampleContextType = "example" ) const ( @@ -92,18 +103,6 @@ type Endpoint struct { DefaultNamespace string `json:",omitempty"` } -const ( - // AciContextType is the endpoint key in the context endpoints for an ACI - // backend - AciContextType = "aci" - // LocalContextType is the endpoint key in the context endpoints for a new - // local backend - LocalContextType = "local" - // ExampleContextType is the endpoint key in the context endpoints for an - // example backend - ExampleContextType = "example" -) - type store struct { root string } diff --git a/context/store/storedefault.go b/context/store/storedefault.go index d81c5fd85..1d1a1e8f6 100644 --- a/context/store/storedefault.go +++ b/context/store/storedefault.go @@ -8,8 +8,6 @@ import ( "github.com/pkg/errors" ) -const defaultContextType = "moby" - // Represents a context as created by the docker cli type defaultContext struct { Metadata ContextMetadata @@ -67,7 +65,7 @@ func dockerDefaultContext() (*DockerContext, error) { }, }, Metadata: ContextMetadata{ - Type: defaultContextType, + Type: DefaultContextType, Description: "Current DOCKER_HOST based configuration", StackOrchestrator: defaultCtx.Metadata.StackOrchestrator, }, diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 0c1412c3c..cc6129867 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -169,6 +169,13 @@ func (s *E2eSuite) TestLegacy() { output := s.NewDockerCommand("run", "--rm", "hello-world").WithTimeout(time.NewTimer(20 * time.Second).C).ExecOrDie() Expect(output).To(ContainSubstring("Hello from Docker!")) }) + + It("should execute legacy commands in other moby contexts", func() { + s.NewDockerCommand("context", "create", "mobyCtx", "--from=default").ExecOrDie() + s.NewDockerCommand("context", "use", "mobyCtx").ExecOrDie() + output, _ := s.NewDockerCommand("swarm", "join").Exec() + Expect(output).To(ContainSubstring("\"docker swarm join\" requires exactly 1 argument.")) + }) } func (s *E2eSuite) TestLeaveLegacyErrorMessagesUnchanged() { From ab3cd0fec121e32d1caea9d07d35169207239b30 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 18 Jun 2020 09:29:01 +0200 Subject: [PATCH 4/4] Renamed ExecIfDefaultCtxType for more explicit behaviour --- cli/main.go | 6 +++--- cli/mobycli/exec.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/main.go b/cli/main.go index d44a8aebc..a8dd0be2f 100644 --- a/cli/main.go +++ b/cli/main.go @@ -101,7 +101,7 @@ func main() { SilenceUsage: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if !isOwnCommand(cmd) { - mobycli.Exec(cmd.Context()) + mobycli.ExecIfDefaultCtxType(cmd.Context()) } return nil }, @@ -127,7 +127,7 @@ func main() { helpFunc := root.HelpFunc() root.SetHelpFunc(func(cmd *cobra.Command, args []string) { if !isOwnCommand(cmd) { - mobycli.Exec(cmd.Context()) + mobycli.ExecIfDefaultCtxType(cmd.Context()) } helpFunc(cmd, args) }) @@ -168,7 +168,7 @@ func main() { fmt.Fprintln(os.Stderr, err) os.Exit(1) } - mobycli.Exec(ctx) + mobycli.ExecIfDefaultCtxType(ctx) checkIfUnknownCommandExistInDefaultContext(err, currentContext) fmt.Fprintln(os.Stderr, err) diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index 95631edb0..9fc0d0348 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -16,8 +16,8 @@ import ( // ComDockerCli name of the classic cli binary const ComDockerCli = "com.docker.cli" -// Exec delegates to com.docker.cli if on moby context -func Exec(ctx context.Context) { +// ExecIfDefaultCtxType delegates to com.docker.cli if on moby context +func ExecIfDefaultCtxType(ctx context.Context) { currentContext := apicontext.CurrentContext(ctx) s := store.ContextStore(ctx)