diff --git a/cmd/track/track_bundle.go b/cmd/track/track_bundle.go index 003d5f228..e11228231 100644 --- a/cmd/track/track_bundle.go +++ b/cmd/track/track_bundle.go @@ -62,14 +62,14 @@ func trackBundleCmd(track trackBundleFn, pullImage pullImageFn, pushImage pushIm or a digest is required. The output is meant to assist enforcement of policies that ensure the - most recent Tekton Bundle is used. As such, each entry contains an - "effective_on" date which is set to 30 days from today. This indicates - the Tekton Bundle usage should be updated within that period. - - If --prune is set, on by default, non-acceptable entries are removed. - Any entry with an effective_on date in the future, and the entry with - the most recent effective_on date *not* in the future are considered - acceptable. + most recent Tekton Bundle is used. Each entry contains an "expires_on" + date which indicates when that specific bundle version should no longer + be used. When a new entry is introduced, an expiration date is added to + the previous newest entry. + + If --prune is set, on by default, expired entries are removed. + Any entry with an expires_on date in the future (or no expires_on date) + is considered current and will not be pruned. `), Example: hd.Doc(` @@ -181,7 +181,7 @@ func trackBundleCmd(track trackBundleFn, pullImage pullImageFn, pushImage pushIm cmd.Flags().BoolVar(¶ms.freshen, "freshen", params.freshen, "resolve image tags to catch updates and use the latest image for the tag") - cmd.Flags().IntVar(¶ms.inEffectDays, "in-effect-days", params.inEffectDays, "number of days representing when the added reference becomes effective") + cmd.Flags().IntVar(¶ms.inEffectDays, "in-effect-days", params.inEffectDays, "number of days after which older bundle entries expire when a new bundle entry is added (most recent entry stays valid until replaced)") cmd.MarkFlagsOneRequired("bundle", "git", "input") diff --git a/cmd/track/track_bundle_test.go b/cmd/track/track_bundle_test.go index 73b65333b..a0fba47ec 100644 --- a/cmd/track/track_bundle_test.go +++ b/cmd/track/track_bundle_test.go @@ -25,6 +25,7 @@ import ( "testing" "github.com/spf13/afero" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/conforma/cli/cmd/root" @@ -269,6 +270,33 @@ func Test_TrackBundleCommand(t *testing.T) { } } +// TestBundleCommandHelp tests that the command help reflects the new expires_on behavior +func TestBundleCommandHelp(t *testing.T) { + trackBundleCmd := trackBundleCmd(nil, nil, nil) + + // Verify the long description mentions expires_on + assert.Contains(t, trackBundleCmd.Long, "expires_on", + "Command help should mention expires_on") + + // Verify it explains the new behavior + assert.Contains(t, trackBundleCmd.Long, "expiration date is added", + "Command help should explain that expiration dates are added") + + // Verify pruning explanation is updated + assert.Contains(t, trackBundleCmd.Long, "expired entries are removed", + "Command help should explain pruning removes expired entries") + + // Verify the in-effect-days flag is documented + foundFlag := false + trackBundleCmd.Flags().VisitAll(func(flag *pflag.Flag) { + if flag.Name == "in-effect-days" { + foundFlag = true + assert.Equal(t, "30", flag.DefValue, "Default value should be 30") + } + }) + assert.True(t, foundFlag, "in-effect-days flag should exist") +} + func TestPreRunE(t *testing.T) { cases := []struct { name string diff --git a/docs/modules/ROOT/pages/ec_track_bundle.adoc b/docs/modules/ROOT/pages/ec_track_bundle.adoc index 6bf2b069d..d186d0c85 100644 --- a/docs/modules/ROOT/pages/ec_track_bundle.adoc +++ b/docs/modules/ROOT/pages/ec_track_bundle.adoc @@ -12,14 +12,14 @@ command will query the registry to determine its value. Either a tag or a digest is required. The output is meant to assist enforcement of policies that ensure the -most recent Tekton Bundle is used. As such, each entry contains an -"effective_on" date which is set to 30 days from today. This indicates -the Tekton Bundle usage should be updated within that period. +most recent Tekton Bundle is used. Each entry contains an "expires_on" +date which indicates when that specific bundle version should no longer +be used. When a new entry is introduced, an expiration date is added to +the previous newest entry. -If --prune is set, on by default, non-acceptable entries are removed. -Any entry with an effective_on date in the future, and the entry with -the most recent effective_on date *not* in the future are considered -acceptable. +If --prune is set, on by default, expired entries are removed. +Any entry with an expires_on date in the future (or no expires_on date) +is considered current and will not be pruned. [source,shell] ---- @@ -65,7 +65,7 @@ Update existing acceptable bundles: --freshen:: resolve image tags to catch updates and use the latest image for the tag (Default: false) -g, --git:: git references to track - may be used multiple times (Default: []) -h, --help:: help for bundle (Default: false) ---in-effect-days:: number of days representing when the added reference becomes effective (Default: 30) +--in-effect-days:: number of days after which older bundle entries expire when a new bundle entry is added (most recent entry stays valid until replaced) (Default: 30) -i, --input:: existing tracking file -o, --output:: write modified tracking file to a file. Use empty string for stdout, default behavior -p, --prune:: remove entries that are no longer acceptable, i.e. a newer entry already effective exists (Default: true) diff --git a/features/__snapshots__/track_bundle.snap b/features/__snapshots__/track_bundle.snap index 47390fce6..9c6a14464 100755 --- a/features/__snapshots__/track_bundle.snap +++ b/features/__snapshots__/track_bundle.snap @@ -3,8 +3,7 @@ /-/-/-/ trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TIMESTAMP}" - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} --- @@ -16,10 +15,8 @@ trusted_tasks: /-/-/-/ trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TIMESTAMP}" - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} - - effective_on: "2006-01-02T15:04:05Z" - expires_on: "${TIMESTAMP}" + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - expires_on: "${TIMESTAMP}" ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 --- @@ -32,8 +29,7 @@ trusted_tasks: /-/-/-/ trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TIMESTAMP}" - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} --- @@ -55,8 +51,7 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://github.com/konflux-ci/build-definitions.git//task/buildah/0.1/buildah.yaml: - - effective_on: "${TIMESTAMP}" - ref: 3672a457e3e89c0591369f609eba727b8e84108f + - ref: 3672a457e3e89c0591369f609eba727b8e84108f --- @@ -68,11 +63,9 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://github.com/konflux-ci/build-definitions.git//task/buildah/0.1/buildah.yaml: - - effective_on: "${TIMESTAMP}" - ref: 3672a457e3e89c0591369f609eba727b8e84108f + - ref: 3672a457e3e89c0591369f609eba727b8e84108f oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TIMESTAMP}" - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} --- @@ -84,8 +77,7 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://forge.io/organization/repository.git//task/0.1/task.yaml: - - effective_on: "${TIMESTAMP}" - ref: f0cacc1af00d + - ref: f0cacc1af00d --- @@ -97,11 +89,9 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://github.com/konflux-ci/build-definitions.git//task/buildah/0.1/buildah.yaml: - - effective_on: "${TIMESTAMP}" - ref: 3672a457e3e89c0591369f609eba727b8e84108f + - ref: 3672a457e3e89c0591369f609eba727b8e84108f oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TIMESTAMP}" - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} --- @@ -113,10 +103,8 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://forge.io/organization/repository.git//task/0.1/task.yaml: - - effective_on: "${TIMESTAMP}" - ref: f0cacc1af00d - - effective_on: "2006-01-02T15:04:05Z" - expires_on: "${TIMESTAMP}" + - ref: f0cacc1af00d + - expires_on: "${TIMESTAMP}" ref: f0cacc1a --- @@ -129,8 +117,7 @@ trusted_tasks: /-/-/-/ trusted_tasks: git+https://forge.io/organization/repository.git//task/0.1/task.yaml: - - effective_on: "2006-01-02T15:04:05Z" - ref: f0cacc1a + - ref: f0cacc1a --- @@ -150,8 +137,7 @@ Error: expected "git+https://${GITHOST}/git/tasks.git//task.yaml" to contain the /-/-/-/ trusted_tasks: git+https://${GITHOST}/git/tasks.git//task.yaml: - - effective_on: "${TIMESTAMP}" - ref: ${LATEST_COMMIT} + - ref: ${LATEST_COMMIT} --- diff --git a/features/track_bundle.feature b/features/track_bundle.feature index 0b82e61aa..9a432617f 100644 --- a/features/track_bundle.feature +++ b/features/track_bundle.feature @@ -22,8 +22,7 @@ Feature: track bundles --- trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TODAY_PLUS_30_DAYS}" - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 + - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 """ @@ -40,11 +39,9 @@ Feature: track bundles --- trusted_tasks: oci://${REGISTRY}/acceptance/bundle:1.0: - - effective_on: "${TODAY_PLUS_30_DAYS}" - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 + - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 oci://${REGISTRY}/acceptance/bundle:1.1: - - effective_on: "${TODAY_PLUS_30_DAYS}" - ref: sha256:7af058b8a7adb24b74875411d625afbf90af6b4ed41b740606032edf1c4a0d1d + - ref: sha256:7af058b8a7adb24b74875411d625afbf90af6b4ed41b740606032edf1c4a0d1d """ @@ -57,8 +54,7 @@ Feature: track bundles --- trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: "${TODAY_PLUS_30_DAYS}" - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 + - ref: sha256:0af8c4f92f4b252b3ef0cbd712e7352196bc33a96c58b6e1d891b26e171deae8 """ @@ -70,8 +66,7 @@ Feature: track bundles --- trusted_tasks: oci://${REGISTRY}/acceptance/bundle:tag: - - effective_on: 2006-01-02T15:04:05Z - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} + - ref: sha256:${REGISTRY_acceptance/bundle:tag_DIGEST} """ And a tekton bundle image named "acceptance/bundle:tag" containing | Task | task1-updated | @@ -112,8 +107,7 @@ Feature: track bundles --- trusted_tasks: git+https://forge.io/organization/repository.git//task/0.1/task.yaml: - - effective_on: 2006-01-02T15:04:05Z - ref: f0cacc1a + - ref: f0cacc1a """ When ec command is run with "track tekton-task --input ${TMPDIR}/bundles.yaml --git git+https://forge.io/organization/repository.git//task/0.1/task.yaml@f0cacc1af00d" Then the exit status should be 0 @@ -125,8 +119,7 @@ Feature: track bundles --- trusted_tasks: git+https://forge.io/organization/repository.git//task/0.1/task.yaml: - - effective_on: 2006-01-02T15:04:05Z - ref: f0cacc1a + - ref: f0cacc1a """ When ec command is run with "track tekton-task --prune --input ${TMPDIR}/bundles.yaml --git git+https://forge.io/organization/repository.git//task/0.1/task.yaml@f0cacc1a" Then the exit status should be 0 @@ -145,8 +138,7 @@ Feature: track bundles --- trusted_tasks: git+https://${GITHOST}/git/tasks.git//task.yaml: - - effective_on: 2006-01-02T15:04:05Z - ref: f0cacc1a + - ref: f0cacc1a """ Given a git repository named "tasks" with | task.yaml | examples/task.yaml | diff --git a/internal/tracker/tracker.go b/internal/tracker/tracker.go index 71969a9b0..0173be564 100644 --- a/internal/tracker/tracker.go +++ b/internal/tracker/tracker.go @@ -34,8 +34,7 @@ import ( const ociPrefix = "oci://" type taskRecord struct { - Ref string `json:"ref"` - EffectiveOn time.Time `json:"effective_on"` + Ref string `json:"ref"` // 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"` @@ -111,20 +110,17 @@ func Track(ctx context.Context, urls []string, input []byte, prune bool, freshen imageUrls, gitUrls := groupUrls(urls) - days := oneDay * time.Duration(inEffectDays) - effectiveOn := time.Now().Add(days).UTC().Round(oneDay) - - if err := t.trackImageReferences(ctx, imageUrls, freshen, effectiveOn); err != nil { + if err := t.trackImageReferences(ctx, imageUrls, freshen); err != nil { return nil, err } - if err := t.trackGitReferences(ctx, gitUrls, freshen, effectiveOn); err != nil { + if err := t.trackGitReferences(ctx, gitUrls, freshen); err != nil { return nil, err } t.filterBundles(prune) - t.setExpiration() + t.setExpiration(inEffectDays) return t.Output() } @@ -143,7 +139,7 @@ func groupUrls(urls []string) ([]string, []string) { return imgs, gits } -func (t *Tracker) trackImageReferences(ctx context.Context, urls []string, freshen bool, effectiveOn time.Time) error { +func (t *Tracker) trackImageReferences(ctx context.Context, urls []string, freshen bool) error { refs, err := image.ParseAndResolveAll(ctx, urls, name.StrictValidation) if err != nil { return err @@ -168,10 +164,9 @@ func (t *Tracker) trackImageReferences(ctx context.Context, urls []string, fresh if hasTask { t.addTrustedTaskRecord(ociPrefix, taskRecord{ - Ref: ref.Digest, - Tag: ref.Tag, - EffectiveOn: effectiveOn, - Repository: ref.Repository, + Ref: ref.Digest, + Tag: ref.Tag, + Repository: ref.Repository, }) } } @@ -179,7 +174,7 @@ func (t *Tracker) trackImageReferences(ctx context.Context, urls []string, fresh return nil } -func (t *Tracker) trackGitReferences(ctx context.Context, urls []string, freshen bool, effectiveOn time.Time) error { +func (t *Tracker) trackGitReferences(ctx context.Context, urls []string, freshen bool) error { if freshen { log.Debug("Freshen is enabled") @@ -224,9 +219,8 @@ func (t *Tracker) trackGitReferences(ctx context.Context, urls []string, freshen } t.addTrustedTaskRecord("", taskRecord{ - Repository: fmt.Sprintf("%s//%s", repository, path), - Ref: rev, - EffectiveOn: effectiveOn, + Repository: fmt.Sprintf("%s//%s", repository, path), + Ref: rev, }) } @@ -263,9 +257,7 @@ func (t *Tracker) filterBundles(prune bool) { // filterRecords reduces the list of records by removing superfluous entries. // It removes records that have the same reference in a certain group. If prune is -// true, it skips any record that is no longer acceptable. Any record with an -// EffectiveOn date in the future, and the record with the most recent -// EffectiveOn date *not* in the future are considered acceptable. +// true, it removes records that have already expired based on their expires_on date. func filterRecords(records []taskRecord, prune bool) []taskRecord { now := time.Now().UTC() @@ -290,17 +282,10 @@ func filterRecords(records []taskRecord, prune bool) []taskRecord { var relevant []taskRecord if prune { - // skip tracks when records should start to be pruned. - skip := false for _, r := range unique { - if skip { - continue - } - relevant = append(relevant, r) - if !skip { - if now.After(r.EffectiveOn) { - skip = true - } + // Keep records that haven't expired yet, or records with no expiration date + if r.ExpiresOn == nil || now.Before(*r.ExpiresOn) { + relevant = append(relevant, r) } } } else { @@ -314,20 +299,23 @@ func filterRecords(records []taskRecord, prune bool) []taskRecord { return relevant } -// setExpiration sets the expires_on attribute on records. The expires_on value for record N is the -// effective_on value of the n-1 record. The first record on the list does not contain an expires_on -// value since there is no newer record that invalidates it in the future. -// TODO: Probably need to compute the expires_on value without requiring the "effective_on" value so -// we don't have to always require both values. But this may be required during some transition -// period. -func (t *Tracker) setExpiration() { +// setExpiration sets the expires_on attribute on records using a duration-based approach. +// Each record expires after a configurable duration based on inEffectDays from when it's added. +// The most recent record (index 0) gets no expiration date, making it the current active record. +func (t *Tracker) setExpiration(inEffectDays int) { + expirationDuration := time.Duration(inEffectDays) * oneDay // Use --in-effect-days flag value + now := time.Now().UTC().Round(oneDay) + for _, records := range t.TrustedTasks { - var expiration *time.Time for i := range records { - if expiration != nil { - records[i].ExpiresOn = expiration + if i == 0 { + // Most recent record doesn't expire + records[i].ExpiresOn = nil + } else if records[i].ExpiresOn == nil { + // Add expires_on to any record that doesn't have one already + expiresOn := now.Add(expirationDuration) + records[i].ExpiresOn = &expiresOn } - expiration = &records[i].EffectiveOn } } } diff --git a/internal/tracker/tracker_test.go b/internal/tracker/tracker_test.go index 22a4a739f..18743bfd5 100644 --- a/internal/tracker/tracker_test.go +++ b/internal/tracker/tracker_test.go @@ -71,10 +71,9 @@ var sampleHashThree = v1.Hash{ } var ( - expectedInEffectDays = 30 - expectedEffectiveOnTime = time.Now().Add(time.Duration(expectedInEffectDays) * oneDay).UTC().Round(oneDay) - expectedEffectiveOn = expectedEffectiveOnTime.Format(time.RFC3339) - expectedExpiresOn = expectedEffectiveOn + expectedInEffectDays = 30 + // setExpiration now uses the inEffectDays value for expiration duration, rounded to day boundary + expectedExpiresOn = time.Now().Add(time.Duration(expectedInEffectDays) * oneDay).UTC().Round(oneDay).Format(time.RFC3339) ) var ( @@ -106,11 +105,9 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/repo:one: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` oci://registry.com/repo:two: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` `), }, { @@ -123,11 +120,9 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` oci://registry.com/two:2.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` `), }, { @@ -139,18 +134,15 @@ func TestTrack(t *testing.T) { `--- trusted_tasks: oci://registry.com/repo:one: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `)), output: hd.Doc( `--- trusted_tasks: oci://registry.com/repo:one: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` oci://registry.com/repo:two: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` `), }, { @@ -162,18 +154,15 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `)), output: hd.Doc(` --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` oci://registry.com/two:2.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` `), }, { @@ -188,8 +177,7 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/two:2.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` `), }, { @@ -201,8 +189,7 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `), }, { @@ -214,15 +201,13 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + tomorrow + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `)), output: hd.Doc(` --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + tomorrow + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `), }, { @@ -235,20 +220,16 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + yesterday + `" + - expires_on: "` + yesterday + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + yesterday + `" + - expires_on: "` + yesterday + `" ref: ` + sampleHashTwo.String() + ` `)), output: hd.Doc(` --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` - - effective_on: "` + yesterday + `" - expires_on: "` + expectedExpiresOn + `" - ref: ` + sampleHashThree.String() + ` + - ref: ` + sampleHashOne.String() + ` `), }, { @@ -261,28 +242,25 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/mixed:0.2: - - effective_on: "` + inTwoDays + `" + - expires_on: "` + inTwoDays + `" ref: ` + sampleHashTwo.String() + ` - - effective_on: "` + inOneDay + `" + - expires_on: "` + inOneDay + `" ref: ` + sampleHashTwo.String() + ` oci://registry.com/mixed:0.3: - - effective_on: "` + inTwoDays + `" + - expires_on: "` + inTwoDays + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + inOneDay + `" + - expires_on: "` + inOneDay + `" ref: ` + sampleHashThree.String() + ` `)), output: hd.Doc(` --- trusted_tasks: oci://registry.com/mixed:0.2: - - effective_on: "` + inOneDay + `" - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashTwo.String() + ` oci://registry.com/mixed:0.3: - - effective_on: "` + inOneDay + `" - ref: ` + sampleHashThree.String() + ` + - ref: ` + sampleHashThree.String() + ` oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `), }, { @@ -295,32 +273,29 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + inFourDays + `" + - expires_on: "` + inFourDays + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + inThreeDays + `" + - expires_on: "` + inThreeDays + `" ref: ` + sampleHashTwo.String() + ` - - effective_on: "` + inTwoDays + `" + - expires_on: "` + inTwoDays + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + inOneDay + `" + - expires_on: "` + inOneDay + `" ref: ` + sampleHashTwo.String() + ` `)), + // All non-consecutive duplicates are kept, plus new record at front + // Existing expiry dates are preserved since they already have ExpiresOn set output: hd.Doc(` --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` - - effective_on: "` + inFourDays + `" - expires_on: "` + expectedExpiresOn + `" + - ref: ` + sampleHashOne.String() + ` + - expires_on: "` + inFourDays + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + inThreeDays + `" - expires_on: "` + inFourDays + `" + - expires_on: "` + inThreeDays + `" ref: ` + sampleHashTwo.String() + ` - - effective_on: "` + inTwoDays + `" - expires_on: "` + inThreeDays + `" + - expires_on: "` + inTwoDays + `" ref: ` + sampleHashThree.String() + ` - - effective_on: "` + inOneDay + `" - expires_on: "` + inTwoDays + `" + - expires_on: "` + inOneDay + `" ref: ` + sampleHashTwo.String() + ` `), }, @@ -331,17 +306,14 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + tomorrow + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `)), output: hd.Doc(` --- trusted_tasks: oci://registry.com/one:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOneUpdated.String() + ` - - effective_on: "` + tomorrow + `" - expires_on: "` + expectedExpiresOn + `" + - ref: ` + sampleHashOneUpdated.String() + ` + - expires_on: "` + expectedExpiresOn + `" ref: ` + sampleHashOne.String() + ` `), }, @@ -355,15 +327,97 @@ func TestTrack(t *testing.T) { --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` + `)), + output: hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - ref: ` + sampleHashOne.String() + ` + `), + }, + { + name: "setExpiration only updates records with nil ExpiresOn", + urls: []string{ + "registry.com/mixed:1.0@" + sampleHashOne.String(), + }, + input: []byte(hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - ref: ` + sampleHashTwo.String() + ` + - expires_on: "` + inThreeDays + `" + ref: ` + sampleHashThree.String() + ` + `)), + // New record becomes most recent (no expiry), existing record with expiry keeps it, + // previously most recent record (no expiry) gets new expiry + output: hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - ref: ` + sampleHashOne.String() + ` + - expires_on: "` + expectedExpiresOn + `" + ref: ` + sampleHashTwo.String() + ` + - expires_on: "` + inThreeDays + `" + ref: ` + sampleHashThree.String() + ` + `), + }, + { + name: "mixed nil and existing expiry dates", + urls: []string{ + "registry.com/mixed:1.0@" + sampleHashOne.String(), + }, + input: []byte(hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - ref: ` + sampleHashTwo.String() + ` + - ref: ` + sampleHashThree.String() + ` + - expires_on: "` + inTwoDays + `" + ref: sha256:existing1 + - expires_on: "` + inThreeDays + `" + ref: sha256:existing2 + `)), + // Most recent stays nil, records without expiry get new expiry, existing expiry preserved + output: hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - ref: ` + sampleHashOne.String() + ` + - expires_on: "` + expectedExpiresOn + `" + ref: ` + sampleHashTwo.String() + ` + - expires_on: "` + expectedExpiresOn + `" + ref: ` + sampleHashThree.String() + ` + - expires_on: "` + inTwoDays + `" + ref: sha256:existing1 + - expires_on: "` + inThreeDays + `" + ref: sha256:existing2 + `), + }, + { + name: "all records already have expiry dates", + urls: []string{ + "registry.com/mixed:1.0@" + sampleHashOne.String(), + }, + input: []byte(hd.Doc(` + --- + trusted_tasks: + oci://registry.com/mixed:1.0: + - expires_on: "` + inOneDay + `" + ref: ` + sampleHashTwo.String() + ` + - expires_on: "` + inTwoDays + `" + ref: ` + sampleHashThree.String() + ` `)), + // New record becomes most recent (no expiry), all existing expiry dates preserved output: hd.Doc(` --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` + - expires_on: "` + inOneDay + `" + ref: ` + sampleHashTwo.String() + ` + - expires_on: "` + inTwoDays + `" + ref: ` + sampleHashThree.String() + ` `), }, } @@ -549,18 +603,16 @@ func TestTrackGitReferences(t *testing.T) { require.NoError(t, tracker.trackGitReferences(context.Background(), []string{ "git+https://git.io/organization/repository//task1.yaml@rev1", "git+ssh://got.io/organization/repository//dir/task2.yaml@rev2", - }, false, expectedEffectiveOnTime)) + }, false)) expected := map[string][]taskRecord{ "git+https://git.io/organization/repository//task1.yaml": {{ - Ref: "rev1", - Repository: "git+https://git.io/organization/repository//task1.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "rev1", + Repository: "git+https://git.io/organization/repository//task1.yaml", }}, "git+ssh://got.io/organization/repository//dir/task2.yaml": {{ - Ref: "rev2", - Repository: "git+ssh://got.io/organization/repository//dir/task2.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "rev2", + Repository: "git+ssh://got.io/organization/repository//dir/task2.yaml", }}, } @@ -593,18 +645,16 @@ func TestTrackGitReferencesWithoutCommitId(t *testing.T) { require.NoError(t, tracker.trackGitReferences(ctx, []string{ "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", - }, true, expectedEffectiveOnTime)) + }, true)) expected := map[string][]taskRecord{ "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml": {{ - Ref: "0916963bac30ea708c0ded4dd9d160fc148fd46f", - Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "0916963bac30ea708c0ded4dd9d160fc148fd46f", + Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", }}, "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml": {{ - Ref: "acf3f1907b51c0e15809a61536bba71809daec68", - Repository: "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "acf3f1907b51c0e15809a61536bba71809daec68", + Repository: "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", }}, } @@ -622,9 +672,8 @@ func TestTrackGitReferencesWithoutFreshen(t *testing.T) { tracker := &Tracker{ TrustedTasks: map[string][]taskRecord{ "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml": {{ - Ref: "f0cacc1a", - Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "f0cacc1a", + Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", }}, }, } @@ -647,22 +696,19 @@ func TestTrackGitReferencesWithoutFreshen(t *testing.T) { require.NoError(t, tracker.trackGitReferences(ctx, []string{ "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", - }, true, expectedEffectiveOnTime)) + }, true)) expected := map[string][]taskRecord{ "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml": {{ - Ref: "0916963bac30ea708c0ded4dd9d160fc148fd46f", - Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "0916963bac30ea708c0ded4dd9d160fc148fd46f", + Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", }, { - Ref: "f0cacc1a", - Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "f0cacc1a", + Repository: "git+test://git.io/repository/.git//tasks/task1/0.1/task.yaml", }}, "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml": {{ - Ref: "acf3f1907b51c0e15809a61536bba71809daec68", - Repository: "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", - EffectiveOn: expectedEffectiveOnTime, + Ref: "acf3f1907b51c0e15809a61536bba71809daec68", + Repository: "git+test://git.io/repository/.git//tasks/task2/0.2/task.yaml", }}, } @@ -682,22 +728,117 @@ func TestInEffectDays(t *testing.T) { client := fakeClient{objects: testObjects, images: testImages} ctx = WithClient(ctx, client) + // Test that inEffectDays parameter is now used for expires_on calculation inEffectDays := 666 - expectedEffectiveOn := time.Now().Add(time.Duration(inEffectDays) * oneDay).UTC().Round(oneDay).Format(time.RFC3339) urls := []string{ "registry.com/mixed:1.0@" + sampleHashOne.String(), } + // Expected: inEffectDays is used for expiration, new records have no expiration expected := hd.Doc(` --- trusted_tasks: oci://registry.com/mixed:1.0: - - effective_on: "` + expectedEffectiveOn + `" - ref: ` + sampleHashOne.String() + ` + - ref: ` + sampleHashOne.String() + ` `) output, err := Track(ctx, urls, nil, true, false, inEffectDays) require.NoError(t, err) require.Equal(t, expected, string(output)) } + +func TestSetExpiration(t *testing.T) { + tests := []struct { + name string + inEffectDays int + inputRecords []taskRecord + expectedOutput []taskRecord + }{ + { + name: "most recent record gets no expiry, nil records get expiry", + inEffectDays: 7, + inputRecords: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, + {Ref: "older", ExpiresOn: nil}, + }, + expectedOutput: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, // Most recent - no expiry + {Ref: "older", ExpiresOn: &[]time.Time{time.Now().UTC().Round(oneDay).Add(7 * oneDay)}[0]}, // Gets expiry + }, + }, + { + name: "preserve existing expiry dates", + inEffectDays: 7, + inputRecords: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, + {Ref: "older", ExpiresOn: &[]time.Time{time.Now().UTC().Add(10 * oneDay)}[0]}, // Existing expiry + }, + expectedOutput: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, // Most recent - no expiry + {Ref: "older", ExpiresOn: &[]time.Time{time.Now().UTC().Add(10 * oneDay)}[0]}, // Keeps existing expiry + }, + }, + { + name: "mixed nil and existing expiry records", + inEffectDays: 5, + inputRecords: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, + {Ref: "nilRecord", ExpiresOn: nil}, + {Ref: "existingRecord", ExpiresOn: &[]time.Time{time.Now().UTC().Add(15 * oneDay)}[0]}, + {Ref: "anotherNil", ExpiresOn: nil}, + }, + expectedOutput: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, // Most recent - no expiry + {Ref: "nilRecord", ExpiresOn: &[]time.Time{time.Now().UTC().Round(oneDay).Add(5 * oneDay)}[0]}, // Gets new expiry + {Ref: "existingRecord", ExpiresOn: &[]time.Time{time.Now().UTC().Add(15 * oneDay)}[0]}, // Keeps existing + {Ref: "anotherNil", ExpiresOn: &[]time.Time{time.Now().UTC().Round(oneDay).Add(5 * oneDay)}[0]}, // Gets new expiry + }, + }, + { + name: "all records already have expiry", + inEffectDays: 3, + inputRecords: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, + {Ref: "existing1", ExpiresOn: &[]time.Time{time.Now().UTC().Add(8 * oneDay)}[0]}, + {Ref: "existing2", ExpiresOn: &[]time.Time{time.Now().UTC().Add(12 * oneDay)}[0]}, + }, + expectedOutput: []taskRecord{ + {Ref: "newest", ExpiresOn: nil}, // Most recent - no expiry + {Ref: "existing1", ExpiresOn: &[]time.Time{time.Now().UTC().Add(8 * oneDay)}[0]}, // Keeps existing + {Ref: "existing2", ExpiresOn: &[]time.Time{time.Now().UTC().Add(12 * oneDay)}[0]}, // Keeps existing + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tracker := &Tracker{ + TrustedTasks: map[string][]taskRecord{ + "test-group": tt.inputRecords, + }, + } + + tracker.setExpiration(tt.inEffectDays) + + actualRecords := tracker.TrustedTasks["test-group"] + require.Len(t, actualRecords, len(tt.expectedOutput)) + + for i, expectedRecord := range tt.expectedOutput { + actualRecord := actualRecords[i] + assert.Equal(t, expectedRecord.Ref, actualRecord.Ref) + + if expectedRecord.ExpiresOn == nil { + assert.Nil(t, actualRecord.ExpiresOn, "Record %d should have nil ExpiresOn", i) + } else { + require.NotNil(t, actualRecord.ExpiresOn, "Record %d should have non-nil ExpiresOn", i) + // Allow some tolerance for time comparison (within 1 second) + timeDiff := actualRecord.ExpiresOn.Sub(*expectedRecord.ExpiresOn) + assert.True(t, timeDiff >= -time.Second && timeDiff <= time.Second, + "Record %d expiry time should be close to expected. Expected: %v, Actual: %v, Diff: %v", + i, *expectedRecord.ExpiresOn, *actualRecord.ExpiresOn, timeDiff) + } + } + }) + } +}