Skip to content

Commit

Permalink
Validate stack-names for empty values
Browse files Browse the repository at this point in the history
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 <github@gone.nl>
  • Loading branch information
thaJeztah committed May 28, 2018
1 parent 05f04bb commit d38f397
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 6 deletions.
28 changes: 28 additions & 0 deletions cli/command/stack/swarm/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package swarm

import (
"context"
"fmt"
"strings"
"unicode"

"github.com/docker/cli/cli/compose/convert"
"github.com/docker/cli/opts"
Expand Down Expand Up @@ -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 == '\''
}
3 changes: 3 additions & 0 deletions cli/command/stack/swarm/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions cli/command/stack/swarm/deploy_bundlefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions cli/command/stack/swarm/deploy_composefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions cli/command/stack/swarm/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions cli/command/stack/swarm/ps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions cli/command/stack/swarm/ps_test.go
Original file line number Diff line number Diff line change
@@ -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: "' '"`))
}
7 changes: 5 additions & 2 deletions cli/command/stack/swarm/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions cli/command/stack/swarm/remove_test.go
Original file line number Diff line number Diff line change
@@ -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: "' '"`))
}
3 changes: 3 additions & 0 deletions cli/command/stack/swarm/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
18 changes: 18 additions & 0 deletions cli/command/stack/swarm/services_test.go
Original file line number Diff line number Diff line change
@@ -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: "' '"`))
}

0 comments on commit d38f397

Please sign in to comment.