From 4daf98b73535ff0e4cfc411d41434e901c94224d Mon Sep 17 00:00:00 2001 From: Mathieu Champlon Date: Tue, 15 May 2018 16:56:45 +0200 Subject: [PATCH 1/6] Imply all Kubernetes namespaces for docker stack list when orchestrator is all or Kubernetes * Add "kubernetes" struct in config file with "allNamespaces" option, to opt-out this behavior when set as "disabled" Signed-off-by: Mathieu Champlon Upstream-commit: 1c1300bef652847875daa8fdaf6279117e299af6 Component: cli --- components/cli/cli/command/stack/kubernetes/list.go | 8 ++++++++ components/cli/cli/config/configfile/file.go | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/components/cli/cli/command/stack/kubernetes/list.go b/components/cli/cli/command/stack/kubernetes/list.go index 4a429731a5..f6efd050aa 100644 --- a/components/cli/cli/command/stack/kubernetes/list.go +++ b/components/cli/cli/command/stack/kubernetes/list.go @@ -10,6 +10,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/formatter" "github.com/docker/cli/cli/command/stack/options" + "github.com/docker/cli/cli/config/configfile" "github.com/pkg/errors" core_v1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" @@ -19,11 +20,18 @@ import ( // GetStacks lists the kubernetes stacks func GetStacks(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) { if opts.AllNamespaces || len(opts.Namespaces) == 0 { + if isAllNamespacesDisabled(kubeCli.ConfigFile().Kubernetes) { + opts.AllNamespaces = true + } return getStacksWithAllNamespaces(kubeCli, opts) } return getStacksWithNamespaces(kubeCli, opts, removeDuplicates(opts.Namespaces)) } +func isAllNamespacesDisabled(kubeCliConfig *configfile.KubernetesConfig) bool { + return kubeCliConfig == nil || kubeCliConfig != nil && kubeCliConfig.AllNamespaces != "disabled" +} + func getStacks(kubeCli *KubeCli, opts options.List) ([]*formatter.Stack, error) { composeClient, err := kubeCli.composeClient() if err != nil { diff --git a/components/cli/cli/config/configfile/file.go b/components/cli/cli/config/configfile/file.go index 5653306622..f658385e16 100644 --- a/components/cli/cli/config/configfile/file.go +++ b/components/cli/cli/config/configfile/file.go @@ -46,6 +46,7 @@ type ConfigFile struct { Proxies map[string]ProxyConfig `json:"proxies,omitempty"` Experimental string `json:"experimental,omitempty"` Orchestrator string `json:"orchestrator,omitempty"` + Kubernetes *KubernetesConfig `json:"kubernetes,omitempty"` } // ProxyConfig contains proxy configuration settings @@ -56,6 +57,11 @@ type ProxyConfig struct { FTPProxy string `json:"ftpProxy,omitempty"` } +// KubernetesConfig contains Kubernetes orchestrator settings +type KubernetesConfig struct { + AllNamespaces string `json:"allNamespaces,omitempty"` +} + // New initializes an empty configuration file for the given filename 'fn' func New(fn string) *ConfigFile { return &ConfigFile{ From 108f480da7170a1c3475357c0e2161daf3f9e656 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Thu, 24 May 2018 16:25:01 +0200 Subject: [PATCH 2/6] --orchestrator flag is now a persistent flag Signed-off-by: Silvin Lubecki Upstream-commit: 770daef564c534b9122efd1e6a0c0dfff8573bc1 Component: cli --- components/cli/cli/flags/common.go | 2 -- components/cli/cmd/docker/docker.go | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/components/cli/cli/flags/common.go b/components/cli/cli/flags/common.go index 9c37aed648..46c3d9d68b 100644 --- a/components/cli/cli/flags/common.go +++ b/components/cli/cli/flags/common.go @@ -55,8 +55,6 @@ func (commonOpts *CommonOptions) InstallFlags(flags *pflag.FlagSet) { flags.StringVarP(&commonOpts.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`) flags.BoolVar(&commonOpts.TLS, "tls", dockerTLS, "Use TLS; implied by --tlsverify") flags.BoolVar(&commonOpts.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote") - flags.StringVar(&commonOpts.Orchestrator, "orchestrator", "", "Orchestrator to use (swarm|kubernetes|all) (experimental)") - flags.SetAnnotation("orchestrator", "experimentalCLI", nil) // TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file") diff --git a/components/cli/cmd/docker/docker.go b/components/cli/cmd/docker/docker.go index 68136657d9..c6a2b90933 100644 --- a/components/cli/cmd/docker/docker.go +++ b/components/cli/cmd/docker/docker.go @@ -51,6 +51,11 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { flags.StringVar(&opts.ConfigDir, "config", cliconfig.Dir(), "Location of client config files") opts.Common.InstallFlags(flags) + // Install persistent flags + persistentFlags := cmd.PersistentFlags() + persistentFlags.StringVar(&opts.Common.Orchestrator, "orchestrator", "", "Orchestrator to use (swarm|kubernetes|all) (experimental)") + persistentFlags.SetAnnotation("orchestrator", "experimentalCLI", nil) + setFlagErrorFunc(dockerCli, cmd, flags, opts) setHelpFunc(dockerCli, cmd, flags, opts) From ec563a652e79f0e7a42eaac963281f6062a23d2c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 May 2018 12:26:14 +0200 Subject: [PATCH 3/6] Minor refactor in stack helper functions - Don't wrap the code to be slightly more readable - Rename `getServiceFilter()` to `getStackServiceFilter()` to be consistent with other helper functions. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 0c175fc21ac693350044dd5d3783f4af0b793751 Component: cli --- .../cli/cli/command/stack/swarm/common.go | 42 ++++--------------- .../cli/cli/command/stack/swarm/deploy.go | 2 +- .../command/stack/swarm/deploy_composefile.go | 2 +- .../cli/cli/command/stack/swarm/remove.go | 2 +- 4 files changed, 12 insertions(+), 36 deletions(-) diff --git a/components/cli/cli/command/stack/swarm/common.go b/components/cli/cli/command/stack/swarm/common.go index b90ca9acfa..b4193df360 100644 --- a/components/cli/cli/command/stack/swarm/common.go +++ b/components/cli/cli/command/stack/swarm/common.go @@ -17,7 +17,7 @@ func getStackFilter(namespace string) filters.Args { return filter } -func getServiceFilter(namespace string) filters.Args { +func getStackServiceFilter(namespace string) filters.Args { return getStackFilter(namespace) } @@ -33,42 +33,18 @@ func getAllStacksFilter() filters.Args { return filter } -func getServices( - ctx context.Context, - apiclient client.APIClient, - namespace string, -) ([]swarm.Service, error) { - return apiclient.ServiceList( - ctx, - types.ServiceListOptions{Filters: getServiceFilter(namespace)}) +func getStackServices(ctx context.Context, apiclient client.APIClient, namespace string) ([]swarm.Service, error) { + return apiclient.ServiceList(ctx, types.ServiceListOptions{Filters: getStackServiceFilter(namespace)}) } -func getStackNetworks( - ctx context.Context, - apiclient client.APIClient, - namespace string, -) ([]types.NetworkResource, error) { - return apiclient.NetworkList( - ctx, - types.NetworkListOptions{Filters: getStackFilter(namespace)}) +func getStackNetworks(ctx context.Context, apiclient client.APIClient, namespace string) ([]types.NetworkResource, error) { + return apiclient.NetworkList(ctx, types.NetworkListOptions{Filters: getStackFilter(namespace)}) } -func getStackSecrets( - ctx context.Context, - apiclient client.APIClient, - namespace string, -) ([]swarm.Secret, error) { - return apiclient.SecretList( - ctx, - types.SecretListOptions{Filters: getStackFilter(namespace)}) +func getStackSecrets(ctx context.Context, apiclient client.APIClient, namespace string) ([]swarm.Secret, error) { + return apiclient.SecretList(ctx, types.SecretListOptions{Filters: getStackFilter(namespace)}) } -func getStackConfigs( - ctx context.Context, - apiclient client.APIClient, - namespace string, -) ([]swarm.Config, error) { - return apiclient.ConfigList( - ctx, - types.ConfigListOptions{Filters: getStackFilter(namespace)}) +func getStackConfigs(ctx context.Context, apiclient client.APIClient, namespace string) ([]swarm.Config, error) { + return apiclient.ConfigList(ctx, types.ConfigListOptions{Filters: getStackFilter(namespace)}) } diff --git a/components/cli/cli/command/stack/swarm/deploy.go b/components/cli/cli/command/stack/swarm/deploy.go index e7d0cf482d..3479fd9210 100644 --- a/components/cli/cli/command/stack/swarm/deploy.go +++ b/components/cli/cli/command/stack/swarm/deploy.go @@ -73,7 +73,7 @@ func checkDaemonIsSwarmManager(ctx context.Context, dockerCli command.Cli) error func pruneServices(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, services map[string]struct{}) { client := dockerCli.Client() - oldServices, err := getServices(ctx, client, namespace.Name()) + oldServices, err := getStackServices(ctx, client, namespace.Name()) if err != nil { fmt.Fprintf(dockerCli.Err(), "Failed to list services: %s\n", err) } diff --git a/components/cli/cli/command/stack/swarm/deploy_composefile.go b/components/cli/cli/command/stack/swarm/deploy_composefile.go index 477a4427aa..2c214e000a 100644 --- a/components/cli/cli/command/stack/swarm/deploy_composefile.go +++ b/components/cli/cli/command/stack/swarm/deploy_composefile.go @@ -209,7 +209,7 @@ func deployServices( apiClient := dockerCli.Client() out := dockerCli.Out() - existingServices, err := getServices(ctx, apiClient, namespace.Name()) + existingServices, err := getStackServices(ctx, apiClient, namespace.Name()) if err != nil { return err } diff --git a/components/cli/cli/command/stack/swarm/remove.go b/components/cli/cli/command/stack/swarm/remove.go index 5ae9afa229..c799102eb4 100644 --- a/components/cli/cli/command/stack/swarm/remove.go +++ b/components/cli/cli/command/stack/swarm/remove.go @@ -22,7 +22,7 @@ func RunRemove(dockerCli command.Cli, opts options.Remove) error { var errs []string for _, namespace := range namespaces { - services, err := getServices(ctx, client, namespace) + services, err := getStackServices(ctx, client, namespace) if err != nil { return err } From e1b9b6c0bed73a962b729a4bfcf12b90199de008 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Fri, 18 May 2018 17:54:18 +0200 Subject: [PATCH 4/6] Remove experimentalCli annotation from all kubernetes flags and commands Signed-off-by: Silvin Lubecki Upstream-commit: f2b75a879ad69bb5faf8c1448c0139400004ef16 Component: cli --- components/cli/cli/command/cli.go | 4 ++-- components/cli/cli/command/cli_test.go | 19 ------------------- components/cli/cli/command/orchestrator.go | 6 +----- components/cli/cli/command/stack/cmd.go | 1 - .../cli/cli/command/stack/kubernetes/cli.go | 1 - components/cli/cli/command/stack/list.go | 2 -- components/cli/cli/command/system/version.go | 1 - components/cli/cmd/docker/docker.go | 3 +-- 8 files changed, 4 insertions(+), 33 deletions(-) diff --git a/components/cli/cli/command/cli.go b/components/cli/cli/command/cli.go index 29c7af2129..62a36b20b3 100644 --- a/components/cli/cli/command/cli.go +++ b/components/cli/cli/command/cli.go @@ -166,7 +166,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error { if err != nil { return errors.Wrap(err, "Experimental field") } - orchestrator, err := GetOrchestrator(hasExperimental, opts.Common.Orchestrator, cli.configFile.Orchestrator) + orchestrator, err := GetOrchestrator(opts.Common.Orchestrator, cli.configFile.Orchestrator) if err != nil { return err } @@ -244,7 +244,7 @@ type ClientInfo struct { // HasKubernetes checks if kubernetes orchestrator is enabled func (c ClientInfo) HasKubernetes() bool { - return c.HasExperimental && (c.Orchestrator == OrchestratorKubernetes || c.Orchestrator == OrchestratorAll) + return c.Orchestrator == OrchestratorKubernetes || c.Orchestrator == OrchestratorAll } // HasSwarm checks if swarm orchestrator is enabled diff --git a/components/cli/cli/command/cli_test.go b/components/cli/cli/command/cli_test.go index e5aa0f908d..1418d1b1df 100644 --- a/components/cli/cli/command/cli_test.go +++ b/components/cli/cli/command/cli_test.go @@ -176,28 +176,14 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "default", configfile: `{ - "experimental": "enabled" }`, expectedOrchestrator: "swarm", expectedKubernetes: false, expectedSwarm: true, }, - { - doc: "kubernetesIsExperimental", - configfile: `{ - "experimental": "disabled", - "orchestrator": "kubernetes" - }`, - envOrchestrator: "kubernetes", - flagOrchestrator: "kubernetes", - expectedOrchestrator: "swarm", - expectedKubernetes: false, - expectedSwarm: true, - }, { doc: "kubernetesConfigFile", configfile: `{ - "experimental": "enabled", "orchestrator": "kubernetes" }`, expectedOrchestrator: "kubernetes", @@ -207,7 +193,6 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "kubernetesEnv", configfile: `{ - "experimental": "enabled" }`, envOrchestrator: "kubernetes", expectedOrchestrator: "kubernetes", @@ -217,7 +202,6 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "kubernetesFlag", configfile: `{ - "experimental": "enabled" }`, flagOrchestrator: "kubernetes", expectedOrchestrator: "kubernetes", @@ -227,7 +211,6 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "allOrchestratorFlag", configfile: `{ - "experimental": "enabled" }`, flagOrchestrator: "all", expectedOrchestrator: "all", @@ -237,7 +220,6 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "envOverridesConfigFile", configfile: `{ - "experimental": "enabled", "orchestrator": "kubernetes" }`, envOrchestrator: "swarm", @@ -248,7 +230,6 @@ func TestOrchestratorSwitch(t *testing.T) { { doc: "flagOverridesEnv", configfile: `{ - "experimental": "enabled" }`, envOrchestrator: "kubernetes", flagOrchestrator: "swarm", diff --git a/components/cli/cli/command/orchestrator.go b/components/cli/cli/command/orchestrator.go index b8224edb18..7daad75eef 100644 --- a/components/cli/cli/command/orchestrator.go +++ b/components/cli/cli/command/orchestrator.go @@ -38,11 +38,7 @@ func normalize(value string) (Orchestrator, error) { // GetOrchestrator checks DOCKER_ORCHESTRATOR environment variable and configuration file // orchestrator value and returns user defined Orchestrator. -func GetOrchestrator(isExperimental bool, flagValue, value string) (Orchestrator, error) { - // Non experimental CLI has kubernetes disabled - if !isExperimental { - return defaultOrchestrator, nil - } +func GetOrchestrator(flagValue, value string) (Orchestrator, error) { // Check flag if o, err := normalize(flagValue); o != orchestratorUnset { return o, err diff --git a/components/cli/cli/command/stack/cmd.go b/components/cli/cli/command/stack/cmd.go index dddafc37fe..06cfc8ee84 100644 --- a/components/cli/cli/command/stack/cmd.go +++ b/components/cli/cli/command/stack/cmd.go @@ -33,7 +33,6 @@ func NewStackCommand(dockerCli command.Cli) *cobra.Command { flags := cmd.PersistentFlags() flags.String("kubeconfig", "", "Kubernetes config file") flags.SetAnnotation("kubeconfig", "kubernetes", nil) - flags.SetAnnotation("kubeconfig", "experimentalCLI", nil) return cmd } diff --git a/components/cli/cli/command/stack/kubernetes/cli.go b/components/cli/cli/command/stack/kubernetes/cli.go index 27bc819fa4..832a0f475c 100644 --- a/components/cli/cli/command/stack/kubernetes/cli.go +++ b/components/cli/cli/command/stack/kubernetes/cli.go @@ -42,7 +42,6 @@ func NewOptions(flags *flag.FlagSet) Options { func AddNamespaceFlag(flags *flag.FlagSet) { flags.String("namespace", "", "Kubernetes namespace to use") flags.SetAnnotation("namespace", "kubernetes", nil) - flags.SetAnnotation("namespace", "experimentalCLI", nil) } // WrapCli wraps command.Cli with kubernetes specifics diff --git a/components/cli/cli/command/stack/list.go b/components/cli/cli/command/stack/list.go index 92e6a09980..da9583e935 100644 --- a/components/cli/cli/command/stack/list.go +++ b/components/cli/cli/command/stack/list.go @@ -30,10 +30,8 @@ func newListCommand(dockerCli command.Cli) *cobra.Command { flags.StringVar(&opts.Format, "format", "", "Pretty-print stacks using a Go template") flags.StringSliceVar(&opts.Namespaces, "namespace", []string{}, "Kubernetes namespaces to use") flags.SetAnnotation("namespace", "kubernetes", nil) - flags.SetAnnotation("namespace", "experimentalCLI", nil) flags.BoolVarP(&opts.AllNamespaces, "all-namespaces", "", false, "List stacks from all Kubernetes namespaces") flags.SetAnnotation("all-namespaces", "kubernetes", nil) - flags.SetAnnotation("all-namespaces", "experimentalCLI", nil) return cmd } diff --git a/components/cli/cli/command/system/version.go b/components/cli/cli/command/system/version.go index d50dc847a3..6d1ec243b8 100644 --- a/components/cli/cli/command/system/version.go +++ b/components/cli/cli/command/system/version.go @@ -108,7 +108,6 @@ func NewVersionCommand(dockerCli command.Cli) *cobra.Command { flags.StringVarP(&opts.format, "format", "f", "", "Format the output using the given Go template") flags.StringVarP(&opts.kubeConfig, "kubeconfig", "k", "", "Kubernetes config file") flags.SetAnnotation("kubeconfig", "kubernetes", nil) - flags.SetAnnotation("kubeconfig", "experimentalCLI", nil) return cmd } diff --git a/components/cli/cmd/docker/docker.go b/components/cli/cmd/docker/docker.go index c6a2b90933..f20530dac5 100644 --- a/components/cli/cmd/docker/docker.go +++ b/components/cli/cmd/docker/docker.go @@ -53,8 +53,7 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { // Install persistent flags persistentFlags := cmd.PersistentFlags() - persistentFlags.StringVar(&opts.Common.Orchestrator, "orchestrator", "", "Orchestrator to use (swarm|kubernetes|all) (experimental)") - persistentFlags.SetAnnotation("orchestrator", "experimentalCLI", nil) + persistentFlags.StringVar(&opts.Common.Orchestrator, "orchestrator", "", "Orchestrator to use (swarm|kubernetes|all)") setFlagErrorFunc(dockerCli, cmd, flags, opts) From be0c4cd5b88d12c526f02284fac2b7ca1b840fc0 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 28 May 2018 14:45:08 +0200 Subject: [PATCH 5/6] Check allNamespace config value while loading configuration file Signed-off-by: Silvin Lubecki Upstream-commit: fb34ffc3279320e4aafae20138b42ee89c21dd21 Component: cli --- components/cli/cli/config/configfile/file.go | 17 +++++++- .../cli/cli/config/configfile/file_test.go | 42 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/components/cli/cli/config/configfile/file.go b/components/cli/cli/config/configfile/file.go index f658385e16..383a03228d 100644 --- a/components/cli/cli/config/configfile/file.go +++ b/components/cli/cli/config/configfile/file.go @@ -3,6 +3,7 @@ package configfile import ( "encoding/base64" "encoding/json" + "fmt" "io" "io/ioutil" "os" @@ -125,7 +126,7 @@ func (configFile *ConfigFile) LoadFromReader(configData io.Reader) error { ac.ServerAddress = addr configFile.AuthConfigs[addr] = ac } - return nil + return checkKubernetesConfiguration(configFile.Kubernetes) } // ContainsAuth returns whether there is authentication configured @@ -318,3 +319,17 @@ func (configFile *ConfigFile) GetAllCredentials() (map[string]types.AuthConfig, func (configFile *ConfigFile) GetFilename() string { return configFile.Filename } + +func checkKubernetesConfiguration(kubeConfig *KubernetesConfig) error { + if kubeConfig == nil { + return nil + } + switch kubeConfig.AllNamespaces { + case "": + case "enabled": + case "disabled": + default: + return fmt.Errorf("invalid 'kubernetes.allNamespaces' value, should be 'enabled' or 'disabled': %s", kubeConfig.AllNamespaces) + } + return nil +} diff --git a/components/cli/cli/config/configfile/file_test.go b/components/cli/cli/config/configfile/file_test.go index c769f53c26..34e3b485b0 100644 --- a/components/cli/cli/config/configfile/file_test.go +++ b/components/cli/cli/config/configfile/file_test.go @@ -371,3 +371,45 @@ func TestGetAllCredentialsCredHelperOverridesDefaultStore(t *testing.T) { assert.Check(t, is.Equal(1, testCredsStore.(*mockNativeStore).GetAllCallCount)) assert.Check(t, is.Equal(0, testCredHelper.(*mockNativeStore).GetAllCallCount)) } + +func TestCheckKubernetesConfigurationRaiseAnErrorOnInvalidValue(t *testing.T) { + testCases := []struct { + name string + config *KubernetesConfig + expectError bool + }{ + { + "no kubernetes config is valid", + nil, + false, + }, + { + "enabled is valid", + &KubernetesConfig{AllNamespaces: "enabled"}, + false, + }, + { + "disabled is valid", + &KubernetesConfig{AllNamespaces: "disabled"}, + false, + }, + { + "empty string is valid", + &KubernetesConfig{AllNamespaces: ""}, + false, + }, + { + "other value is invalid", + &KubernetesConfig{AllNamespaces: "unknown"}, + true, + }, + } + for _, test := range testCases { + err := checkKubernetesConfiguration(test.config) + if test.expectError { + assert.Assert(t, err != nil, test.name) + } else { + assert.NilError(t, err, test.name) + } + } +} From 2720165925c9c58c70a8dcc657d0ecd2c6bee85e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 May 2018 12:21:41 +0200 Subject: [PATCH 6/6] Validate stack-names for empty values Add validation for stack names to prevent an empty name resulting in _all_ stacks to be returned after filtering, which can result in removal of services for all stacks if `--prune`, or `docker stack rm` is used. Before this change; docker stack deploy -c docker-compose.yml one docker stack deploy -c docker-compose.yml two docker stack deploy -c docker-compose.yml three docker stack deploy -c docker-compose.yml --prune '' Removing service one_web Removing service two_web Removing service three_web After this change: docker stack deploy -c docker-compose.yml one docker stack deploy -c docker-compose.yml two docker stack deploy -c docker-compose.yml three docker stack deploy -c docker-compose.yml --prune '' invalid stack name: "" Other stack commands were updated as well: Before this change; docker stack deploy -c docker-compose.yml '' Creating network _default failed to create network _default: Error response from daemon: rpc error: code = InvalidArgument desc = name must be valid as a DNS name component docker stack ps '' nothing found in stack: docker stack rm '' Removing service one_web Removing service three_web Removing service two_web After this change: docker stack deploy -c docker-compose.yml '' invalid stack name: "" docker stack ps '' invalid stack name: "" docker stack rm '' invalid stack name: "" Signed-off-by: Sebastiaan van Stijn Upstream-commit: d38f397da19271119fc3ea7b20293a7dd41964f8 Component: cli --- .../cli/cli/command/stack/swarm/common.go | 28 +++++++++++++++++++ .../cli/cli/command/stack/swarm/deploy.go | 3 ++ .../command/stack/swarm/deploy_bundlefile.go | 3 ++ .../command/stack/swarm/deploy_composefile.go | 3 ++ .../cli/command/stack/swarm/deploy_test.go | 10 +++++++ components/cli/cli/command/stack/swarm/ps.go | 10 ++++--- .../cli/cli/command/stack/swarm/ps_test.go | 18 ++++++++++++ .../cli/cli/command/stack/swarm/remove.go | 7 +++-- .../cli/command/stack/swarm/remove_test.go | 18 ++++++++++++ .../cli/cli/command/stack/swarm/services.go | 3 ++ .../cli/command/stack/swarm/services_test.go | 18 ++++++++++++ 11 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 components/cli/cli/command/stack/swarm/ps_test.go create mode 100644 components/cli/cli/command/stack/swarm/remove_test.go create mode 100644 components/cli/cli/command/stack/swarm/services_test.go diff --git a/components/cli/cli/command/stack/swarm/common.go b/components/cli/cli/command/stack/swarm/common.go index b4193df360..62ff0d9ff5 100644 --- a/components/cli/cli/command/stack/swarm/common.go +++ b/components/cli/cli/command/stack/swarm/common.go @@ -2,6 +2,9 @@ package swarm import ( "context" + "fmt" + "strings" + "unicode" "github.com/docker/cli/cli/compose/convert" "github.com/docker/cli/opts" @@ -48,3 +51,28 @@ 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/components/cli/cli/command/stack/swarm/deploy.go b/components/cli/cli/command/stack/swarm/deploy.go index 3479fd9210..0504e3381b 100644 --- a/components/cli/cli/command/stack/swarm/deploy.go +++ b/components/cli/cli/command/stack/swarm/deploy.go @@ -24,6 +24,9 @@ const ( func RunDeploy(dockerCli command.Cli, opts options.Deploy) error { ctx := context.Background() + if err := validateStackName(opts.Namespace); err != nil { + return err + } if err := validateResolveImageFlag(dockerCli, &opts); err != nil { return err } diff --git a/components/cli/cli/command/stack/swarm/deploy_bundlefile.go b/components/cli/cli/command/stack/swarm/deploy_bundlefile.go index 96d8c1ef5c..7b59126baf 100644 --- a/components/cli/cli/command/stack/swarm/deploy_bundlefile.go +++ b/components/cli/cli/command/stack/swarm/deploy_bundlefile.go @@ -16,6 +16,9 @@ import ( ) func deployBundle(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error { + if err := validateStackName(opts.Namespace); err != nil { + return err + } bundle, err := loadBundlefile(dockerCli.Err(), opts.Namespace, opts.Bundlefile) if err != nil { return err diff --git a/components/cli/cli/command/stack/swarm/deploy_composefile.go b/components/cli/cli/command/stack/swarm/deploy_composefile.go index 2c214e000a..a0b8b127d6 100644 --- a/components/cli/cli/command/stack/swarm/deploy_composefile.go +++ b/components/cli/cli/command/stack/swarm/deploy_composefile.go @@ -18,6 +18,9 @@ import ( ) 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 diff --git a/components/cli/cli/command/stack/swarm/deploy_test.go b/components/cli/cli/command/stack/swarm/deploy_test.go index 6a108e5f16..690ab48444 100644 --- a/components/cli/cli/command/stack/swarm/deploy_test.go +++ b/components/cli/cli/command/stack/swarm/deploy_test.go @@ -4,6 +4,7 @@ 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" @@ -26,6 +27,15 @@ 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/components/cli/cli/command/stack/swarm/ps.go b/components/cli/cli/command/stack/swarm/ps.go index ce90856fbe..79a154a8f2 100644 --- a/components/cli/cli/command/stack/swarm/ps.go +++ b/components/cli/cli/command/stack/swarm/ps.go @@ -13,19 +13,21 @@ import ( // RunPS is the swarm implementation of docker stack ps func RunPS(dockerCli command.Cli, opts options.PS) error { - namespace := opts.Namespace - client := dockerCli.Client() - ctx := context.Background() + if err := validateStackName(opts.Namespace); err != nil { + return err + } filter := getStackFilterFromOpt(opts.Namespace, opts.Filter) + ctx := context.Background() + client := dockerCli.Client() tasks, err := client.TaskList(ctx, types.TaskListOptions{Filters: filter}) if err != nil { return err } if len(tasks) == 0 { - return fmt.Errorf("nothing found in stack: %s", namespace) + return fmt.Errorf("nothing found in stack: %s", opts.Namespace) } format := opts.Format diff --git a/components/cli/cli/command/stack/swarm/ps_test.go b/components/cli/cli/command/stack/swarm/ps_test.go new file mode 100644 index 0000000000..1989f140bd --- /dev/null +++ b/components/cli/cli/command/stack/swarm/ps_test.go @@ -0,0 +1,18 @@ +package swarm + +import ( + "testing" + + "github.com/docker/cli/cli/command/stack/options" + "github.com/docker/cli/internal/test" + "github.com/gotestyourself/gotestyourself/assert" + is "github.com/gotestyourself/gotestyourself/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/components/cli/cli/command/stack/swarm/remove.go b/components/cli/cli/command/stack/swarm/remove.go index c799102eb4..e75597d065 100644 --- a/components/cli/cli/command/stack/swarm/remove.go +++ b/components/cli/cli/command/stack/swarm/remove.go @@ -16,12 +16,15 @@ import ( // RunRemove is the swarm implementation of docker stack remove func RunRemove(dockerCli command.Cli, opts options.Remove) error { - namespaces := opts.Namespaces + if err := validateStackNames(opts.Namespaces); err != nil { + return err + } + client := dockerCli.Client() ctx := context.Background() var errs []string - for _, namespace := range namespaces { + for _, namespace := range opts.Namespaces { services, err := getStackServices(ctx, client, namespace) if err != nil { return err diff --git a/components/cli/cli/command/stack/swarm/remove_test.go b/components/cli/cli/command/stack/swarm/remove_test.go new file mode 100644 index 0000000000..bfc43e742e --- /dev/null +++ b/components/cli/cli/command/stack/swarm/remove_test.go @@ -0,0 +1,18 @@ +package swarm + +import ( + "testing" + + "github.com/docker/cli/cli/command/stack/options" + "github.com/docker/cli/internal/test" + "github.com/gotestyourself/gotestyourself/assert" + is "github.com/gotestyourself/gotestyourself/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/components/cli/cli/command/stack/swarm/services.go b/components/cli/cli/command/stack/swarm/services.go index 07b990adc8..0225918678 100644 --- a/components/cli/cli/command/stack/swarm/services.go +++ b/components/cli/cli/command/stack/swarm/services.go @@ -14,6 +14,9 @@ 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/components/cli/cli/command/stack/swarm/services_test.go b/components/cli/cli/command/stack/swarm/services_test.go new file mode 100644 index 0000000000..dbd56222d5 --- /dev/null +++ b/components/cli/cli/command/stack/swarm/services_test.go @@ -0,0 +1,18 @@ +package swarm + +import ( + "testing" + + "github.com/docker/cli/cli/command/stack/options" + "github.com/docker/cli/internal/test" + "github.com/gotestyourself/gotestyourself/assert" + is "github.com/gotestyourself/gotestyourself/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: "' '"`)) +}