Browse code

Fix regression when filtering container names using a leading slash

Commit 5c8da2e96738addd8a175e5b9a23b8c37d4a4dfe updated the filtering behavior
to match container-names without having to specify the leading slash.

This change caused a regression in situations where a regex was provided as
filter, using an explicit leading slash (`--filter name=^/mycontainername`).

This fix changes the filters to match containers both with, and without the
leading slash, effectively making the leading slash optional when filtering.

With this fix, filters with and without a leading slash produce the same result:

$ docker ps --filter name=^a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 2 minutes ago Up 2 minutes a2
56e53770e316 busybox "sh" 2 minutes ago Up 2 minutes a1

$ docker ps --filter name=^/a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 2 minutes ago Up 2 minutes a2
56e53770e316 busybox "sh" 3 minutes ago Up 3 minutes a1

$ docker ps --filter name=^b
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
b69003b6a6fe busybox "sh" About a minute ago Up About a minute b1

$ docker ps --filter name=^/b
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
b69003b6a6fe busybox "sh" 56 seconds ago Up 54 seconds b1

$ docker ps --filter name=/a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 3 minutes ago Up 3 minutes a2
56e53770e316 busybox "sh" 4 minutes ago Up 4 minutes a1

$ docker ps --filter name=a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
21afd6362b0c busybox "sh" 3 minutes ago Up 3 minutes a2
56e53770e316 busybox "sh" 4 minutes ago Up 4 minutes a1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2018/08/29 04:40:13
Showing 2 changed files
... ...
@@ -146,7 +146,8 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex
146 146
 				continue
147 147
 			}
148 148
 			for _, eachName := range idNames {
149
-				if ctx.filters.Match("name", strings.TrimPrefix(eachName, "/")) {
149
+				// match both on container name with, and without slash-prefix
150
+				if ctx.filters.Match("name", eachName) || ctx.filters.Match("name", strings.TrimPrefix(eachName, "/")) {
150 151
 					matches[id] = true
151 152
 				}
152 153
 			}
... ...
@@ -429,7 +430,7 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite
429 429
 	}
430 430
 
431 431
 	// Do not include container if the name doesn't match
432
-	if !ctx.filters.Match("name", strings.TrimPrefix(container.Name, "/")) {
432
+	if !ctx.filters.Match("name", container.Name) && !ctx.filters.Match("name", strings.TrimPrefix(container.Name, "/")) {
433 433
 		return excludeContainer
434 434
 	}
435 435
 
... ...
@@ -4,7 +4,6 @@ import (
4 4
 	"io/ioutil"
5 5
 	"os"
6 6
 	"path/filepath"
7
-	"strings"
8 7
 	"testing"
9 8
 
10 9
 	"github.com/docker/docker/api/types"
... ...
@@ -35,6 +34,7 @@ func TestMain(m *testing.M) {
35 35
 // work against it. It takes in a pointer to Daemon so that
36 36
 // minor operations are not repeated by the caller
37 37
 func setupContainerWithName(t *testing.T, name string, daemon *Daemon) *container.Container {
38
+	t.Helper()
38 39
 	var (
39 40
 		id              = uuid.New()
40 41
 		computedImageID = digest.FromString(id)
... ...
@@ -46,6 +46,9 @@ func setupContainerWithName(t *testing.T, name string, daemon *Daemon) *containe
46 46
 
47 47
 	c := container.NewBaseContainer(id, cRoot)
48 48
 	// these are for passing includeContainerInList
49
+	if name[0] != '/' {
50
+		name = "/" + name
51
+	}
49 52
 	c.Name = name
50 53
 	c.Running = true
51 54
 	c.HostConfig = &containertypes.HostConfig{}
... ...
@@ -68,7 +71,7 @@ func setupContainerWithName(t *testing.T, name string, daemon *Daemon) *containe
68 68
 func containerListContainsName(containers []*types.Container, name string) bool {
69 69
 	for _, container := range containers {
70 70
 		for _, containerName := range container.Names {
71
-			if strings.TrimPrefix(containerName, "/") == name {
71
+			if containerName == name {
72 72
 				return true
73 73
 			}
74 74
 		}
... ...
@@ -110,16 +113,33 @@ func TestNameFilter(t *testing.T) {
110 110
 	containerList, err := d.Containers(&types.ContainerListOptions{
111 111
 		Filters: filters.NewArgs(filters.Arg("name", "^a")),
112 112
 	})
113
-	assert.Assert(t, err == nil)
113
+	assert.NilError(t, err)
114 114
 	assert.Assert(t, is.Len(containerList, 2))
115 115
 	assert.Assert(t, containerListContainsName(containerList, one.Name))
116 116
 	assert.Assert(t, containerListContainsName(containerList, two.Name))
117 117
 
118
+	// Same as above but with slash prefix should produce the same result
119
+	containerListWithPrefix, err := d.Containers(&types.ContainerListOptions{
120
+		Filters: filters.NewArgs(filters.Arg("name", "^/a")),
121
+	})
122
+	assert.NilError(t, err)
123
+	assert.Assert(t, is.Len(containerListWithPrefix, 2))
124
+	assert.Assert(t, containerListContainsName(containerListWithPrefix, one.Name))
125
+	assert.Assert(t, containerListContainsName(containerListWithPrefix, two.Name))
126
+
118 127
 	// Same as above but make sure it works for exact names
119 128
 	containerList, err = d.Containers(&types.ContainerListOptions{
120 129
 		Filters: filters.NewArgs(filters.Arg("name", "b1")),
121 130
 	})
122
-	assert.Assert(t, err == nil)
131
+	assert.NilError(t, err)
123 132
 	assert.Assert(t, is.Len(containerList, 1))
124 133
 	assert.Assert(t, containerListContainsName(containerList, three.Name))
134
+
135
+	// Same as above but with slash prefix should produce the same result
136
+	containerListWithPrefix, err = d.Containers(&types.ContainerListOptions{
137
+		Filters: filters.NewArgs(filters.Arg("name", "/b1")),
138
+	})
139
+	assert.NilError(t, err)
140
+	assert.Assert(t, is.Len(containerListWithPrefix, 1))
141
+	assert.Assert(t, containerListContainsName(containerListWithPrefix, three.Name))
125 142
 }