fix(purge): enforce ago limits and improve docs (#564)

* Add validation for purge ago values that overflow

* Move ago parsing to only happen once

* update docs

Author: estebanreyl

README.md

@@ -121,14 +121,17 @@ The filter flag is used to specify the repository and a regex filter, if a tag i
 
 Examples of filters
 
-| Intention                                                                        | Flag                                  |
-|----------------------------------------------------------------------------------|---------------------------------------|
-| Untag all tags that begin with hello in app repository                           | --filter `"app:^hello.*"`             |
-| Untag tags that end with world in app repository                                 | --filter `"app:\w*world\b"`           |
-| Untag tags that include hello-world in their name in app repository              | --filter `"app:hello-world"`          |
-| Untag all tags that are older than the duration in repositories ending in /cache | --filter `".*/cache:.*"`              |
-| Untag all tags that are older than the duration in app repository                | --filter `"app:.*"`                   |
-| Untag all tags that are older than the duration in all repositories              | --filter `".*:.*"`                    |
+| Intention                                                                           | Flag                                  |
+|-------------------------------------------------------------------------------------|---------------------------------------|
+| Untag all tags that begin with hello in app repository                              | --filter `"app:^hello.*"`             |
+| Untag tags that end with world in app repository                                    | --filter `"app:\w*world\b"`           |
+| Untag tags that include hello-world in their name in app repository                 | --filter `"app:hello-world"`          |
+| Untag all tags in repositories ending in /cache                                     | --filter `".*/cache:.*"`              |
+| Untag all tags in app repository                                                    | --filter `"app:.*"`                   |
+| Untag all tags in all repositories                                                  | --filter `".*:.*"`                    |
+| Clean only untagged manifests in all repos (with --untagged)                        | --filter `".*:^$"`                    |
+| Clean only untagged manifests in app repo (with --untagged)                         | --filter `"app:^$"`                   |
+
 
 #### Ago flag
 
@@ -160,7 +163,7 @@ The following table further explains the functionality of this flag.
 | To delete all images that were last modified before 10 minutes ago            | --ago 10m   |
 | To delete all images that were last modified before 1 hour and 15 minutes ago | --ago 1h15m |
 
-The duration should be of the form \[integer\]d\[string\] where the first integer specifies the number of days and the string is in a go style duration (can be omitted)
+The duration should be of the form \[integer\]d\[string\] where the first integer specifies the number of days and the string is in a go style duration (can be omitted). The maximimum ago duration is set to 150 years but that will effectively clean nothing up as no images should be that old.
 
 ### Optional purge flags
 

cmd/acr/purge.go

@@ -65,6 +65,7 @@ var (
 // Default settings for regexp2
 const (
 	defaultRegexpMatchTimeoutSeconds int64 = 60
+	maxAgoDurationYears              int   = 150 // Maximum duration in years for --ago flag to prevent overflow
 )
 
 // purgeParameters defines the parameters that the purge command uses (including the registry name, username and password).
@@ -145,7 +146,7 @@ func newPurgeCmd(rootParams *rootParameters) *cobra.Command {
 	cmd.Flags().BoolVar(&purgeParams.untagged, "untagged", false, "If the untagged flag is set all the manifests that do not have any tags associated to them will be also purged, except if they belong to a manifest list that contains at least one tag")
 	cmd.Flags().BoolVar(&purgeParams.dryRun, "dry-run", false, "If the dry-run flag is set no manifest or tag will be deleted, the output would be the same as if they were deleted")
 	cmd.Flags().BoolVar(&purgeParams.includeLocked, "include-locked", false, "If the include-locked flag is set, locked manifests and tags (where deleteEnabled or writeEnabled is false) will be unlocked before deletion")
-	cmd.Flags().StringVar(&purgeParams.ago, "ago", "", "The tags and untagged manifests that were last updated before this duration will be deleted, the format is [number]d[string] where the first number represents an amount of days and the string is in a Go duration format (e.g. 2d3h6m selects images older than 2 days, 3 hours and 6 minutes)")
+	cmd.Flags().StringVar(&purgeParams.ago, "ago", "", "The tags and untagged manifests that were last updated before this duration will be deleted, the format is [number]d[string] where the first number represents an amount of days and the string is in a Go duration format (e.g. 2d3h6m selects images older than 2 days, 3 hours and 6 minutes). Maximum duration is capped at 150 years to prevent overflow")
 	cmd.Flags().IntVar(&purgeParams.keep, "keep", 0, "Number of latest to-be-deleted tags to keep, use this when you want to keep at least x number of latest tags that could be deleted meeting all other filter criteria")
 	cmd.Flags().StringArrayVarP(&purgeParams.filters, "filter", "f", nil, "Specify the repository and a regular expression filter for the tag name, if a tag matches the filter and is older than the duration specified in ago it will be deleted. Note: If backtracking is used in the regexp it's possible for the expression to run into an infinite loop. The default timeout is set to 1 minute for evaluation of any filter expression. Use the '--filter-timeout-seconds' option to set a different value.")
 	cmd.Flags().StringArrayVarP(&purgeParams.configs, "config", "c", nil, "Authentication config paths (e.g. C://Users/docker/config.json)")
@@ -170,16 +171,22 @@ func purge(ctx context.Context,
 	dryRun bool,
 	includeLocked bool) (deletedTagsCount int, deletedManifestsCount int, err error) {
 
+	// Parse the duration once instead of for every repository
+	agoDuration, err := parseDuration(tagDeletionSince)
+	if err != nil {
+		return 0, 0, err
+	}
+
 	// In order to print a summary of the deleted tags/manifests the counters get updated everytime a repo is purged.
 	for repoName, tagRegex := range tagFilters {
-		singleDeletedTagsCount, manifestToTagsCountMap, err := purgeTags(ctx, acrClient, repoParallelism, loginURL, repoName, tagDeletionSince, tagRegex, tagsToKeep, filterTimeout, dryRun, includeLocked)
+		singleDeletedTagsCount, manifestToTagsCountMap, err := purgeTags(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, tagRegex, tagsToKeep, filterTimeout, dryRun, includeLocked)
 		if err != nil {
 			return deletedTagsCount, deletedManifestsCount, fmt.Errorf("failed to purge tags: %w", err)
 		}
 		singleDeletedManifestsCount := 0
 		// If the untagged flag is set then also manifests are deleted.
 		if removeUtaggedManifests {
-			singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, tagDeletionSince, manifestToTagsCountMap, dryRun, includeLocked)
+			singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, manifestToTagsCountMap, dryRun, includeLocked)
 			if err != nil {
 				return deletedTagsCount, deletedManifestsCount, fmt.Errorf("failed to purge manifests: %w", err)
 			}
@@ -193,18 +200,14 @@ func purge(ctx context.Context,
 
 }
 
-// purgeTags deletes all tags that are older than the ago value and that match the tagFilter string.
-func purgeTags(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, ago string, tagFilter string, keep int, regexpMatchTimeoutSeconds int64, dryRun bool, includeLocked bool) (int, map[string]int, error) {
+// purgeTags deletes all tags that are older than the agoDuration value and that match the tagFilter string.
+func purgeTags(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, agoDuration time.Duration, tagFilter string, keep int, regexpMatchTimeoutSeconds int64, dryRun bool, includeLocked bool) (int, map[string]int, error) {
 	if dryRun {
 		fmt.Printf("Would delete tags for repository: %s\n", repoName)
 	} else {
 		fmt.Printf("Deleting tags for repository: %s\n", repoName)
 	}
 	manifestToTagsCountMap := make(map[string]int) // This map is used to keep track of how many tags would have been deleted per manifest.
-	agoDuration, err := parseDuration(ago)
-	if err != nil {
-		return -1, manifestToTagsCountMap, err
-	}
 	timeToCompare := time.Now().UTC()
 	// Since the parseDuration function returns a negative duration, it is added to the current duration in order to be able to easily compare
 	// with the LastUpdatedTime attribute a tag has.
@@ -274,14 +277,47 @@ func parseDuration(ago string) (time.Duration, error) {
 			return time.Duration(0), err
 		}
 	}
+	// Cap at maxAgoDurationYears to prevent overflow
+	const maxDays = maxAgoDurationYears * 365
+	originalDays := days
+	capped := false
+	if days > maxDays {
+		days = maxDays
+		capped = true
+		fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears)
+	}
 	// The number of days gets converted to hours.
 	duration := time.Duration(days) * 24 * time.Hour
 	if len(durationString) > 0 {
 		agoDuration, err := time.ParseDuration(durationString)
 		if err != nil {
-			return time.Duration(0), err
+			// Check if it's an overflow error from time.ParseDuration
+			if strings.Contains(err.Error(), "invalid duration") || strings.Contains(err.Error(), "overflow") {
+				// If days were already capped, just use that and ignore the overflow portion
+				if capped {
+					return (-1 * duration), nil
+				}
+				// Cap at max duration and continue
+				agoDuration = time.Duration(maxDays) * 24 * time.Hour
+				fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears)
+			} else {
+				return time.Duration(0), err
+			}
 		}
+		// Cap the additional duration to prevent overflow when adding
+		maxDuration := time.Duration(maxDays) * 24 * time.Hour
+		if agoDuration > maxDuration {
+			agoDuration = maxDuration
+			if originalDays <= maxDays && !capped {
+				// Only print warning if we haven't already printed one for days
+				fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears)
+			}
+		}
+		// Make sure the combined duration doesn't exceed max
 		duration = duration + agoDuration
+		if duration > maxDuration {
+			duration = maxDuration
+		}
 	}
 	return (-1 * duration), nil
 }
@@ -360,16 +396,12 @@ func getTagsToDelete(ctx context.Context,
 
 // purgeDanglingManifests deletes all manifests that do not have any tags associated with them.
 // except the ones that are referenced by a multiarch manifest or that have subject.
-func purgeDanglingManifests(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, tagDeletionSince string, manifestToTagsCountMap map[string]int, dryRun bool, includeLocked bool) (int, error) {
+func purgeDanglingManifests(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, agoDuration time.Duration, manifestToTagsCountMap map[string]int, dryRun bool, includeLocked bool) (int, error) {
 	if dryRun {
 		fmt.Printf("Would delete manifests for repository: %s\n", repoName)
 	} else {
 		fmt.Printf("Deleting manifests for repository: %s\n", repoName)
 	}
-	agoDuration, err := parseDuration(tagDeletionSince)
-	if err != nil {
-		return -1, err
-	}
 	timeToCompare := time.Now().UTC().Add(agoDuration)
 	// Contrary to getTagsToDelete, getManifestsToDelete gets all the Manifests at once, this was done because if there is a manifest that has no
 	// tag but is referenced by a multiarch manifest that has tags then it should not be deleted. Or if a manifest has no tag, but it has subject,