Use OCI "History" type instead of inventing our own copy

The most notable change here is that the OCI's type uses a pointer for `Created`, which we probably should've been too, so most of these changes are accounting for that (and embedding our `Equal` implementation in the one single place it was used).

Signed-off-by: Tianon Gravi <admwiggin@gmail.com>

api/server/router/image/image_routes.go

@@ -294,13 +294,18 @@ func (ir *imageRouter) toImageInspect(img *image.Image) (*types.ImageInspect, er
 		repoDigests = []string{}
 	}
 
+	var created string
+	if img.Created != nil {
+		created = img.Created.Format(time.RFC3339Nano)
+	}
+
 	return &types.ImageInspect{
 		ID:              img.ID().String(),
 		RepoTags:        repoTags,
 		RepoDigests:     repoDigests,
 		Parent:          img.Parent.String(),
 		Comment:         comment,
-		Created:         img.Created.Format(time.RFC3339Nano),
+		Created:         created,
 		Container:       img.Container,
 		ContainerConfig: &img.ContainerConfig,
 		DockerVersion:   img.DockerVersion,

daemon/containerd/image.go

@@ -68,30 +68,12 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
 		exposedPorts[nat.Port(k)] = v
 	}
 
-	derefTimeSafely := func(t *time.Time) time.Time {
-		if t != nil {
-			return *t
-		}
-		return time.Time{}
-	}
-
-	var imgHistory []image.History
-	for _, h := range ociimage.History {
-		imgHistory = append(imgHistory, image.History{
-			Created:    derefTimeSafely(h.Created),
-			Author:     h.Author,
-			CreatedBy:  h.CreatedBy,
-			Comment:    h.Comment,
-			EmptyLayer: h.EmptyLayer,
-		})
-	}
-
 	img := image.NewImage(image.ID(desc.Digest))
 	img.V1Image = image.V1Image{
 		ID:           string(desc.Digest),
 		OS:           ociimage.OS,
 		Architecture: ociimage.Architecture,
-		Created:      derefTimeSafely(ociimage.Created),
+		Created:      ociimage.Created,
 		Config: &containertypes.Config{
 			Entrypoint:   ociimage.Config.Entrypoint,
 			Env:          ociimage.Config.Env,
@@ -106,7 +88,7 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
 	}
 
 	img.RootFS = rootfs
-	img.History = imgHistory
+	img.History = ociimage.History
 
 	if options.Details {
 		lastUpdated := time.Unix(0, 0)

daemon/containerd/image_builder.go

@@ -402,21 +402,9 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 		exposedPorts[string(k)] = v
 	}
 
-	var ociHistory []ocispec.History
-	for _, history := range imgToCreate.History {
-		created := history.Created
-		ociHistory = append(ociHistory, ocispec.History{
-			Created:    &created,
-			CreatedBy:  history.CreatedBy,
-			Author:     history.Author,
-			Comment:    history.Comment,
-			EmptyLayer: history.EmptyLayer,
-		})
-	}
-
 	// make an ocispec.Image from the docker/image.Image
 	ociImgToCreate := ocispec.Image{
-		Created: &imgToCreate.Created,
+		Created: imgToCreate.Created,
 		Author:  imgToCreate.Author,
 		Platform: ocispec.Platform{
 			Architecture: imgToCreate.Architecture,
@@ -437,7 +425,7 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 			StopSignal:   imgToCreate.Config.StopSignal,
 		},
 		RootFS:  rootfs,
-		History: ociHistory,
+		History: imgToCreate.History,
 	}
 
 	var layers []ocispec.Descriptor

daemon/images/image_import.go

@@ -58,7 +58,7 @@ func (i *ImageService) ImportImage(ctx context.Context, newRef reference.Named,
 			Architecture:  platform.Architecture,
 			Variant:       platform.Variant,
 			OS:            platform.OS,
-			Created:       created,
+			Created:       &created,
 			Comment:       msg,
 		},
 		RootFS: &image.RootFS{
@@ -66,7 +66,7 @@ func (i *ImageService) ImportImage(ctx context.Context, newRef reference.Named,
 			DiffIDs: []layer.DiffID{l.DiffID()},
 		},
 		History: []image.History{{
-			Created: created,
+			Created: &created,
 			Comment: msg,
 		}},
 	})

daemon/images/image_list.go

@@ -53,8 +53,8 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 		}
 		// Resolve multiple values to the oldest image,
 		// equivalent to ANDing all the values together.
-		if beforeFilter.IsZero() || beforeFilter.After(img.Created) {
-			beforeFilter = img.Created
+		if img.Created != nil && (beforeFilter.IsZero() || beforeFilter.After(*img.Created)) {
+			beforeFilter = *img.Created
 		}
 		return nil
 	})
@@ -69,8 +69,8 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 		}
 		// Resolve multiple values to the newest image,
 		// equivalent to ANDing all the values together.
-		if sinceFilter.Before(img.Created) {
-			sinceFilter = img.Created
+		if img.Created != nil && sinceFilter.Before(*img.Created) {
+			sinceFilter = *img.Created
 		}
 		return nil
 	})
@@ -97,10 +97,10 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 		default:
 		}
 
-		if !beforeFilter.IsZero() && !img.Created.Before(beforeFilter) {
+		if !beforeFilter.IsZero() && (img.Created == nil || !img.Created.Before(beforeFilter)) {
 			continue
 		}
-		if !sinceFilter.IsZero() && !img.Created.After(sinceFilter) {
+		if !sinceFilter.IsZero() && (img.Created == nil || !img.Created.After(sinceFilter)) {
 			continue
 		}
 
@@ -256,10 +256,14 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 }
 
 func newImageSummary(image *image.Image, size int64) *types.ImageSummary {
+	var created int64
+	if image.Created != nil {
+		created = image.Created.Unix()
+	}
 	summary := &types.ImageSummary{
 		ParentID: image.Parent.String(),
 		ID:       image.ID().String(),
-		Created:  image.Created.Unix(),
+		Created:  created,
 		Size:     size,
 		// -1 indicates that the value has not been set (avoids ambiguity
 		// between 0 (default) and "not set". We cannot use a pointer (nil)

daemon/images/image_prune.go

@@ -75,7 +75,7 @@ func (i *ImageService) ImagesPrune(ctx context.Context, pruneFilters filters.Arg
 			if len(i.referenceStore.References(dgst)) == 0 && len(i.imageStore.Children(id)) != 0 {
 				continue
 			}
-			if !until.IsZero() && img.Created.After(until) {
+			if !until.IsZero() && (img.Created == nil || img.Created.After(until)) {
 				continue
 			}
 			if img.Config != nil && !matchLabels(pruneFilters, img.Config.Labels) {

daemon/images/image_squash.go

@@ -76,10 +76,10 @@ func (i *ImageService) SquashImage(id, parent string) (string, error) {
 	}
 
 	newImage.History = append(newImage.History, image.History{
-		Created: now,
+		Created: &now,
 		Comment: historyComment,
 	})
-	newImage.Created = now
+	newImage.Created = &now
 
 	b, err := json.Marshal(&newImage)
 	if err != nil {

image/cache/cache.go

@@ -227,7 +227,7 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
 
 			if compare(&img.ContainerConfig, config) {
 				// check for the most up to date match
-				if match == nil || match.Created.Before(img.Created) {
+				if img.Created != nil && (match == nil || match.Created.Before(*img.Created)) {
 					match = img
 				}
 			}

image/image.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
-	"reflect"
 	"runtime"
 	"strings"
 	"time"
@@ -53,7 +52,7 @@ type V1Image struct {
 	Comment string `json:"comment,omitempty"`
 
 	// Created is the timestamp at which the image was created
-	Created time.Time `json:"created"`
+	Created *time.Time `json:"created"`
 
 	// Container is the ID of the container that was used to create the image.
 	//
@@ -261,44 +260,21 @@ func NewChildImage(img *Image, child ChildConfig, os string) *Image {
 }
 
 // History stores build commands that were used to create an image
-type History struct {
-	// Created is the timestamp at which the image was created
-	Created time.Time `json:"created"`
-	// Author is the name of the author that was specified when committing the
-	// image, or as specified through MAINTAINER (deprecated) in the Dockerfile.
-	Author string `json:"author,omitempty"`
-	// CreatedBy keeps the Dockerfile command used while building the image
-	CreatedBy string `json:"created_by,omitempty"`
-	// Comment is the commit message that was set when committing the image
-	Comment string `json:"comment,omitempty"`
-	// EmptyLayer is set to true if this history item did not generate a
-	// layer. Otherwise, the history item is associated with the next
-	// layer in the RootFS section.
-	EmptyLayer bool `json:"empty_layer,omitempty"`
-}
+type History = ocispec.History
 
 // NewHistory creates a new history struct from arguments, and sets the created
 // time to the current time in UTC
 func NewHistory(author, comment, createdBy string, isEmptyLayer bool) History {
+	now := time.Now().UTC()
 	return History{
 		Author:     author,
-		Created:    time.Now().UTC(),
+		Created:    &now,
 		CreatedBy:  createdBy,
 		Comment:    comment,
 		EmptyLayer: isEmptyLayer,
 	}
 }
 
-// Equal compares two history structs for equality
-func (h History) Equal(i History) bool {
-	if !h.Created.Equal(i.Created) {
-		return false
-	}
-	i.Created = h.Created
-
-	return reflect.DeepEqual(h, i)
-}
-
 // Exporter provides interface for loading and saving images
 type Exporter interface {
 	Load(io.ReadCloser, io.Writer, bool) error

image/image_test.go

@@ -56,29 +56,6 @@ func TestMarshalKeyOrder(t *testing.T) {
 	}
 }
 
-const sampleHistoryJSON = `{
-	"created": "2021-01-13T09:35:56Z",
-	"created_by": "image_test.go"
-}`
-
-func TestHistoryEqual(t *testing.T) {
-	h := historyFromJSON(t, sampleHistoryJSON)
-	hCopy := h
-	assert.Check(t, h.Equal(hCopy))
-
-	hUTC := historyFromJSON(t, `{"created": "2021-01-13T14:00:00Z"}`)
-	hOffset0 := historyFromJSON(t, `{"created": "2021-01-13T14:00:00+00:00"}`)
-	assert.Check(t, hUTC.Created != hOffset0.Created)
-	assert.Check(t, hUTC.Equal(hOffset0))
-}
-
-func historyFromJSON(t *testing.T, historyJSON string) History {
-	var h History
-	err := json.Unmarshal([]byte(historyJSON), &h)
-	assert.Check(t, err)
-	return h
-}
-
 func TestImage(t *testing.T) {
 	cid := "50a16564e727"
 	config := &container.Config{

image/tarexport/load.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 
 	"github.com/docker/distribution"
@@ -396,8 +397,16 @@ func checkValidParent(img, parent *image.Image) bool {
 	if len(img.History)-len(parent.History) != 1 {
 		return false
 	}
-	for i, h := range parent.History {
-		if !h.Equal(img.History[i]) {
+	for i, hP := range parent.History {
+		hC := img.History[i]
+		if (hP.Created == nil) != (hC.Created == nil) {
+			return false
+		}
+		if hP.Created != nil && !hP.Created.Equal(*hC.Created) {
+			return false
+		}
+		hC.Created = hP.Created
+		if !reflect.DeepEqual(hP, hC) {
 			return false
 		}
 	}

image/tarexport/save.go

@@ -379,10 +379,11 @@ func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]distribution.Desc
 	var layers []digest.Digest
 	var foreignSrcs map[layer.DiffID]distribution.Descriptor
 	for i, diffID := range img.RootFS.DiffIDs {
+		v1ImgCreated := time.Unix(0, 0)
 		v1Img := image.V1Image{
 			// This is for backward compatibility used for
 			// pre v1.9 docker.
-			Created: time.Unix(0, 0),
+			Created: &v1ImgCreated,
 		}
 		if i == len(img.RootFS.DiffIDs)-1 {
 			v1Img = img.V1Image
@@ -422,26 +423,30 @@ func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]distribution.Desc
 	if err := os.MkdirAll(blobDir, 0o755); err != nil {
 		return nil, err
 	}
-	if err := system.Chtimes(blobDir, img.Created, img.Created); err != nil {
-		return nil, err
-	}
-	if err := system.Chtimes(filepath.Dir(blobDir), img.Created, img.Created); err != nil {
-		return nil, err
+	if img.Created != nil {
+		if err := system.Chtimes(blobDir, *img.Created, *img.Created); err != nil {
+			return nil, err
+		}
+		if err := system.Chtimes(filepath.Dir(blobDir), *img.Created, *img.Created); err != nil {
+			return nil, err
+		}
 	}
 
 	configFile := filepath.Join(blobDir, dgst.Encoded())
 	if err := os.WriteFile(configFile, img.RawJSON(), 0o644); err != nil {
 		return nil, err
 	}
-	if err := system.Chtimes(configFile, img.Created, img.Created); err != nil {
-		return nil, err
+	if img.Created != nil {
+		if err := system.Chtimes(configFile, *img.Created, *img.Created); err != nil {
+			return nil, err
+		}
 	}
 
 	s.images[id].layers = layers
 	return foreignSrcs, nil
 }
 
-func (s *saveSession) saveLayer(id layer.ChainID, legacyImg image.V1Image, createdTime time.Time) (distribution.Descriptor, error) {
+func (s *saveSession) saveLayer(id layer.ChainID, legacyImg image.V1Image, createdTime *time.Time) (distribution.Descriptor, error) {
 	if _, exists := s.savedLayers[legacyImg.ID]; exists {
 		return distribution.Descriptor{}, nil
 	}
@@ -499,10 +504,12 @@ func (s *saveSession) saveLayer(id layer.ChainID, legacyImg image.V1Image, creat
 			return distribution.Descriptor{}, err
 		}
 
-		for _, fname := range []string{outDir, configPath, layerPath} {
-			// todo: maybe save layer created timestamp?
-			if err := system.Chtimes(fname, createdTime, createdTime); err != nil {
-				return distribution.Descriptor{}, errors.Wrap(err, "could not set layer timestamp")
+		if createdTime != nil {
+			for _, fname := range []string{outDir, configPath, layerPath} {
+				// todo: maybe save layer created timestamp?
+				if err := system.Chtimes(fname, *createdTime, *createdTime); err != nil {
+					return distribution.Descriptor{}, errors.Wrap(err, "could not set layer timestamp")
+				}
 			}
 		}