Browse code

Do not rely on string comparison in truncindex

Signed-off-by: Alexander Morozov <lk4d4@docker.com>

Alexander Morozov authored on 2015/11/04 10:19:18
Showing 8 changed files
... ...
@@ -242,7 +242,7 @@ func (s *router) deleteImages(ctx context.Context, w http.ResponseWriter, r *htt
242 242
 
243 243
 	name := vars["name"]
244 244
 
245
-	if name == "" {
245
+	if strings.TrimSpace(name) == "" {
246 246
 		return fmt.Errorf("image name cannot be blank")
247 247
 	}
248 248
 
... ...
@@ -1,14 +1,10 @@
1 1
 package daemon
2 2
 
3 3
 import (
4
-	"strings"
5
-
6 4
 	"github.com/Sirupsen/logrus"
7 5
 	"github.com/docker/docker/api/types"
8 6
 	derr "github.com/docker/docker/errors"
9
-	"github.com/docker/docker/graph/tags"
10 7
 	"github.com/docker/docker/image"
11
-	"github.com/docker/docker/pkg/parsers"
12 8
 	"github.com/docker/docker/pkg/stringid"
13 9
 	"github.com/docker/docker/runconfig"
14 10
 	"github.com/docker/docker/volume"
... ...
@@ -38,17 +34,7 @@ func (daemon *Daemon) ContainerCreate(params *ContainerCreateConfig) (types.Cont
38 38
 
39 39
 	container, err := daemon.create(params)
40 40
 	if err != nil {
41
-		if daemon.Graph().IsNotExist(err, params.Config.Image) {
42
-			if strings.Contains(params.Config.Image, "@") {
43
-				return types.ContainerCreateResponse{ID: "", Warnings: warnings}, derr.ErrorCodeNoSuchImageHash.WithArgs(params.Config.Image)
44
-			}
45
-			img, tag := parsers.ParseRepositoryTag(params.Config.Image)
46
-			if tag == "" {
47
-				tag = tags.DefaultTag
48
-			}
49
-			return types.ContainerCreateResponse{ID: "", Warnings: warnings}, derr.ErrorCodeNoSuchImageTag.WithArgs(img, tag)
50
-		}
51
-		return types.ContainerCreateResponse{ID: "", Warnings: warnings}, err
41
+		return types.ContainerCreateResponse{ID: "", Warnings: warnings}, daemon.graphNotExistToErrcode(params.Config.Image, err)
52 42
 	}
53 43
 
54 44
 	return types.ContainerCreateResponse{ID: container.ID, Warnings: warnings}, nil
... ...
@@ -149,7 +149,7 @@ func (daemon *Daemon) Get(prefixOrName string) (*Container, error) {
149 149
 	containerID, indexError := daemon.idIndex.Get(prefixOrName)
150 150
 	if indexError != nil {
151 151
 		// When truncindex defines an error type, use that instead
152
-		if strings.Contains(indexError.Error(), "no such id") {
152
+		if indexError == truncindex.ErrNotExist {
153 153
 			return nil, derr.ErrorCodeNoSuchContainer.WithArgs(prefixOrName)
154 154
 		}
155 155
 		return nil, indexError
156 156
new file mode 100644
... ...
@@ -0,0 +1,23 @@
0
+package daemon
1
+
2
+import (
3
+	"strings"
4
+
5
+	derr "github.com/docker/docker/errors"
6
+	"github.com/docker/docker/graph/tags"
7
+	"github.com/docker/docker/pkg/parsers"
8
+)
9
+
10
+func (d *Daemon) graphNotExistToErrcode(imageName string, err error) error {
11
+	if d.Graph().IsNotExist(err, imageName) {
12
+		if strings.Contains(imageName, "@") {
13
+			return derr.ErrorCodeNoSuchImageHash.WithArgs(imageName)
14
+		}
15
+		img, tag := parsers.ParseRepositoryTag(imageName)
16
+		if tag == "" {
17
+			tag = tags.DefaultTag
18
+		}
19
+		return derr.ErrorCodeNoSuchImageTag.WithArgs(img, tag)
20
+	}
21
+	return err
22
+}
... ...
@@ -55,7 +55,7 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I
55 55
 
56 56
 	img, err := daemon.repositories.LookupImage(imageRef)
57 57
 	if err != nil {
58
-		return nil, err
58
+		return nil, daemon.graphNotExistToErrcode(imageRef, err)
59 59
 	}
60 60
 
61 61
 	var removedRepositoryRef bool
... ...
@@ -221,7 +221,10 @@ func (graph *Graph) Exists(id string) bool {
221 221
 func (graph *Graph) Get(name string) (*image.Image, error) {
222 222
 	id, err := graph.idIndex.Get(name)
223 223
 	if err != nil {
224
-		return nil, fmt.Errorf("could not find image: %v", err)
224
+		if err == truncindex.ErrNotExist {
225
+			return nil, fmt.Errorf("image %s does not exist", name)
226
+		}
227
+		return nil, err
225 228
 	}
226 229
 	img, err := graph.loadImage(id)
227 230
 	if err != nil {
... ...
@@ -230,10 +230,10 @@ func (s *DockerSuite) TestRmiBlank(c *check.C) {
230 230
 	c.Assert(out, checker.Contains, "image name cannot be blank", check.Commentf("out: %s", out))
231 231
 
232 232
 	out, _, err = dockerCmdWithError("rmi", " ")
233
-	// Should have failed to delete '' image
233
+	// Should have failed to delete ' ' image
234 234
 	c.Assert(err, checker.NotNil)
235 235
 	// Expected error message not generated
236
-	c.Assert(out, checker.Contains, "no such id", check.Commentf("out: %s", out))
236
+	c.Assert(out, checker.Contains, "image name cannot be blank", check.Commentf("out: %s", out))
237 237
 }
238 238
 
239 239
 func (s *DockerSuite) TestRmiContainerImageNotFound(c *check.C) {
... ...
@@ -22,6 +22,9 @@ var (
22 22
 
23 23
 	// ErrIllegalChar is returned when a space is in the ID
24 24
 	ErrIllegalChar = errors.New("illegal character: ' '")
25
+
26
+	// ErrNotExist is returned when ID or its prefix not found in index.
27
+	ErrNotExist = errors.New("ID does not exist")
25 28
 )
26 29
 
27 30
 // TruncIndex allows the retrieval of string identifiers by any of their unique prefixes.
... ...
@@ -116,7 +119,7 @@ func (idx *TruncIndex) Get(s string) (string, error) {
116 116
 	if id != "" {
117 117
 		return id, nil
118 118
 	}
119
-	return "", fmt.Errorf("no such id: %s", s)
119
+	return "", ErrNotExist
120 120
 }
121 121
 
122 122
 // Iterate iterates over all stored IDs, and passes each of them to the given handler.