Merge pull request #1599 from zregvart/issue/EC-392

Removes support for `task-bundles` in `ec track`
enterprise-contract · May 13, 2024 · 20834d4 · 20834d4
2 parents 558b3a1 + 3c727b7
commit 20834d4
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 296 deletions.
diff --git a/cmd/track/track_bundle.go b/cmd/track/track_bundle.go
@@ -52,10 +52,6 @@ func trackBundleCmd(track trackBundleFn, pullImage pullImageFn, pushImage pushIm
 		Long: hd.Doc(`
 			Record tracking information about Tekton bundles
 
-			Given one or more Tekton Bundles, categorize each as "task-bundles",
-			ignoring those that are not. Then, generate a YAML representation of
-			this categorization.
-
 			Each Tekton Bundle is expected to be a proper OCI image reference. They
 			may contain a tag, a digest, or both. If a digest is not provided, this
 			command will query the registry to determine its value. Either a tag

diff --git a/docs/modules/ROOT/pages/ec_track_bundle.adoc b/docs/modules/ROOT/pages/ec_track_bundle.adoc
@@ -4,10 +4,6 @@ Record tracking information about Tekton bundles== Synopsis
 
 Record tracking information about Tekton bundles
 
-Given one or more Tekton Bundles, categorize each as "task-bundles",
-ignoring those that are not. Then, generate a YAML representation of
-this categorization.
-
 Each Tekton Bundle is expected to be a proper OCI image reference. They
 may contain a tag, a digest, or both. If a digest is not provided, this
 command will query the registry to determine its value. Either a tag

diff --git a/features/__snapshots__/track_bundle.snap b/features/__snapshots__/track_bundle.snap
@@ -1,11 +1,6 @@
 
 [:stdout - 1]
 /-/-/-/
-task-bundles:
-  ${REGISTRY}/acceptance/bundle:
-    - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-      effective_on: "${TIMESTAMP}"
-      tag: tag
 trusted_tasks:
   oci://${REGISTRY}/acceptance/bundle:tag:
     - effective_on: "${TIMESTAMP}"
@@ -19,15 +14,6 @@ trusted_tasks:
 
 [Fresh tags:stdout - 1]
 /-/-/-/
-task-bundles:
-  ${REGISTRY}/acceptance/bundle:
-    - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-      effective_on: "${TIMESTAMP}"
-      tag: tag
-    - digest: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8
-      effective_on: "2006-01-02T15:04:05Z"
-      expires_on: "${TIMESTAMP}"
-      tag: tag
 trusted_tasks:
   oci://${REGISTRY}/acceptance/bundle:tag:
     - effective_on: "${TIMESTAMP}"
@@ -44,11 +30,6 @@ trusted_tasks:
 
 [Pipeline definition is ignored from mixed bundle:stdout - 1]
 /-/-/-/
-task-bundles:
-  ${REGISTRY}/acceptance/bundle:
-    - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-      effective_on: "${TIMESTAMP}"
-      tag: tag
 trusted_tasks:
   oci://${REGISTRY}/acceptance/bundle:tag:
     - effective_on: "${TIMESTAMP}"
@@ -85,11 +66,6 @@ trusted_tasks:
 
 [track tekton-task alias:stdout - 1]
 /-/-/-/
-task-bundles:
-  ${REGISTRY}/acceptance/bundle:
-    - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-      effective_on: "${TIMESTAMP}"
-      tag: tag
 trusted_tasks:
   git+https://github.com/redhat-appstudio/build-definitions.git//task/buildah/0.1/buildah.yaml:
     - effective_on: "${TIMESTAMP}"
@@ -119,11 +95,6 @@ trusted_tasks:
 
 [Track tekton-task alias:stdout - 1]
 /-/-/-/
-task-bundles:
-  ${REGISTRY}/acceptance/bundle:
-    - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-      effective_on: "${TIMESTAMP}"
-      tag: tag
 trusted_tasks:
   git+https://github.com/redhat-appstudio/build-definitions.git//task/buildah/0.1/buildah.yaml:
     - effective_on: "${TIMESTAMP}"

diff --git a/features/track_bundle.feature b/features/track_bundle.feature
@@ -20,11 +20,6 @@ Feature: track bundles
     Then registry image "tracked/bundle:tag" should contain a layer with
     """
     ---
-    task-bundles:
-      ${REGISTRY}/acceptance/bundle:
-        - digest: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8
-          effective_on: "${TODAY_PLUS_30_DAYS}"
-          tag: tag
     trusted_tasks:
       oci://${REGISTRY}/acceptance/bundle:tag:
         - effective_on: "${TODAY_PLUS_30_DAYS}"
@@ -43,14 +38,6 @@ Feature: track bundles
     Then registry image "tracked/bundle:tag" should contain a layer with
     """
     ---
-    task-bundles:
-      ${REGISTRY}/acceptance/bundle:
-        - digest: sha256:7af058b8a7adb24b74875411d625afbf90af6b4ed41b740606032edf1c4a0d1d
-          effective_on: "${TODAY_PLUS_30_DAYS}"
-          tag: "1.1"
-        - digest: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8
-          effective_on: "${TODAY_PLUS_30_DAYS}"
-          tag: "1.0"
     trusted_tasks:
       oci://${REGISTRY}/acceptance/bundle:1.0:
         - effective_on: "${TODAY_PLUS_30_DAYS}"
@@ -68,11 +55,6 @@ Feature: track bundles
     Then running conftest "pull oci://${REGISTRY}/tracked/bundle:tag" produces "policy/data/data/trusted_tekton_tasks.yml" containing:
     """
     ---
-    task-bundles:
-      ${REGISTRY}/acceptance/bundle:
-        - digest: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8
-          effective_on: "${TODAY_PLUS_30_DAYS}"
-          tag: tag
     trusted_tasks:
       oci://${REGISTRY}/acceptance/bundle:tag:
         - effective_on: "${TODAY_PLUS_30_DAYS}"
@@ -86,11 +68,10 @@ Feature: track bundles
       And a track bundle file named "${TMPDIR}/bundles.yaml" containing
     """
     ---
-    task-bundles:
-      ${REGISTRY}/acceptance/bundle:
-        - digest: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
-          effective_on: 2006-01-02T15:04:05Z
-          tag: tag
+    trusted_tasks:
+      oci://${REGISTRY}/acceptance/bundle:tag:
+        - effective_on: 2006-01-02T15:04:05Z
+          ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST}
     """
       And a tekton bundle image named "acceptance/bundle:tag" containing
       | Task     | task1-updated     |

diff --git a/internal/tracker/bundle_info.go b/internal/tracker/bundle_info.go
@@ -20,40 +20,31 @@ import (
 	"context"
 
 	"github.com/tektoncd/pipeline/pkg/remote/oci"
-	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/enterprise-contract/ec-cli/internal/image"
 )
 
-type bundleInfo struct {
-	ref         image.ImageReference
-	collections sets.Set[string] // Set of collection where the bundle should be tracked under.
-}
-
-// newBundleInfo returns information about the bundle, such as which collections it should
-// be added to.
-func newBundleInfo(ctx context.Context, ref image.ImageReference) (*bundleInfo, error) {
-	info := bundleInfo{ref: ref, collections: sets.New[string]()}
-
+// containsTask returns if the bundle contains a Tekton Task
+func containsTask(ctx context.Context, ref image.ImageReference) (bool, error) {
 	client := NewClient(ctx)
-	img, err := client.GetImage(ctx, info.ref.Ref())
+	img, err := client.GetImage(ctx, ref.Ref())
 	if err != nil {
-		return nil, err
+		return false, err
 	}
 
 	manifest, err := img.Manifest()
 	if err != nil {
-		return nil, err
+		return false, err
 	}
 
 	for _, layer := range manifest.Layers {
 		if kind, ok := layer.Annotations[oci.KindAnnotation]; ok {
 			switch kind {
 			case "task":
-				info.collections.Insert(taskCollection)
+				return true, nil
 			}
 		}
 	}
 
-	return &info, nil
+	return false, nil
 }
diff --git a/internal/tracker/tracker.go b/internal/tracker/tracker.go
@@ -20,20 +20,17 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"sort"
 	"strings"
 	"time"
 
 	"github.com/google/go-containerregistry/pkg/name"
 	log "github.com/sirupsen/logrus"
 	"github.com/stuart-warren/yamlfmt"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/yaml"
 
 	"github.com/enterprise-contract/ec-cli/internal/image"
 )
 
-const taskCollection = "task-bundles"
 const ociPrefix = "oci://"
 
 type taskRecord struct {
@@ -46,20 +43,8 @@ type taskRecord struct {
 	Repository string     `json:"-"`
 }
 
-type bundleRecord struct {
-	Digest      string    `json:"digest"`
-	EffectiveOn time.Time `json:"effective_on"`
-	// ExpiresOn should be omitted if there isn't a value. Not using a pointer means it will always
-	// have a value, e.g. 0001-01-01T00:00:00Z.
-	ExpiresOn  *time.Time `json:"expires_on,omitempty"`
-	Tag        string     `json:"tag"`
-	Repository string     `json:"-"`
-}
-
 type Tracker struct {
-	// TaskBundles is deprecated and will be removed in the future. Use TrustedTasks instead.
-	TaskBundles  map[string][]bundleRecord `json:"task-bundles,omitempty"`
-	TrustedTasks map[string][]taskRecord   `json:"trusted_tasks,omitempty"`
+	TrustedTasks map[string][]taskRecord `json:"trusted_tasks,omitempty"`
 }
 
 // newTracker returns a new initialized instance of Tracker. If path
@@ -83,9 +68,6 @@ func (t *Tracker) setDefaults() {
 	if t.TrustedTasks == nil {
 		t.TrustedTasks = map[string][]taskRecord{}
 	}
-	if t.TaskBundles == nil {
-		t.TaskBundles = map[string][]bundleRecord{}
-	}
 }
 
 // addTrustedTaskRecord includes the given Tekton bundle Task record in the tracker.
@@ -104,18 +86,6 @@ func (t *Tracker) addTrustedTaskRecord(prefix string, record taskRecord) {
 	}
 }
 
-// addBundleRecord includes the given bundle record to the tracker.
-func (t *Tracker) addBundleRecord(record bundleRecord) {
-	collection := t.TaskBundles
-
-	newRecords := []bundleRecord{record}
-	if _, ok := collection[record.Repository]; !ok {
-		collection[record.Repository] = newRecords
-	} else {
-		collection[record.Repository] = append(newRecords, collection[record.Repository]...)
-	}
-}
-
 // Output serializes the Tracker state as YAML
 func (t Tracker) Output() ([]byte, error) {
 	out, err := yaml.Marshal(t)
@@ -137,23 +107,6 @@ func Track(ctx context.Context, urls []string, input []byte, prune bool, freshen
 		return nil, err
 	}
 
-	if len(t.TrustedTasks) == 0 && len(t.TaskBundles) > 0 {
-		log.Debug("converting deprecated task-bundles format to trusted_tasks")
-		for repo, bundles := range t.TaskBundles {
-			for i := len(bundles) - 1; i >= 0; i-- {
-				bundle := bundles[i]
-				t.addTrustedTaskRecord(ociPrefix, taskRecord{
-					Ref:         bundle.Digest,
-					Tag:         bundle.Tag,
-					EffectiveOn: bundle.EffectiveOn,
-					ExpiresOn:   bundle.ExpiresOn,
-					Repository:  repo,
-				})
-			}
-		}
-	}
-	t.TaskBundles = map[string][]bundleRecord{}
-
 	imageUrls, gitUrls := groupUrls(urls)
 
 	if err := t.trackImageReferences(ctx, imageUrls, freshen); err != nil {
@@ -168,10 +121,6 @@ func Track(ctx context.Context, urls []string, input []byte, prune bool, freshen
 
 	t.setExpiration()
 
-	if err := t.convertToOldFormat(); err != nil {
-		return nil, err
-	}
-
 	return t.Output()
 }
 
@@ -208,12 +157,12 @@ func (t *Tracker) trackImageReferences(ctx context.Context, urls []string, fresh
 	effective_on := effectiveOn()
 	for _, ref := range refs {
 		log.Debugf("Processing bundle %q", ref.String())
-		info, err := newBundleInfo(ctx, ref)
+		hasTask, err := containsTask(ctx, ref)
 		if err != nil {
 			return err
 		}
 
-		for range sets.List(info.collections) {
+		if hasTask {
 			t.addTrustedTaskRecord(ociPrefix, taskRecord{
 				Ref:         ref.Digest,
 				Tag:         ref.Tag,
@@ -392,57 +341,6 @@ func (t *Tracker) setExpiration() {
 	}
 }
 
-func (t *Tracker) convertToOldFormat() error {
-	for group, tasks := range t.TrustedTasks {
-		repo := ociRefFromGroup(group)
-		if repo == "" {
-			// Not an OCI group
-			continue
-		}
-		for _, task := range tasks {
-			ref, err := name.NewTag(repo)
-			if err != nil {
-				return fmt.Errorf("cannot parse existing repo as a tag ref: %w", err)
-			}
-			t.addBundleRecord(bundleRecord{
-				Digest:      task.Ref,
-				Tag:         ref.TagStr(),
-				Repository:  ref.Repository.Name(),
-				EffectiveOn: task.EffectiveOn,
-				ExpiresOn:   task.ExpiresOn,
-			})
-		}
-	}
-
-	for _, bundles := range t.TaskBundles {
-		// Sort the task bundles in reverse order. The first bundle being the most recent one. The
-		// sorting function returns true if the bundle at "i" is considered newer than the bundle at
-		// "j". It is assumed that every bundle has an EffectiveOn date and a Tag, but some bundles
-		// may not have an ExpiresOn date.
-		sort.SliceStable(bundles, func(i, j int) bool {
-			if !bundles[i].EffectiveOn.Equal(bundles[j].EffectiveOn) {
-				return bundles[i].EffectiveOn.After(bundles[j].EffectiveOn)
-			}
-
-			iExpiresOn := bundles[i].ExpiresOn
-			jExpiresOn := bundles[j].ExpiresOn
-			// A missing ExpiresOn value is always considered to be newer than an explicit value.
-			// Only one defines an expiration date. "i" is newer if it is the one that is null.
-			if (iExpiresOn == nil || jExpiresOn == nil) && iExpiresOn != jExpiresOn {
-				return iExpiresOn == nil
-			}
-			if iExpiresOn != nil && jExpiresOn != nil && !iExpiresOn.Equal(*jExpiresOn) {
-				return iExpiresOn.After(*jExpiresOn)
-			}
-
-			// Records are pretty similar. Use the tag as a tie breaker to produce a stable order.
-			return bundles[i].Tag > bundles[j].Tag
-		})
-	}
-
-	return nil
-}
-
 // ociRefFromGroup returns the OCI image reference from the given group, e.g.
 // oci://registry.local/spam:latest -> registry.local/spam:latest
 // If the group does not represent an OCI image reference, an empty string is returned.