Skip to content

Commit

Permalink
bugfix: filtered list-type image should also be ignored if not changed
Browse files Browse the repository at this point in the history
  • Loading branch information
@hhyasdf
hhyasdf committed Jun 28, 2023
1 parent b78c4f0 commit 3e702d5
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 60 deletions.
29 changes: 19 additions & 10 deletions pkg/sync/destination.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sync
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"

Expand Down Expand Up @@ -95,30 +96,38 @@ func (i *ImageDestination) PushManifest(manifestByte []byte) error {
return i.destination.PutManifest(i.ctx, manifestByte, nil)
}

// CheckManifestChanged checks if manifest of destination (tag) has changed, by manifest bytes or digest
// CheckManifestChanged checks if manifest of destination (tag) has changed
func (i *ImageDestination) CheckManifestChanged(newManifestByte []byte) bool {
// just use tag to get manifest
oldManifestByte := i.GetManifest(i.tag)

var a bytes.Buffer
_ = json.Compact(&a, oldManifestByte)

var b bytes.Buffer
_ = json.Compact(&b, newManifestByte)

return a.String() != b.String()
}

func (i *ImageDestination) GetManifest(tagOrDigest string) []byte {
var err error

// create source to check manifest
source, err := i.destinationRef.NewImageSource(i.ctx, i.sysctx)
if err != nil {
// if the source cannot be created, manifest not exist
return false
return nil
}

// just use tag to get manifest
tDigest := digest.Digest(i.tag)
tDigest := digest.Digest(tagOrDigest)
tManifestByte, _, err := source.GetManifest(i.ctx, &tDigest)
if err != nil {
// if error happens, it's considered that the manifest not exist
return false
}

if !bytes.Equal(tManifestByte, newManifestByte) {
return false
return nil
}

return true
return tManifestByte
}

// PutABlob push a blob to destination image
Expand Down
55 changes: 30 additions & 25 deletions pkg/sync/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,66 +10,67 @@ import (
"github.com/tidwall/gjson"
)

// GenerateManifestObj returns a manifest object and a sub manifest object array for list-type manifests.
// GenerateManifestObj returns a new manifest object along with its byte serialization, and a sub manifest object array
// for list-type manifests.
// For list type manifest, the origin manifest info might be modified because of platform filters, and a nil manifest
// object will be returned if no sub manifest need to transport.
// For non-list type manifests, which doesn't match the filters, a nil manifest object will be returned.
func GenerateManifestObj(manifestBytes []byte, manifestType string, osFilterList, archFilterList []string,
i *ImageSource, parent *manifest.Schema2List) (interface{}, []manifest.Manifest, error) {
i *ImageSource, parent *manifest.Schema2List) (interface{}, []byte, []manifest.Manifest, error) {

switch manifestType {
case manifest.DockerV2Schema2MediaType:
manifestInfo, err := manifest.Schema2FromManifest(manifestBytes)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

// platform info stored in config blob
if parent == nil && manifestInfo.ConfigInfo().Digest != "" {
blob, _, err := i.GetABlob(manifestInfo.ConfigInfo())
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
defer blob.Close()
bytes, err := io.ReadAll(blob)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
results := gjson.GetManyBytes(bytes, "architecture", "os")

if !platformValidate(osFilterList, archFilterList,
&manifest.Schema2PlatformSpec{Architecture: results[0].String(), OS: results[1].String()}) {
return nil, nil, nil
return nil, nil, nil, nil
}
}

return manifestInfo, nil, nil
return manifestInfo, manifestBytes, nil, nil
case manifest.DockerV2Schema1MediaType, manifest.DockerV2Schema1SignedMediaType:
manifestInfo, err := manifest.Schema1FromManifest(manifestBytes)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

// v1 only support architecture and this field is for information purposes and not currently used by the engine.
if parent == nil && !platformValidate(osFilterList, archFilterList,
&manifest.Schema2PlatformSpec{Architecture: manifestInfo.Architecture}) {
return nil, nil, nil
return nil, nil, nil, nil
}

return manifestInfo, nil, nil
return manifestInfo, manifestBytes, nil, nil
case specsv1.MediaTypeImageManifest:
//TODO: platform filter?
ociImage, err := manifest.OCI1FromManifest(manifestBytes)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}
return ociImage, nil, nil
return ociImage, manifestBytes, nil, nil
case manifest.DockerV2ListMediaType:
var subManifestInfoSlice []manifest.Manifest

manifestSchemaListInfo, err := manifest.Schema2ListFromManifest(manifestBytes)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

var filteredDescriptors []manifest.Schema2ManifestDescriptor
Expand All @@ -83,14 +84,14 @@ func GenerateManifestObj(manifestBytes []byte, manifestType string, osFilterList
filteredDescriptors = append(filteredDescriptors, manifestDescriptorElem)
manifestByte, manifestType, err := i.source.GetManifest(i.ctx, &manifestDescriptorElem.Digest)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

//TODO: will the sub manifest be list-type?
subManifest, _, err := GenerateManifestObj(manifestByte, manifestType,
subManifest, _, _, err := GenerateManifestObj(manifestByte, manifestType,
archFilterList, osFilterList, i, manifestSchemaListInfo)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

if subManifest != nil {
Expand All @@ -100,21 +101,23 @@ func GenerateManifestObj(manifestBytes []byte, manifestType string, osFilterList

// no sub manifests need to transport
if len(filteredDescriptors) == 0 {
return nil, nil, nil
return nil, nil, nil, nil
}

// return a new Schema2List
if len(filteredDescriptors) != len(manifestSchemaListInfo.Manifests) {
manifestSchemaListInfo.Manifests = filteredDescriptors
}

return manifestSchemaListInfo, subManifestInfoSlice, nil
newManifestBytes, _ := manifestSchemaListInfo.Serialize()

return manifestSchemaListInfo, newManifestBytes, subManifestInfoSlice, nil
case specsv1.MediaTypeImageIndex:
var subManifestInfoSlice []manifest.Manifest

ociIndexes, err := manifest.OCI1IndexFromManifest(manifestBytes)
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

var filteredDescriptors []specsv1.Descriptor
Expand All @@ -132,32 +135,34 @@ func GenerateManifestObj(manifestBytes []byte, manifestType string, osFilterList

manifestByte, manifestType, innerErr := i.source.GetManifest(i.ctx, &descriptor.Digest)
if innerErr != nil {
return nil, nil, innerErr
return nil, nil, nil, innerErr
}

//TODO: will the sub manifest be list-type?
subManifest, _, innerErr := GenerateManifestObj(manifestByte, manifestType,
subManifest, _, _, innerErr := GenerateManifestObj(manifestByte, manifestType,
archFilterList, osFilterList, i, nil)
if innerErr != nil {
return nil, nil, err
return nil, nil, nil, err
}

subManifestInfoSlice = append(subManifestInfoSlice, subManifest.(manifest.Manifest))
}

// no sub manifests need to transport
if len(filteredDescriptors) == 0 {
return nil, nil, nil
return nil, nil, nil, nil
}

// return a new Schema2List
if len(filteredDescriptors) != len(ociIndexes.Manifests) {
ociIndexes.Manifests = filteredDescriptors
}

return ociIndexes, subManifestInfoSlice, nil
newManifestBytes, _ := ociIndexes.Serialize()

return ociIndexes, newManifestBytes, subManifestInfoSlice, nil
default:
return nil, nil, fmt.Errorf("unsupported manifest type: %v", manifestType)
return nil, nil, nil, fmt.Errorf("unsupported manifest type: %v", manifestType)
}
}

Expand Down
38 changes: 13 additions & 25 deletions pkg/sync/task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,21 @@ func (t *Task) Run() error {
}
t.Infof("Get manifest from %s/%s:%s", t.source.GetRegistry(), t.source.GetRepository(), t.source.GetTag())

if changed := t.destination.CheckManifestChanged(manifestBytes); changed {
// do nothing if manifest is not changed
t.Infof("Dest manifest %s/%s:%s is not changed, will do nothing",
t.destination.GetRegistry(), t.destination.GetRepository(), t.destination.GetTag())
return nil
}

thisManifestInfo, subManifestInfoSlice, err := GenerateManifestObj(manifestBytes, manifestType,
destManifestInfo, destManifestBytes, subManifestInfoSlice, err := GenerateManifestObj(manifestBytes, manifestType,
t.osFilterList, t.archFilterList, t.source, nil)
if err != nil {
return t.Errorf("Get manifest info from %s/%s:%s error: %v",
t.source.GetRegistry(), t.source.GetRepository(), t.source.GetTag(), err)
}

if thisManifestInfo == nil {
if changed := t.destination.CheckManifestChanged(destManifestBytes); !changed {
// do nothing if manifest is not changed
t.Infof("Dest manifest %s/%s:%s is not changed, will do nothing",
t.destination.GetRegistry(), t.destination.GetRepository(), t.destination.GetTag())
return nil
}

if destManifestInfo == nil {
t.Infof("Skip synchronization from %s/%s:%s to %s/%s:%s, mismatch of os or architecture",
t.source.GetRegistry(), t.source.GetRepository(), t.source.GetTag(),
t.destination.GetRegistry(), t.destination.GetRepository(), t.destination.GetTag())
Expand All @@ -78,7 +78,7 @@ func (t *Task) Run() error {
var blobInfos []types.BlobInfo
if len(subManifestInfoSlice) == 0 {
// non-list type image
blobInfos, err = t.source.GetBlobInfos(thisManifestInfo.(manifest.Manifest))
blobInfos, err = t.source.GetBlobInfos(destManifestInfo.(manifest.Manifest))
if err != nil {
return t.Errorf("Get blob infos from %s/%s:%s error: %v",
t.source.GetRegistry(), t.source.GetRepository(), t.source.GetTag(), err)
Expand All @@ -98,13 +98,7 @@ func (t *Task) Run() error {

// Push manifest list
if manifestType == manifest.DockerV2ListMediaType {
manifestSchemaListInfo := thisManifestInfo.(*manifest.Schema2List)

manifestBytes, err = manifestSchemaListInfo.Serialize()
if err != nil {
return err
}

manifestSchemaListInfo := destManifestInfo.(*manifest.Schema2List)
var subManifestByte []byte

// push manifest to destination
Expand All @@ -129,13 +123,7 @@ func (t *Task) Run() error {
manifestDescriptorElem.Platform.OS, manifestDescriptorElem.Platform.Architecture)
}
} else if manifestType == specsv1.MediaTypeImageIndex {
ociIndex := thisManifestInfo.(*manifest.OCI1Index)

manifestBytes, err = ociIndex.Serialize()
if err != nil {
return err
}

ociIndex := destManifestInfo.(*manifest.OCI1Index)
var subManifestByte []byte

// push manifest to destination
Expand All @@ -161,7 +149,7 @@ func (t *Task) Run() error {
}
}

if err := t.destination.PushManifest(manifestBytes); err != nil {
if err := t.destination.PushManifest(destManifestBytes); err != nil {
return t.Errorf("Put manifest to %s/%s:%s error: %v",
t.destination.GetRegistry(), t.destination.GetRepository(), t.destination.GetTag(), err)
}
Expand Down

0 comments on commit 3e702d5

Please sign in to comment.