allow tagless promotions

Also tweak the unit test and e2e tests to make sure we can promote
images that don't have tags.

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()",

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{

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",
     },
 )

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 {

test-e2e/fixture/recursive/foo/manifest.yaml

@@ -18,3 +18,4 @@ images:
     - 1.0-linux_amd64
     sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd:
     - 1.0-linux_s390x
+    sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: []

test-e2e/fixture/sanity/manifest.yaml

@@ -18,3 +18,4 @@ images:
     - 1.0-linux_amd64
     sha256:2740382935148a02bf425a893d14848dd6238e405935440ce5c13b771a33f2fd:
     - 1.0-linux_s390x
+    sha256:03aec0c717de7850ee3e3165ecdf73cf1abf0bdb5b6cce04695eeb80637360f8: []

test-e2e/golden/foo/NOTAG-0

@@ -0,0 +1 @@
+foo NOTAG-0

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