Merge pull request moby#15693 from vdemeester/15659-image-label-filter

Fix moby#15659 : filter by label for docker images commited

graph/list.go

@@ -108,17 +108,19 @@ func (s *TagStore) Images(filterArgs, filter string, all bool) ([]*types.Image,
 			} else {
 				// get the boolean list for if only the untagged images are requested
 				delete(allImages, id)
-				if !imageFilters.MatchKVList("label", image.ContainerConfig.Labels) {
-					continue
+
+				if len(imageFilters["label"]) > 0 {
+					if image.Config == nil {
+						// Very old image that do not have image.Config (or even labels)
+						continue
+					}
+					// We are now sure image.Config is not nil
+					if !imageFilters.MatchKVList("label", image.Config.Labels) {
+						continue
+					}
 				}
 				if filtTagged {
-					newImage := new(types.Image)
-					newImage.ParentID = image.Parent
-					newImage.ID = image.ID
-					newImage.Created = image.Created.Unix()
-					newImage.Size = image.Size
-					newImage.VirtualSize = s.graph.GetParentsSize(image) + image.Size
-					newImage.Labels = image.ContainerConfig.Labels
+					newImage := newImage(image, s.graph.GetParentsSize(image))
 
 					if utils.DigestReference(ref) {
 						newImage.RepoTags = []string{}
@@ -144,18 +146,19 @@ func (s *TagStore) Images(filterArgs, filter string, all bool) ([]*types.Image,
 	// Display images which aren't part of a repository/tag
 	if filter == "" || filtLabel {
 		for _, image := range allImages {
-			if !imageFilters.MatchKVList("label", image.ContainerConfig.Labels) {
-				continue
+			if len(imageFilters["label"]) > 0 {
+				if image.Config == nil {
+					// Very old image that do not have image.Config (or even labels)
+					continue
+				}
+				// We are now sure image.Config is not nil
+				if !imageFilters.MatchKVList("label", image.Config.Labels) {
+					continue
+				}
 			}
-			newImage := new(types.Image)
-			newImage.ParentID = image.Parent
+			newImage := newImage(image, s.graph.GetParentsSize(image))
 			newImage.RepoTags = []string{"<none>:<none>"}
 			newImage.RepoDigests = []string{"<none>@<none>"}
-			newImage.ID = image.ID
-			newImage.Created = image.Created.Unix()
-			newImage.Size = image.Size
-			newImage.VirtualSize = s.graph.GetParentsSize(image) + image.Size
-			newImage.Labels = image.ContainerConfig.Labels
 
 			images = append(images, newImage)
 		}
@@ -165,3 +168,16 @@ func (s *TagStore) Images(filterArgs, filter string, all bool) ([]*types.Image,
 
 	return images, nil
 }
+
+func newImage(image *image.Image, parentSize int64) *types.Image {
+	newImage := new(types.Image)
+	newImage.ParentID = image.Parent
+	newImage.ID = image.ID
+	newImage.Created = image.Created.Unix()
+	newImage.Size = image.Size
+	newImage.VirtualSize = parentSize + image.Size
+	if image.Config != nil {
+		newImage.Labels = image.Config.Labels
+	}
+	return newImage
+}

integration-cli/docker_cli_images_test.go

@@ -104,35 +104,40 @@ func (s *DockerSuite) TestImagesFilterLabel(c *check.C) {
 	image1ID, err := buildImage(imageName1,
 		`FROM scratch
 		 LABEL match me`, true)
-	if err != nil {
-		c.Fatal(err)
-	}
+	c.Assert(err, check.IsNil)
 
 	image2ID, err := buildImage(imageName2,
 		`FROM scratch
 		 LABEL match="me too"`, true)
-	if err != nil {
-		c.Fatal(err)
-	}
+	c.Assert(err, check.IsNil)
 
 	image3ID, err := buildImage(imageName3,
 		`FROM scratch
 		 LABEL nomatch me`, true)
-	if err != nil {
-		c.Fatal(err)
-	}
+	c.Assert(err, check.IsNil)
 
 	out, _ := dockerCmd(c, "images", "--no-trunc", "-q", "-f", "label=match")
 	out = strings.TrimSpace(out)
-	if (!strings.Contains(out, image1ID) && !strings.Contains(out, image2ID)) || strings.Contains(out, image3ID) {
-		c.Fatalf("Expected ids %s,%s got %s", image1ID, image2ID, out)
-	}
+	c.Assert(out, check.Matches, fmt.Sprintf("[\\s\\w]*%s[\\s\\w]*", image1ID))
+	c.Assert(out, check.Matches, fmt.Sprintf("[\\s\\w]*%s[\\s\\w]*", image2ID))
+	c.Assert(out, check.Not(check.Matches), fmt.Sprintf("[\\s\\w]*%s[\\s\\w]*", image3ID))
 
 	out, _ = dockerCmd(c, "images", "--no-trunc", "-q", "-f", "label=match=me too")
 	out = strings.TrimSpace(out)
-	if out != image2ID {
-		c.Fatalf("Expected %s got %s", image2ID, out)
-	}
+	c.Assert(out, check.Equals, image2ID)
+}
+
+// Regression : #15659
+func (s *DockerSuite) TestImagesFilterLabelWithCommit(c *check.C) {
+	// Create a container
+	dockerCmd(c, "run", "--name", "bar", "busybox", "/bin/sh")
+	// Commit with labels "using changes"
+	out, _ := dockerCmd(c, "commit", "-c", "LABEL foo.version=1.0.0-1", "-c", "LABEL foo.name=bar", "-c", "LABEL foo.author=starlord", "bar", "bar:1.0.0-1")
+	imageID := strings.TrimSpace(out)
+
+	out, _ = dockerCmd(c, "images", "--no-trunc", "-q", "-f", "label=foo.version=1.0.0-1")
+	out = strings.TrimSpace(out)
+	c.Assert(out, check.Equals, imageID)
 }
 
 func (s *DockerSuite) TestImagesFilterSpaceTrimCase(c *check.C) {

integration-cli/docker_cli_save_load_test.go

@@ -128,6 +128,10 @@ func (s *DockerSuite) TestSaveImageId(c *check.C) {
 	out, _ = dockerCmd(c, "images", "-q", repoName)
 	cleanedShortImageID := strings.TrimSpace(out)
 
+	// Make sure IDs are not empty
+	c.Assert(cleanedLongImageID, check.Not(check.Equals), "", check.Commentf("Id should not be empty."))
+	c.Assert(cleanedShortImageID, check.Not(check.Equals), "", check.Commentf("Id should not be empty."))
+
 	saveCmd := exec.Command(dockerBinary, "save", cleanedShortImageID)
 	tarCmd := exec.Command("tar", "t")