diff --git a/cli/command/stack/common.go b/cli/command/stack/common.go new file mode 100644 index 0000000000..6de410da84 --- /dev/null +++ b/cli/command/stack/common.go @@ -0,0 +1,31 @@ +package stack + +import ( + "fmt" + "strings" + "unicode" +) + +// validateStackName checks if the provided string is a valid stack name (namespace). +// It currently only does a rudimentary check if the string is empty, or consists +// of only whitespace and quoting characters. +func validateStackName(namespace string) error { + v := strings.TrimFunc(namespace, quotesOrWhitespace) + if v == "" { + return fmt.Errorf("invalid stack name: %q", namespace) + } + return nil +} + +func validateStackNames(namespaces []string) error { + for _, ns := range namespaces { + if err := validateStackName(ns); err != nil { + return err + } + } + return nil +} + +func quotesOrWhitespace(r rune) bool { + return unicode.IsSpace(r) || r == '"' || r == '\'' +} diff --git a/cli/command/stack/deploy.go b/cli/command/stack/deploy.go index 5ca032d758..0165bbda1a 100644 --- a/cli/command/stack/deploy.go +++ b/cli/command/stack/deploy.go @@ -1,12 +1,18 @@ package stack import ( + "context" + "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/stack/kubernetes" + "github.com/docker/cli/cli/command/stack/loader" "github.com/docker/cli/cli/command/stack/options" "github.com/docker/cli/cli/command/stack/swarm" + composetypes "github.com/docker/cli/cli/compose/types" + "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) func newDeployCommand(dockerCli command.Cli, common *commonOptions) *cobra.Command { @@ -19,20 +25,32 @@ func newDeployCommand(dockerCli command.Cli, common *commonOptions) *cobra.Comma Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] - switch { - case common == nil: // Top level deploy commad - return swarm.RunDeploy(dockerCli, opts) - case common.orchestrator.HasAll(): - return errUnsupportedAllOrchestrator - case common.orchestrator.HasKubernetes(): - kli, err := kubernetes.WrapCli(dockerCli, kubernetes.NewOptions(cmd.Flags(), common.orchestrator)) - if err != nil { - return err - } - return kubernetes.RunDeploy(kli, opts) - default: - return swarm.RunDeploy(dockerCli, opts) + if err := validateStackName(opts.Namespace); err != nil { + return err } + + commonOrchestrator := command.OrchestratorSwarm // default for top-level deploy command + if common != nil { + commonOrchestrator = common.orchestrator + } + + switch { + case opts.Bundlefile == "" && len(opts.Composefiles) == 0: + return errors.Errorf("Please specify either a bundle file (with --bundle-file) or a Compose file (with --compose-file).") + case opts.Bundlefile != "" && len(opts.Composefiles) != 0: + return errors.Errorf("You cannot specify both a bundle file and a Compose file.") + case opts.Bundlefile != "": + if commonOrchestrator != command.OrchestratorSwarm { + return errors.Errorf("bundle files are not supported on another orchestrator than swarm.") + } + return swarm.DeployBundle(context.Background(), dockerCli, opts) + } + + config, err := loader.LoadComposefile(dockerCli, opts) + if err != nil { + return err + } + return RunDeploy(dockerCli, cmd.Flags(), config, commonOrchestrator, opts) }, } @@ -54,3 +72,19 @@ func newDeployCommand(dockerCli command.Cli, common *commonOptions) *cobra.Comma kubernetes.AddNamespaceFlag(flags) return cmd } + +// RunDeploy performs a stack deploy against the specified orchestrator +func RunDeploy(dockerCli command.Cli, flags *pflag.FlagSet, config *composetypes.Config, commonOrchestrator command.Orchestrator, opts options.Deploy) error { + switch { + case commonOrchestrator.HasAll(): + return errUnsupportedAllOrchestrator + case commonOrchestrator.HasKubernetes(): + kli, err := kubernetes.WrapCli(dockerCli, kubernetes.NewOptions(flags, commonOrchestrator)) + if err != nil { + return err + } + return kubernetes.RunDeploy(kli, opts, config) + default: + return swarm.RunDeploy(dockerCli, opts, config) + } +} diff --git a/cli/command/stack/deploy_test.go b/cli/command/stack/deploy_test.go new file mode 100644 index 0000000000..89dbc6e1b7 --- /dev/null +++ b/cli/command/stack/deploy_test.go @@ -0,0 +1,17 @@ +package stack + +import ( + "io/ioutil" + "testing" + + "github.com/docker/cli/internal/test" + "gotest.tools/assert" +) + +func TestDeployWithEmptyName(t *testing.T) { + cmd := newDeployCommand(test.NewFakeCli(&fakeClient{}), nil) + cmd.SetArgs([]string{"' '"}) + cmd.SetOutput(ioutil.Discard) + + assert.ErrorContains(t, cmd.Execute(), `invalid stack name: "' '"`) +} diff --git a/cli/command/stack/kubernetes/deploy.go b/cli/command/stack/kubernetes/deploy.go index 877cb1ef89..501e0a240b 100644 --- a/cli/command/stack/kubernetes/deploy.go +++ b/cli/command/stack/kubernetes/deploy.go @@ -5,14 +5,14 @@ import ( "io" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/command/stack/loader" "github.com/docker/cli/cli/command/stack/options" + composetypes "github.com/docker/cli/cli/compose/types" "github.com/morikuni/aec" "github.com/pkg/errors" ) // RunDeploy is the kubernetes implementation of docker stack deploy -func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { +func RunDeploy(dockerCli *KubeCli, opts options.Deploy, cfg *composetypes.Config) error { cmdOut := dockerCli.Out() // Check arguments if len(opts.Composefiles) == 0 { @@ -29,11 +29,6 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error { return err } - // Parse the compose file - cfg, err := loader.LoadComposefile(dockerCli, opts) - if err != nil { - return err - } stack, err := stacks.FromCompose(dockerCli.Err(), opts.Namespace, cfg) if err != nil { return err diff --git a/cli/command/stack/ps.go b/cli/command/stack/ps.go index c1c18d204a..4d6215351b 100644 --- a/cli/command/stack/ps.go +++ b/cli/command/stack/ps.go @@ -19,6 +19,10 @@ func newPsCommand(dockerCli command.Cli, common *commonOptions) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] + if err := validateStackName(opts.Namespace); err != nil { + return err + } + switch { case common.orchestrator.HasAll(): return errUnsupportedAllOrchestrator diff --git a/cli/command/stack/ps_test.go b/cli/command/stack/ps_test.go index c192a7a5ab..39dd3c85fc 100644 --- a/cli/command/stack/ps_test.go +++ b/cli/command/stack/ps_test.go @@ -51,6 +51,14 @@ func TestStackPsErrors(t *testing.T) { } } +func TestRunPSWithEmptyName(t *testing.T) { + cmd := newPsCommand(test.NewFakeCli(&fakeClient{}), &orchestrator) + cmd.SetArgs([]string{"' '"}) + cmd.SetOutput(ioutil.Discard) + + assert.ErrorContains(t, cmd.Execute(), `invalid stack name: "' '"`) +} + func TestStackPsEmptyStack(t *testing.T) { fakeCli := test.NewFakeCli(&fakeClient{ taskListFunc: func(options types.TaskListOptions) ([]swarm.Task, error) { diff --git a/cli/command/stack/remove.go b/cli/command/stack/remove.go index 8738e6eafa..737713e48a 100644 --- a/cli/command/stack/remove.go +++ b/cli/command/stack/remove.go @@ -19,6 +19,10 @@ func newRemoveCommand(dockerCli command.Cli, common *commonOptions) *cobra.Comma Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespaces = args + if err := validateStackNames(opts.Namespaces); err != nil { + return err + } + switch { case common.orchestrator.HasAll(): return errUnsupportedAllOrchestrator diff --git a/cli/command/stack/remove_test.go b/cli/command/stack/remove_test.go index 61080fbf3d..c7032d845f 100644 --- a/cli/command/stack/remove_test.go +++ b/cli/command/stack/remove_test.go @@ -41,6 +41,14 @@ func fakeClientForRemoveStackTest(version string) *fakeClient { } } +func TestRemoveWithEmptyName(t *testing.T) { + cmd := newRemoveCommand(test.NewFakeCli(&fakeClient{}), &orchestrator) + cmd.SetArgs([]string{"good", "' '", "alsogood"}) + cmd.SetOutput(ioutil.Discard) + + assert.ErrorContains(t, cmd.Execute(), `invalid stack name: "' '"`) +} + func TestRemoveStackVersion124DoesNotRemoveConfigsOrSecrets(t *testing.T) { client := fakeClientForRemoveStackTest("1.24") cmd := newRemoveCommand(test.NewFakeCli(client), &orchestrator) diff --git a/cli/command/stack/services.go b/cli/command/stack/services.go index 0379409b31..ca6d42c231 100644 --- a/cli/command/stack/services.go +++ b/cli/command/stack/services.go @@ -19,6 +19,10 @@ func newServicesCommand(dockerCli command.Cli, common *commonOptions) *cobra.Com Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Namespace = args[0] + if err := validateStackName(opts.Namespace); err != nil { + return err + } + switch { case common.orchestrator.HasAll(): return errUnsupportedAllOrchestrator diff --git a/cli/command/stack/services_test.go b/cli/command/stack/services_test.go index 0bc8839b64..64a58b9970 100644 --- a/cli/command/stack/services_test.go +++ b/cli/command/stack/services_test.go @@ -80,6 +80,14 @@ func TestStackServicesErrors(t *testing.T) { } } +func TestRunServicesWithEmptyName(t *testing.T) { + cmd := newServicesCommand(test.NewFakeCli(&fakeClient{}), &orchestrator) + cmd.SetArgs([]string{"' '"}) + cmd.SetOutput(ioutil.Discard) + + assert.ErrorContains(t, cmd.Execute(), `invalid stack name: "' '"`) +} + func TestStackServicesEmptyServiceList(t *testing.T) { fakeCli := test.NewFakeCli(&fakeClient{ serviceListFunc: func(options types.ServiceListOptions) ([]swarm.Service, error) { diff --git a/cli/command/stack/swarm/common.go b/cli/command/stack/swarm/common.go index 62ff0d9ff5..b4193df360 100644 --- a/cli/command/stack/swarm/common.go +++ b/cli/command/stack/swarm/common.go @@ -2,9 +2,6 @@ package swarm import ( "context" - "fmt" - "strings" - "unicode" "github.com/docker/cli/cli/compose/convert" "github.com/docker/cli/opts" @@ -51,28 +48,3 @@ func getStackSecrets(ctx context.Context, apiclient client.APIClient, namespace func getStackConfigs(ctx context.Context, apiclient client.APIClient, namespace string) ([]swarm.Config, error) { return apiclient.ConfigList(ctx, types.ConfigListOptions{Filters: getStackFilter(namespace)}) } - -// validateStackName checks if the provided string is a valid stack name (namespace). -// -// It currently only does a rudimentary check if the string is empty, or consists -// of only whitespace and quoting characters. -func validateStackName(namespace string) error { - v := strings.TrimFunc(namespace, quotesOrWhitespace) - if len(v) == 0 { - return fmt.Errorf("invalid stack name: %q", namespace) - } - return nil -} - -func validateStackNames(namespaces []string) error { - for _, ns := range namespaces { - if err := validateStackName(ns); err != nil { - return err - } - } - return nil -} - -func quotesOrWhitespace(r rune) bool { - return unicode.IsSpace(r) || r == '"' || r == '\'' -} diff --git a/cli/command/stack/swarm/deploy.go b/cli/command/stack/swarm/deploy.go index 0504e3381b..d11c328f3f 100644 --- a/cli/command/stack/swarm/deploy.go +++ b/cli/command/stack/swarm/deploy.go @@ -7,6 +7,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/stack/options" "github.com/docker/cli/cli/compose/convert" + composetypes "github.com/docker/cli/cli/compose/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/api/types/versions" "github.com/pkg/errors" @@ -21,26 +22,14 @@ const ( ) // RunDeploy is the swarm implementation of docker stack deploy -func RunDeploy(dockerCli command.Cli, opts options.Deploy) error { +func RunDeploy(dockerCli command.Cli, opts options.Deploy, cfg *composetypes.Config) error { ctx := context.Background() - if err := validateStackName(opts.Namespace); err != nil { - return err - } if err := validateResolveImageFlag(dockerCli, &opts); err != nil { return err } - switch { - case opts.Bundlefile == "" && len(opts.Composefiles) == 0: - return errors.Errorf("Please specify either a bundle file (with --bundle-file) or a Compose file (with --compose-file).") - case opts.Bundlefile != "" && len(opts.Composefiles) != 0: - return errors.Errorf("You cannot specify both a bundle file and a Compose file.") - case opts.Bundlefile != "": - return deployBundle(ctx, dockerCli, opts) - default: - return deployCompose(ctx, dockerCli, opts) - } + return deployCompose(ctx, dockerCli, opts, cfg) } // validateResolveImageFlag validates the opts.resolveImage command line option diff --git a/cli/command/stack/swarm/deploy_bundlefile.go b/cli/command/stack/swarm/deploy_bundlefile.go index 7b59126baf..8db6f66b0e 100644 --- a/cli/command/stack/swarm/deploy_bundlefile.go +++ b/cli/command/stack/swarm/deploy_bundlefile.go @@ -15,10 +15,8 @@ import ( "github.com/pkg/errors" ) -func deployBundle(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error { - if err := validateStackName(opts.Namespace); err != nil { - return err - } +// DeployBundle deploy a bundlefile (dab) on a swarm. +func DeployBundle(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error { bundle, err := loadBundlefile(dockerCli.Err(), opts.Namespace, opts.Bundlefile) if err != nil { return err diff --git a/cli/command/stack/swarm/deploy_composefile.go b/cli/command/stack/swarm/deploy_composefile.go index a0b8b127d6..e4574fd987 100644 --- a/cli/command/stack/swarm/deploy_composefile.go +++ b/cli/command/stack/swarm/deploy_composefile.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/command/stack/loader" "github.com/docker/cli/cli/command/stack/options" "github.com/docker/cli/cli/compose/convert" composetypes "github.com/docker/cli/cli/compose/types" @@ -17,15 +16,7 @@ import ( "github.com/pkg/errors" ) -func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error { - if err := validateStackName(opts.Namespace); err != nil { - return err - } - config, err := loader.LoadComposefile(dockerCli, opts) - if err != nil { - return err - } - +func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy, config *composetypes.Config) error { if err := checkDaemonIsSwarmManager(ctx, dockerCli); err != nil { return err } diff --git a/cli/command/stack/swarm/deploy_test.go b/cli/command/stack/swarm/deploy_test.go index 64aafec382..b1df60dcba 100644 --- a/cli/command/stack/swarm/deploy_test.go +++ b/cli/command/stack/swarm/deploy_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/docker/cli/cli/command/stack/options" "github.com/docker/cli/cli/compose/convert" "github.com/docker/cli/internal/test" "github.com/docker/docker/api/types" @@ -27,15 +26,6 @@ func TestPruneServices(t *testing.T) { assert.Check(t, is.DeepEqual(buildObjectIDs([]string{objectName("foo", "remove")}), client.removedServices)) } -func TestDeployWithEmptyName(t *testing.T) { - ctx := context.Background() - client := &fakeClient{} - dockerCli := test.NewFakeCli(client) - - err := deployCompose(ctx, dockerCli, options.Deploy{Namespace: "' '", Prune: true}) - assert.Check(t, is.Error(err, `invalid stack name: "' '"`)) -} - // TestServiceUpdateResolveImageChanged tests that the service's // image digest, and "ForceUpdate" is preserved if the image did not change in // the compose file diff --git a/cli/command/stack/swarm/ps.go b/cli/command/stack/swarm/ps.go index 79a154a8f2..5b28a39e73 100644 --- a/cli/command/stack/swarm/ps.go +++ b/cli/command/stack/swarm/ps.go @@ -13,10 +13,6 @@ import ( // RunPS is the swarm implementation of docker stack ps func RunPS(dockerCli command.Cli, opts options.PS) error { - if err := validateStackName(opts.Namespace); err != nil { - return err - } - filter := getStackFilterFromOpt(opts.Namespace, opts.Filter) ctx := context.Background() diff --git a/cli/command/stack/swarm/ps_test.go b/cli/command/stack/swarm/ps_test.go deleted file mode 100644 index 33401da655..0000000000 --- a/cli/command/stack/swarm/ps_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package swarm - -import ( - "testing" - - "github.com/docker/cli/cli/command/stack/options" - "github.com/docker/cli/internal/test" - "gotest.tools/assert" - is "gotest.tools/assert/cmp" -) - -func TestRunPSWithEmptyName(t *testing.T) { - client := &fakeClient{} - dockerCli := test.NewFakeCli(client) - - err := RunPS(dockerCli, options.PS{Namespace: "' '"}) - assert.Check(t, is.Error(err, `invalid stack name: "' '"`)) -} diff --git a/cli/command/stack/swarm/remove.go b/cli/command/stack/swarm/remove.go index e75597d065..4dedef12f3 100644 --- a/cli/command/stack/swarm/remove.go +++ b/cli/command/stack/swarm/remove.go @@ -16,10 +16,6 @@ import ( // RunRemove is the swarm implementation of docker stack remove func RunRemove(dockerCli command.Cli, opts options.Remove) error { - if err := validateStackNames(opts.Namespaces); err != nil { - return err - } - client := dockerCli.Client() ctx := context.Background() diff --git a/cli/command/stack/swarm/remove_test.go b/cli/command/stack/swarm/remove_test.go deleted file mode 100644 index 3d30b288ac..0000000000 --- a/cli/command/stack/swarm/remove_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package swarm - -import ( - "testing" - - "github.com/docker/cli/cli/command/stack/options" - "github.com/docker/cli/internal/test" - "gotest.tools/assert" - is "gotest.tools/assert/cmp" -) - -func TestRunRemoveWithEmptyName(t *testing.T) { - client := &fakeClient{} - dockerCli := test.NewFakeCli(client) - - err := RunRemove(dockerCli, options.Remove{Namespaces: []string{"good", "' '", "alsogood"}}) - assert.Check(t, is.Error(err, `invalid stack name: "' '"`)) -} diff --git a/cli/command/stack/swarm/services.go b/cli/command/stack/swarm/services.go index 0225918678..07b990adc8 100644 --- a/cli/command/stack/swarm/services.go +++ b/cli/command/stack/swarm/services.go @@ -14,9 +14,6 @@ import ( // RunServices is the swarm implementation of docker stack services func RunServices(dockerCli command.Cli, opts options.Services) error { - if err := validateStackName(opts.Namespace); err != nil { - return err - } ctx := context.Background() client := dockerCli.Client() diff --git a/cli/command/stack/swarm/services_test.go b/cli/command/stack/swarm/services_test.go deleted file mode 100644 index 9e3ca47f3a..0000000000 --- a/cli/command/stack/swarm/services_test.go +++ /dev/null @@ -1,18 +0,0 @@ -package swarm - -import ( - "testing" - - "github.com/docker/cli/cli/command/stack/options" - "github.com/docker/cli/internal/test" - "gotest.tools/assert" - is "gotest.tools/assert/cmp" -) - -func TestRunServicesWithEmptyName(t *testing.T) { - client := &fakeClient{} - dockerCli := test.NewFakeCli(client) - - err := RunServices(dockerCli, options.Services{Namespace: "' '"}) - assert.Check(t, is.Error(err, `invalid stack name: "' '"`)) -}