List images with multiple since/before filters

The List Images API endpoint has accepted multiple values for the
`since` and `before` filter predicates, but thanks to Go's randomizing
of map iteration order, it would pick an arbitrary image to compare
created timestamps against. In other words, the behaviour was undefined.
Change these filter predicates to have well-defined semantics: the
logical AND of all values for each of the respective predicates. As
timestamps are a totally-ordered relation, this is exactly equivalent to
applying the newest and oldest creation timestamps for the `since` and
`before` predicates, respectively.

Signed-off-by: Cory Snider <csnider@mirantis.com>

daemon/images/image_list.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"time"
 
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
@@ -46,20 +47,36 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 	}
 
 	var (
-		beforeFilter, sinceFilter *image.Image
+		beforeFilter, sinceFilter time.Time
 		err                       error
 	)
 	err = opts.Filters.WalkValues("before", func(value string) error {
-		beforeFilter, err = i.GetImage(ctx, value, imagetypes.GetImageOpts{})
-		return err
+		img, err := i.GetImage(ctx, value, imagetypes.GetImageOpts{})
+		if err != nil {
+			return err
+		}
+		// Resolve multiple values to the oldest image,
+		// equivalent to ANDing all the values together.
+		if beforeFilter.IsZero() || beforeFilter.After(img.Created) {
+			beforeFilter = img.Created
+		}
+		return nil
 	})
 	if err != nil {
 		return nil, err
 	}
 
 	err = opts.Filters.WalkValues("since", func(value string) error {
-		sinceFilter, err = i.GetImage(ctx, value, imagetypes.GetImageOpts{})
-		return err
+		img, err := i.GetImage(ctx, value, imagetypes.GetImageOpts{})
+		if err != nil {
+			return err
+		}
+		// Resolve multiple values to the newest image,
+		// equivalent to ANDing all the values together.
+		if sinceFilter.Before(img.Created) {
+			sinceFilter = img.Created
+		}
+		return nil
 	})
 	if err != nil {
 		return nil, err
@@ -84,16 +101,11 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 		default:
 		}
 
-		if beforeFilter != nil {
-			if img.Created.Equal(beforeFilter.Created) || img.Created.After(beforeFilter.Created) {
-				continue
-			}
+		if !beforeFilter.IsZero() && !img.Created.Before(beforeFilter) {
+			continue
 		}
-
-		if sinceFilter != nil {
-			if img.Created.Equal(sinceFilter.Created) || img.Created.Before(sinceFilter.Created) {
-				continue
-			}
+		if !sinceFilter.IsZero() && !img.Created.After(sinceFilter) {
+			continue
 		}
 
 		if opts.Filters.Contains("label") {

integration/image/list_test.go

@@ -2,12 +2,16 @@ package image // import "github.com/docker/docker/integration/image"
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/integration/internal/container"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/skip"
@@ -51,3 +55,41 @@ func TestImagesFilterMultiReference(t *testing.T) {
 		}
 	}
 }
+
+func TestImagesFilterBeforeSince(t *testing.T) {
+	defer setupTest(t)()
+	client := testEnv.APIClient()
+	ctx := context.Background()
+
+	name := strings.ToLower(t.Name())
+	ctr := container.Create(ctx, t, client, container.WithName(name))
+
+	imgs := make([]string, 5)
+	for i := range imgs {
+		if i > 0 {
+			// Make really really sure each image has a distinct timestamp.
+			time.Sleep(time.Millisecond)
+		}
+		id, err := client.ContainerCommit(ctx, ctr, types.ContainerCommitOptions{Reference: fmt.Sprintf("%s:v%d", name, i)})
+		assert.NilError(t, err)
+		imgs[i] = id.ID
+	}
+
+	filter := filters.NewArgs(
+		filters.Arg("since", imgs[0]),
+		filters.Arg("before", imgs[len(imgs)-1]),
+	)
+	list, err := client.ImageList(ctx, types.ImageListOptions{Filters: filter})
+	assert.NilError(t, err)
+
+	var listedIDs []string
+	for _, i := range list {
+		t.Logf("ImageList: ID=%v RepoTags=%v", i.ID, i.RepoTags)
+		listedIDs = append(listedIDs, i.ID)
+	}
+	// The ImageList API sorts the list by created timestamp... truncated to
+	// 1-second precision. Since all the images were created within
+	// milliseconds of each other, listedIDs is effectively unordered and
+	// the assertion must therefore be order-independent.
+	assert.DeepEqual(t, listedIDs, imgs[1:len(imgs)-1], cmpopts.SortSlices(func(a, b string) bool { return a < b }))
+}