Remove deprecated fields tag_filter, allow_tagged, and repo (#93)

These fields have been deprecated for a long time. This removes them in preparation for a 1.0 release.

Author: sethvargo

README.md

@@ -98,18 +98,6 @@ The payload is expected to be JSON with the following fields:
     and create a dedicated service account that has granular permissions on a
     subset of repositories.
 
-- `tag_filter` (_Deprecated_) - This option is deprecated and only exists to
-  maintain backwards compatibility with some existing broken behavior. You
-  should not use it. If specified, any image where **the first tag** matches
-  this given regular expression will be deleted. The image will not be deleted
-  if other tags match the regular expression. The regular expressions are parsed
-  according to the [Go regexp package][go-re].
-
-- `allow_tagged` (_Deprecated_) - This option is deprecated and has no effect.
-  By default, GCR Cleaner will not delete tagged images. To delete tagged
-  images, specify `tag_filter_any` or `tag_filter_all`. Specifying either of
-  these will enable deletion by tag.
-
 
 ## Permissions
 

cmd/gcr-cleaner-cli/main.go

@@ -51,11 +51,6 @@ var (
 	tagFilterAll = flag.String("tag-filter-all", "", "Delete images where all tags match this regular expression")
 	keepPtr      = flag.Int("keep", 0, "Minimum to keep")
 	dryRunPtr    = flag.Bool("dry-run", false, "Do a noop on delete api call")
-
-	// tagFilterPtr and allow-tagged are deprecated
-	// TODO(sethvargo): remove before 1.0.0
-	allowTaggedPtr    = flag.Bool("allow-tagged", false, "DEPRECATED: Delete tagged images")
-	tagFilterFirstPtr = flag.String("tag-filter", "", "DEPRECATED: Tags pattern to clean")
 )
 
 func main() {
@@ -115,14 +110,7 @@ func realMain(ctx context.Context, logger *gcrcleaner.Logger) error {
 	}
 	sort.Strings(repos)
 
-	if *allowTaggedPtr {
-		fmt.Fprintf(stderr, "DEPRECATION: -allow-tagged is deprecated, specifying any tags will enable deleting of tagged images\n")
-	}
-	if *tagFilterFirstPtr != "" {
-		fmt.Fprintf(stderr, "DEPRECATION: -tag-filter is deprecated, use -tag-filter-any or -tag-filter-all instead\n")
-	}
-
-	tagFilter, err := gcrcleaner.BuildTagFilter(*tagFilterFirstPtr, *tagFilterAny, *tagFilterAll)
+	tagFilter, err := gcrcleaner.BuildTagFilter(*tagFilterAny, *tagFilterAll)
 	if err != nil {
 		return fmt.Errorf("failed to parse tag filter: %w", err)
 	}

pkg/gcrcleaner/filter.go

@@ -29,19 +29,13 @@ type TagFilter interface {
 // BuildTagFilter builds and compiles a new tag filter for the given inputs. All
 // inputs are strings to be compiled to regular expressions and are mutually
 // exclusive.
-func BuildTagFilter(first, any, all string) (TagFilter, error) {
+func BuildTagFilter(any, all string) (TagFilter, error) {
 	// Ensure only one tag filter type is given.
-	if (first != "" && any != "") || (first != "" && all != "") || (any != "" && all != "") {
+	if any != "" && all != "" {
 		return nil, fmt.Errorf("only one tag filter type may be specified")
 	}
 
 	switch {
-	case first != "":
-		re, err := regexp.Compile(first)
-		if err != nil {
-			return nil, fmt.Errorf("failed to compile tag_filter regular expression %q: %w", first, err)
-		}
-		return &TagFilterFirst{re}, nil
 	case any != "":
 		re, err := regexp.Compile(any)
 		if err != nil {
@@ -56,7 +50,7 @@ func BuildTagFilter(first, any, all string) (TagFilter, error) {
 		return &TagFilterAll{re}, nil
 	default:
 		// If no filters were provided, return the null filter which just returns
-		// false for all matches. This preserves the "allow_tagged" behavior.
+		// false for all matches.
 		return &TagFilterNull{}, nil
 	}
 }
@@ -74,27 +68,6 @@ func (f *TagFilterNull) Name() string {
 	return "(none)"
 }
 
-var _ TagFilter = (*TagFilterFirst)(nil)
-
-// TagFilterFirst filters based on the first item in the list. If the list is
-// empty or if the first item does not match the regex, it returns false.
-type TagFilterFirst struct {
-	re *regexp.Regexp
-}
-
-func (f *TagFilterFirst) Name() string {
-	return fmt.Sprintf("first(%s)", f.re.String())
-}
-
-func (f *TagFilterFirst) Matches(tags []string) bool {
-	if len(tags) < 1 || f.re == nil {
-		return false
-	}
-	return f.re.MatchString(tags[0])
-}
-
-var _ TagFilter = (*TagFilterAny)(nil)
-
 // TagFilterAny filters based on the entire list. If any tag in the list
 // matches, it returns true. If no tags match, it returns false.
 type TagFilterAny struct {

pkg/gcrcleaner/filter_test.go

@@ -24,59 +24,34 @@ func TestBuildTagFilter(t *testing.T) {
 	t.Parallel()
 
 	cases := []struct {
-		name            string
-		first, any, all string
-		err             bool
-		exp             reflect.Type
+		name     string
+		any, all string
+		err      bool
+		exp      reflect.Type
 	}{
 		{
-			name:  "empty",
-			first: "",
-			any:   "",
-			all:   "",
-			exp:   reflect.TypeOf(&TagFilterNull{}),
+			name: "empty",
+			any:  "",
+			all:  "",
+			exp:  reflect.TypeOf(&TagFilterNull{}),
 		},
 		{
-			name:  "first_any",
-			first: "a",
-			any:   "b",
-			all:   "",
-			err:   true,
+			name: "any_all",
+			any:  "b",
+			all:  "c",
+			err:  true,
 		},
 		{
-			name:  "first_all",
-			first: "a",
-			any:   "",
-			all:   "c",
-			err:   true,
+			name: "any",
+			any:  "a",
+			all:  "",
+			exp:  reflect.TypeOf(&TagFilterAny{}),
 		},
 		{
-			name:  "any_all",
-			first: "",
-			any:   "b",
-			all:   "c",
-			err:   true,
-		},
-		{
-			name:  "first",
-			first: "a",
-			any:   "",
-			all:   "",
-			exp:   reflect.TypeOf(&TagFilterFirst{}),
-		},
-		{
-			name:  "any",
-			first: "",
-			any:   "a",
-			all:   "",
-			exp:   reflect.TypeOf(&TagFilterAny{}),
-		},
-		{
-			name:  "all",
-			first: "",
-			any:   "",
-			all:   "a",
-			exp:   reflect.TypeOf(&TagFilterAll{}),
+			name: "all",
+			any:  "",
+			all:  "a",
+			exp:  reflect.TypeOf(&TagFilterAll{}),
 		},
 	}
 
@@ -86,7 +61,7 @@ func TestBuildTagFilter(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			f, err := BuildTagFilter(tc.first, tc.any, tc.all)
+			f, err := BuildTagFilter(tc.any, tc.all)
 			if (err != nil) != tc.err {
 				t.Fatal(err)
 			}
@@ -97,55 +72,6 @@ func TestBuildTagFilter(t *testing.T) {
 	}
 }
 
-func TestTagFilterFirst_Matches(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name string
-		re   *regexp.Regexp
-		tags []string
-		exp  bool
-	}{
-		{
-			name: "empty_re",
-			re:   nil,
-			tags: nil,
-			exp:  false,
-		},
-		{
-			name: "empty_tags",
-			re:   regexp.MustCompile(`.*`),
-			tags: nil,
-			exp:  false,
-		},
-		{
-			name: "matches_first",
-			re:   regexp.MustCompile(`.*`),
-			tags: []string{"tag1", "tag2"},
-			exp:  true,
-		},
-		{
-			name: "doesnt_match_second",
-			re:   regexp.MustCompile(`^tag2$`),
-			tags: []string{"tag1", "tag2"},
-			exp:  false,
-		},
-	}
-
-	for _, tc := range cases {
-		tc := tc
-
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			f := &TagFilterFirst{re: tc.re}
-			if got, want := f.Matches(tc.tags), tc.exp; got != want {
-				t.Errorf("expected %q matches %q to be %t", tc.re, tc.tags, want)
-			}
-		})
-	}
-}
-
 func TestTagFilterAny_Matches(t *testing.T) {
 	t.Parallel()
 

pkg/gcrcleaner/server.go

@@ -143,7 +143,7 @@ func (s *Server) clean(ctx context.Context, r io.ReadCloser) (map[string][]strin
 	}
 
 	since := time.Now().UTC().Add(sub)
-	tagFilter, err := BuildTagFilter(p.TagFilterFirst, p.TagFilterAny, p.TagFilterAll)
+	tagFilter, err := BuildTagFilter(p.TagFilterAny, p.TagFilterAll)
 	if err != nil {
 		return nil, http.StatusBadRequest, fmt.Errorf("failed to build tag filter: %w", err)
 	}
@@ -155,16 +155,12 @@ func (s *Server) clean(ctx context.Context, r io.ReadCloser) (map[string][]strin
 			repos = append(repos, t)
 		}
 	}
-	if t := strings.TrimSpace(p.Repo); t != "" {
-		s.logger.Warn("specifying \"repo\" is deprecated - switch to \"repos\"")
-		repos = append(repos, t)
-	}
 	if p.Recursive {
 		s.logger.Debug("gathering child repositories recursively")
 
 		allRepos, err := s.cleaner.ListChildRepositories(ctx, repos)
 		if err != nil {
-			return nil, http.StatusBadRequest, fmt.Errorf("failed to list child repositories for %q: %w", p.Repo, err)
+			return nil, http.StatusBadRequest, fmt.Errorf("failed to list child repositories: %w", err)
 		}
 		s.logger.Debug("recursively listed child repositories",
 			"in", repos,
@@ -218,11 +214,6 @@ func (s *Server) handleError(w http.ResponseWriter, err error, status int) {
 
 // Payload is the expected incoming payload format.
 type Payload struct {
-	// Repo is the name of the repo to clean.
-	//
-	// Deprecated: Use Repos instead.
-	Repo string `json:"repo"`
-
 	// Repos is the list of repositories to clean.
 	Repos sortedStringSlice `json:"repos"`
 
@@ -251,20 +242,6 @@ type Payload struct {
 
 	// Recursive enables cleaning all child repositories.
 	Recursive bool `json:"recursive"`
-
-	// TagFilterFirst is the tags pattern to be allowed removing. If specified, any
-	// images where the first tag matches the given regular expression will be
-	// deleted.
-	//
-	// Deprecated: Use tag_filter_all or tag_filter_any instead.
-	TagFilterFirst string `json:"tag_filter"`
-
-	// AllowTagged is a Boolean value determine if tagged images are allowed to be
-	// deleted.
-	//
-	// Deprecated: Use tag_filter_all or tag_filter_any instead. Setting either of
-	// these values enables deleting tagged images.
-	AllowTagged bool `json:"allow_tagged"`
 }
 
 type pubsubMessage struct {