From e2b9f5bc570da20de7f10ff9433c4e87278ca094 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 29 May 2020 17:07:48 +0200 Subject: [PATCH] Move login to root command, delegate to classic login when only one arg and not dot in it --- cli/cmd/context/context.go | 3 -- cli/cmd/context/inspect.go | 3 +- cli/cmd/context/login/login.go | 36 ----------------------- cli/cmd/login/login.go | 52 ++++++++++++++++++++++++++++++++++ cli/dockerclassic/exec.go | 10 ++++++- cli/main.go | 10 ++++++- cli/main_test.go | 15 ++++++++++ tests/e2e/e2e_test.go | 18 ++++++++++++ 8 files changed, 104 insertions(+), 43 deletions(-) delete mode 100644 cli/cmd/context/login/login.go create mode 100644 cli/cmd/login/login.go diff --git a/cli/cmd/context/context.go b/cli/cmd/context/context.go index ef524d12f..a36420997 100644 --- a/cli/cmd/context/context.go +++ b/cli/cmd/context/context.go @@ -29,8 +29,6 @@ package context import ( "github.com/spf13/cobra" - - "github.com/docker/api/cli/cmd/context/login" ) // Command manages contexts @@ -46,7 +44,6 @@ func Command() *cobra.Command { removeCommand(), showCommand(), useCommand(), - login.Command(), inspectCommand(), ) diff --git a/cli/cmd/context/inspect.go b/cli/cmd/context/inspect.go index 1b4efa783..1c60fd41b 100644 --- a/cli/cmd/context/inspect.go +++ b/cli/cmd/context/inspect.go @@ -39,8 +39,7 @@ func inspectCommand() *cobra.Command { Short: "Display detailed information on one or more contexts", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - dockerclassic.Exec(cmd.Context()) - return nil + return dockerclassic.ExecCmd(cmd) }, } return cmd diff --git a/cli/cmd/context/login/login.go b/cli/cmd/context/login/login.go deleted file mode 100644 index 9e10c667a..000000000 --- a/cli/cmd/context/login/login.go +++ /dev/null @@ -1,36 +0,0 @@ -package login - -import ( - "github.com/pkg/errors" - "github.com/spf13/cobra" - - "github.com/docker/api/client" -) - -// Command returns the compose command with its child commands -func Command() *cobra.Command { - command := &cobra.Command{ - Short: "Cloud login for docker contexts", - Use: "login", - } - command.AddCommand( - azureLoginCommand(), - ) - return command -} - -func azureLoginCommand() *cobra.Command { - azureLoginCmd := &cobra.Command{ - Use: "azure", - RunE: func(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - cs, err := client.GetCloudService(ctx, "aci") - if err != nil { - return errors.Wrap(err, "cannot connect to backend") - } - return cs.Login(ctx, nil) - }, - } - - return azureLoginCmd -} diff --git a/cli/cmd/login/login.go b/cli/cmd/login/login.go new file mode 100644 index 000000000..a8d796cc6 --- /dev/null +++ b/cli/cmd/login/login.go @@ -0,0 +1,52 @@ +package login + +import ( + "strings" + + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "github.com/docker/api/cli/dockerclassic" + "github.com/docker/api/client" +) + +// Command returns the login command +func Command() *cobra.Command { + cmd := &cobra.Command{ + Use: "login [OPTIONS] [SERVER] | login azure", + Short: "Log in to a Docker registry", + Long: "Log in to a Docker registry or cloud backend.\nIf no registry server is specified, the default is defined by the daemon.", + Args: cobra.MaximumNArgs(1), + RunE: runLogin, + } + // define flags for backward compatibility with docker-classic + flags := cmd.Flags() + flags.StringP("username", "u", "", "Username") + flags.StringP("password", "p", "", "Password") + flags.BoolP("password-stdin", "", false, "Take the password from stdin") + + return cmd +} + +func runLogin(cmd *cobra.Command, args []string) error { + if len(args) == 1 && !strings.Contains(args[0], ".") { + backend := args[0] + switch backend { + case "azure": + return cloudLogin(cmd, "aci") + default: + return errors.New("Unknown backend type for cloud login : " + backend) + } + } + return dockerclassic.ExecCmd(cmd) +} + +func cloudLogin(cmd *cobra.Command, backendType string) error { + ctx := cmd.Context() + cs, err := client.GetCloudService(ctx, backendType) + if err != nil { + return errors.Wrap(err, "cannot connect to backend") + } + return cs.Login(ctx, nil) + +} diff --git a/cli/dockerclassic/exec.go b/cli/dockerclassic/exec.go index ab2eec7a4..a2d127960 100644 --- a/cli/dockerclassic/exec.go +++ b/cli/dockerclassic/exec.go @@ -6,11 +6,13 @@ import ( "os" "os/exec" + "github.com/spf13/cobra" + apicontext "github.com/docker/api/context" "github.com/docker/api/context/store" ) -// Exec will delegate the cli command to docker-classic +// Exec delegates to docker-classic func Exec(ctx context.Context) { currentContext := apicontext.CurrentContext(ctx) s := store.ContextStore(ctx) @@ -34,3 +36,9 @@ func Exec(ctx context.Context) { os.Exit(0) } } + +// ExecCmd delegates the cli command to docker-classic. 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()) + return nil +} diff --git a/cli/main.go b/cli/main.go index 7684448dd..586b4bae4 100644 --- a/cli/main.go +++ b/cli/main.go @@ -37,6 +37,8 @@ import ( "syscall" "time" + "github.com/docker/api/cli/cmd/login" + "github.com/docker/api/cli/dockerclassic" "github.com/pkg/errors" @@ -60,6 +62,11 @@ import ( var ( runningOwnCommand bool + ownCommands = map[string]struct{}{ + "context": {}, + "login": {}, + "serve": {}, + } ) func init() { @@ -80,7 +87,7 @@ func isOwnCommand(cmd *cobra.Command) bool { if cmd == nil { return false } - if cmd.Name() == "context" || cmd.Name() == "serve" { + if _, ok := ownCommands[cmd.Name()]; ok { return true } return isOwnCommand(cmd.Parent()) @@ -114,6 +121,7 @@ func main() { cmd.LogsCommand(), cmd.RmCommand(), compose.Command(), + login.Command(), ) helpFunc := root.HelpFunc() diff --git a/cli/main_test.go b/cli/main_test.go index 76638e205..b0448b4b2 100644 --- a/cli/main_test.go +++ b/cli/main_test.go @@ -33,6 +33,11 @@ import ( "path/filepath" "testing" + "github.com/docker/api/cli/cmd" + "github.com/docker/api/cli/cmd/context" + "github.com/docker/api/cli/cmd/login" + "github.com/docker/api/cli/cmd/run" + "github.com/stretchr/testify/require" "github.com/docker/api/cli/config" @@ -70,3 +75,13 @@ func TestDetermineCurrentContext(t *testing.T) { require.NoError(t, err) require.Equal(t, "other-context", c) } + +func TestCheckOwnCommand(t *testing.T) { + require.True(t, isOwnCommand(login.Command())) + require.True(t, isOwnCommand(context.Command())) + require.True(t, isOwnCommand(cmd.ServeCommand())) + require.False(t, isOwnCommand(run.Command())) + require.False(t, isOwnCommand(cmd.ExecCommand())) + require.False(t, isOwnCommand(cmd.LogsCommand())) + require.False(t, isOwnCommand(cmd.PsCommand())) +} diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index fe4355c85..c7fe4ed3a 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -76,6 +76,24 @@ func (s *E2eSuite) TestContextLegacy() { }) } +func (s *E2eSuite) TestClassicLoginWithparameters() { + 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")) + Expect(err).NotTo(BeNil()) +} + +func (s *E2eSuite) TestCloudLogin() { + output, err := s.NewDockerCommand("login", "mycloudbackend").Exec() + Expect(output).To(ContainSubstring("Unknown backend type for cloud login : mycloudbackend")) + Expect(err).NotTo(BeNil()) +} + func (s *E2eSuite) TestSetupError() { It("should display an error if cannot shell out to docker-classic", func() { err := os.Setenv("PATH", s.BinDir)