Skip to content

Commit

Permalink
Remove deprecated fields tag_filter, allow_tagged, and repo (#93)
Browse files Browse the repository at this point in the history
These fields have been deprecated for a long time. This removes them in preparation for a 1.0 release.
  • Loading branch information
sethvargo authored Jul 20, 2022
1 parent 0ae9d25 commit 99120be
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 175 deletions.
12 changes: 0 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 1 addition & 13 deletions cmd/gcr-cleaner-cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
}
Expand Down
33 changes: 3 additions & 30 deletions pkg/gcrcleaner/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand All @@ -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 {
Expand Down
116 changes: 21 additions & 95 deletions pkg/gcrcleaner/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}),
},
}

Expand All @@ -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)
}
Expand All @@ -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()

Expand Down
27 changes: 2 additions & 25 deletions pkg/gcrcleaner/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down Expand Up @@ -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"`

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 99120be

Please sign in to comment.