diff --git a/lib/dockerregistry/inventory.go b/lib/dockerregistry/inventory.go index 4254890d9..253fb820d 100644 --- a/lib/dockerregistry/inventory.go +++ b/lib/dockerregistry/inventory.go @@ -256,34 +256,33 @@ func ToPromotionEdges(mfests []Manifest) map[PromotionEdge]interface{} { for _, mfest := range mfests { for _, image := range mfest.Images { for digest, tagArray := range image.Dmap { - for _, tag := range tagArray { - for _, rc := range mfest.Registries { - if rc == *mfest.srcRegistry { - continue - } - edge := PromotionEdge{ - SrcRegistry: *mfest.srcRegistry, - SrcImageTag: ImageTag{ - ImageName: image.ImageName, - Tag: tag}, - Digest: digest, - DstRegistry: rc, - } + for _, destRC := range mfest.Registries { + if destRC == *mfest.srcRegistry { + continue + } - // Renames change how edges are created. - regImgPath := RegistryImagePath(mfest.srcRegistry.Name) + "/" + RegistryImagePath(image.ImageName) - if renames, ok := mfest.renamesDenormalized[regImgPath]; ok { - if imgName, ok := renames[rc.Name]; ok { - edge.DstImageTag = ImageTag{ - ImageName: imgName, - Tag: tag} - edges[edge] = nil - continue - } + if len(tagArray) > 0 { + for _, tag := range tagArray { + edge := mkPromotionEdge( + *mfest.srcRegistry, + destRC, + image.ImageName, + digest, + tag, + mfest.renamesDenormalized) + edges[edge] = nil } - edge.DstImageTag = ImageTag{ - ImageName: image.ImageName, - Tag: tag} + } else { + // If this digest does not have any associated tags, + // still create a promotion edge for it (tagless + // promotion). + edge := mkPromotionEdge( + *mfest.srcRegistry, + destRC, + image.ImageName, + digest, + "", // No associated tag; still promote! + mfest.renamesDenormalized) edges[edge] = nil } } @@ -293,9 +292,47 @@ func ToPromotionEdges(mfests []Manifest) map[PromotionEdge]interface{} { return edges } +func mkPromotionEdge( + srcRC, dstRC RegistryContext, + srcImageName ImageName, + digest Digest, + tag Tag, + rd RenamesDenormalized) PromotionEdge { + + edge := PromotionEdge{ + SrcRegistry: srcRC, + SrcImageTag: ImageTag{ + ImageName: srcImageName, + Tag: tag}, + Digest: digest, + DstRegistry: dstRC, + } + + // Renames change how edges are created. + // nolint[lll] + regImgPath := RegistryImagePath(srcRC.Name) + "/" + RegistryImagePath(srcImageName) + if renames, ok := rd[regImgPath]; ok { + if imgName, ok := renames[dstRC.Name]; ok { + edge.DstImageTag = ImageTag{ + ImageName: imgName, + Tag: tag} + return edge + } + } + + // Without renames, the name in the destination is the same as the name in + // the source. + edge.DstImageTag = ImageTag{ + ImageName: srcImageName, + Tag: tag} + return edge +} + // This filters out those edges from ToPromotionEdges (found in []Manifest), to // only those PromotionEdges that makes sense to keep around. For example, we // want to remove all edges that have already been promoted. +// +// nolint[gocyclo] func (sc *SyncContext) getPromotionCandidates( edges map[PromotionEdge]interface{}) map[PromotionEdge]interface{} { @@ -323,6 +360,16 @@ func (sc *SyncContext) getPromotionCandidates( continue } + // If this edge is for a tagless promotion, skip if the digest exists in + // the destination. + if edge.DstImageTag.Tag == "" && dp.DigestExists { + // Still, log a warning if the source is missing the image. + if !sp.DigestExists { + klog.Errorf("edge %s: skipping %s/%s@%s because it was already promoted, but it is still _LOST_ (can't find it in src registry! please backfill it!)\n", edge, edge.SrcRegistry.Name, edge.SrcImageTag.ImageName, edge.Digest) + } + continue + } + // If src vertex missing, LOST && NOP. We just need the digest to exist // in src (we don't care if it points to the wrong tag). if !sp.DigestExists { @@ -1551,11 +1598,27 @@ func (sc *SyncContext) Promote( rpr := req.RequestParams.(PromotionRequest) if rpr.TagOp == Move || rpr.TagOp == Add { - err := writeImage( - ToFQIN(rpr.RegistrySrc, rpr.ImageNameSrc, rpr.Digest), - ToPQIN(rpr.RegistryDest, rpr.ImageNameDest, rpr.Tag)) - if err != nil { + srcVertex := ToFQIN(rpr.RegistrySrc, rpr.ImageNameSrc, rpr.Digest) + + var dstVertex string + + if len(rpr.Tag) > 0 { + dstVertex = ToPQIN( + rpr.RegistryDest, + rpr.ImageNameDest, + rpr.Tag) + } else { + // If there is no tag, then it is a tagless promotion. So + // the destination vertex must be referenced with a digest + // (FQIN), not a tag (PQIN). + dstVertex = ToFQIN( + rpr.RegistryDest, + rpr.ImageNameDest, + rpr.Digest) + } + + if err := writeImage(srcVertex, dstVertex); err != nil { klog.Error(err) errors = append(errors, Error{ Context: "running writeImage()", diff --git a/lib/dockerregistry/inventory_test.go b/lib/dockerregistry/inventory_test.go index 6c89d19a6..6be5d29c3 100644 --- a/lib/dockerregistry/inventory_test.go +++ b/lib/dockerregistry/inventory_test.go @@ -2136,6 +2136,41 @@ func TestPromotion(t *testing.T) { Digest: "sha256:000", Tag: "0.9"}: 1}, }, + { + "Promote 1 digest (tagless promotion)", + Manifest{ + Registries: registries, + Images: []Image{ + { + ImageName: "a", + Dmap: DigestTags{ + "sha256:000": TagSlice{}}}}, + srcRegistry: &srcRC}, + SyncContext{ + Inv: MasterInventory{ + "gcr.io/foo": RegInvImage{ + "a": DigestTags{ + "sha256:000": TagSlice{}}}, + "gcr.io/bar": RegInvImage{ + "a": DigestTags{ + // "bar" already has it + "sha256:000": TagSlice{}}}, + "gcr.io/cat": RegInvImage{ + "c": DigestTags{ + "sha256:222": TagSlice{}}}}}, + nil, + CapturedRequests{ + PromotionRequest{ + TagOp: Add, + RegistrySrc: srcRegName, + RegistryDest: registries[2].Name, + ServiceAccount: registries[2].ServiceAccount, + ImageNameSrc: "a", + ImageNameDest: "a", + Digest: "sha256:000", + Tag: ""}: 1, + }, + }, { "NOP; dest has extra tag, but NOP because -delete-extra-tags NOT specified", Manifest{ diff --git a/test-e2e/BUILD.bazel b/test-e2e/BUILD.bazel index 39ade028a..59cbb66a1 100644 --- a/test-e2e/BUILD.bazel +++ b/test-e2e/BUILD.bazel @@ -76,6 +76,18 @@ container_image( layers = [":golden-layer-foo-1.0-linux_s390x"], ) +# foo: tagless image +container_layer( + name = "golden-layer-foo-NOTAG-0", + directory = "/golden/foo/NOTAG", + files = ["golden/foo/NOTAG-0"], +) + +container_image( + name = "golden-image-foo-NOTAG-0", + layers = [":golden-layer-foo-NOTAG-0"], +) + # bar container_layer( name = "golden-layer-bar-1.0", @@ -94,6 +106,11 @@ container_bundle( images = { "{STABLE_TEST_STAGING_IMG_REPOSITORY}/golden-foo/foo:1.0-linux_amd64": "//test-e2e:golden-image-foo-1.0-linux_amd64", "{STABLE_TEST_STAGING_IMG_REPOSITORY}/golden-foo/foo:1.0-linux_s390x": "//test-e2e:golden-image-foo-1.0-linux_s390x", + # We are forced to tag this image by Bazel in order to push it up (the + # container_push() rule will also reject images with an empty tag + # argument). In the e2e binary, we have to then strip the tag manually + # afterwards. + "{STABLE_TEST_STAGING_IMG_REPOSITORY}/golden-foo/foo:NOTAG-0": "//test-e2e:golden-image-foo-NOTAG-0", "{STABLE_TEST_STAGING_IMG_REPOSITORY}/golden-bar/bar:1.0": "//test-e2e:golden-image-bar-1.0", }, ) diff --git a/test-e2e/e2e.go b/test-e2e/e2e.go index 404e7ae61..6f9be2637 100644 --- a/test-e2e/e2e.go +++ b/test-e2e/e2e.go @@ -173,6 +173,15 @@ func testSetup(repoRoot string, t E2ETest) error { "--purge", fmt.Sprintf("%s/golden-foo/foo:1.0", pushRepo), }, + // Remove tag for tagless image. + { + "gcloud", + "container", + "images", + "untag", + "--quiet", + fmt.Sprintf("%s/golden-foo/foo:NOTAG-0", pushRepo), + }, } for _, cmd := range cmds { diff --git a/test-e2e/fixture/recursive/foo/manifest.yaml b/test-e2e/fixture/recursive/foo/manifest.yaml index 2b5406cb9..931c0fa60 100644 --- a/test-e2e/fixture/recursive/foo/manifest.yaml +++ b/test-e2e/fixture/recursive/foo/manifest.yaml @@ -18,3 +18,4 @@ images: - 1.0-linux_amd64 sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd: - 1.0-linux_s390x + sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: [] diff --git a/test-e2e/fixture/sanity/manifest.yaml b/test-e2e/fixture/sanity/manifest.yaml index 2b5406cb9..931c0fa60 100644 --- a/test-e2e/fixture/sanity/manifest.yaml +++ b/test-e2e/fixture/sanity/manifest.yaml @@ -18,3 +18,4 @@ images: - 1.0-linux_amd64 sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd: - 1.0-linux_s390x + sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: [] diff --git a/test-e2e/golden/foo/NOTAG-0 b/test-e2e/golden/foo/NOTAG-0 new file mode 100644 index 000000000..300c51c39 --- /dev/null +++ b/test-e2e/golden/foo/NOTAG-0 @@ -0,0 +1 @@ +foo NOTAG-0 diff --git a/test-e2e/tests.yaml b/test-e2e/tests.yaml index c281168a0..6f95fd87d 100644 --- a/test-e2e/tests.yaml +++ b/test-e2e/tests.yaml @@ -26,6 +26,7 @@ - 1.0-linux_amd64 sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd: - 1.0-linux_s390x + sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: [] - name: eu.gcr.io/k8s-cip-test-prod/some/subdir before: [] after: *golden-images @@ -60,6 +61,7 @@ - 1.0-linux_amd64 sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd: - 1.0-linux_s390x + sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: [] - name: eu.gcr.io/k8s-cip-test-prod/some/subdir before: [] after: *golden-images-recursive