From 3dd2d2d6689a471815227ca6cd7b6bcf60d08692 Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Thu, 3 Dec 2020 00:40:08 +0200 Subject: [PATCH] Implement policy-level tag prefix matching Tag prefix matching allows the user to filter tags based on include/exclude criteria and offers the possibility to enable prefix trimming prior to policy evaluation. Signed-off-by: Aurel Canciu --- api/v1alpha1/imagepolicy_types.go | 21 ++++++ api/v1alpha1/zz_generated.deepcopy.go | 30 +++++++++ ...image.toolkit.fluxcd.io_imagepolicies.yaml | 22 +++++++ controllers/imagepolicy_controller.go | 6 +- internal/policy/alphabetical.go | 35 +++++++++- internal/policy/alphabetical_test.go | 44 ++++++++++--- internal/policy/filter.go | 46 +++++++++++++ internal/policy/filter_test.go | 64 +++++++++++++++++++ internal/policy/policer.go | 4 +- internal/policy/semver.go | 27 ++++++-- internal/policy/semver_test.go | 46 +++++++++++-- 11 files changed, 321 insertions(+), 24 deletions(-) create mode 100644 internal/policy/filter.go create mode 100644 internal/policy/filter_test.go diff --git a/api/v1alpha1/imagepolicy_types.go b/api/v1alpha1/imagepolicy_types.go index 0d46a98c..75aaeb70 100644 --- a/api/v1alpha1/imagepolicy_types.go +++ b/api/v1alpha1/imagepolicy_types.go @@ -36,6 +36,11 @@ type ImagePolicySpec struct { // selecting the most recent image // +required Policy ImagePolicyChoice `json:"policy"` + // TagPrefixMatcher enables filtering for only a subset of tags based on their + // literal prefix. If not provided, all the tags from the repository will be + // ordered and compared. + // +optional + TagPrefixMatcher *TagPrefixMatcher `json:"tagPrefxMatcher,omitempty"` } // ImagePolicyChoice is a union of all the types of policy that can be @@ -69,6 +74,22 @@ type AlphabeticalPolicy struct { Order string `json:"order,omitempty"` } +// TagPrefixMatcher enables filtering tags based on a set of defined rules +type TagPrefixMatcher struct { + // Include specifies a set of literal tag prefixes used to filter in tags + // from the given list. + // +optional + Include []string `json:"include"` + // Exclude specifies a set of literal tag prefixes used to filter out tags + // from the given list. + // +optional + Exclude []string `json:"exclude"` + // Trim instructs the policy to remove the specified include matched + // prefixes prior to running the sort evaluation. + // +optional + Trim bool `json:"trim"` +} + // ImagePolicyStatus defines the observed state of ImagePolicy type ImagePolicyStatus struct { // LatestImage gives the first in the list of images scanned by diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 1fb469b4..357f5872 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -130,6 +130,11 @@ func (in *ImagePolicySpec) DeepCopyInto(out *ImagePolicySpec) { *out = *in out.ImageRepositoryRef = in.ImageRepositoryRef in.Policy.DeepCopyInto(&out.Policy) + if in.TagPrefixMatcher != nil { + in, out := &in.TagPrefixMatcher, &out.TagPrefixMatcher + *out = new(TagPrefixMatcher) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImagePolicySpec. @@ -307,3 +312,28 @@ func (in *SemVerPolicy) DeepCopy() *SemVerPolicy { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TagPrefixMatcher) DeepCopyInto(out *TagPrefixMatcher) { + *out = *in + if in.Include != nil { + in, out := &in.Include, &out.Include + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Exclude != nil { + in, out := &in.Exclude, &out.Exclude + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TagPrefixMatcher. +func (in *TagPrefixMatcher) DeepCopy() *TagPrefixMatcher { + if in == nil { + return nil + } + out := new(TagPrefixMatcher) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml index 89e30701..6da29b51 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml @@ -81,6 +81,28 @@ spec: - range type: object type: object + tagPrefxMatcher: + description: TagPrefixMatcher enables filtering for only a subset + of tags based on their literal prefix. If not provided, all the + tags from the repository will be ordered and compared. + properties: + exclude: + description: Exclude specifies a set of literal tag prefixes used + to filter out tags from the given list. + items: + type: string + type: array + include: + description: Include specifies a set of literal tag prefixes used + to filter in tags from the given list. + items: + type: string + type: array + trim: + description: Trim instructs the policy to remove the specified + include matched prefixes prior to running the sort evaluation. + type: boolean + type: object required: - imageRepositoryRef - policy diff --git a/controllers/imagepolicy_controller.go b/controllers/imagepolicy_controller.go index 9a8cf798..bee5f0dd 100644 --- a/controllers/imagepolicy_controller.go +++ b/controllers/imagepolicy_controller.go @@ -125,8 +125,10 @@ func (r *ImagePolicyReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) var latest string if policer != nil { - tags := r.Database.Tags(repo.Status.CanonicalImageName) - latest, err = policer.Latest(tags) + tags, err := r.Database.Tags(repo.Status.CanonicalImageName) + if err == nil { + latest, err = policer.Latest(tags, pol.Spec.TagPrefixMatcher) + } } if err != nil { imagev1alpha1.SetImagePolicyReadiness( diff --git a/internal/policy/alphabetical.go b/internal/policy/alphabetical.go index d9778474..ac5ad17d 100644 --- a/internal/policy/alphabetical.go +++ b/internal/policy/alphabetical.go @@ -19,6 +19,9 @@ package policy import ( "fmt" "sort" + "strings" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1" ) const ( @@ -51,17 +54,43 @@ func NewAlphabetical(order string) (*Alphabetical, error) { } // Latest returns latest version from a provided list of strings -func (p *Alphabetical) Latest(versions []string) (string, error) { +func (p *Alphabetical) Latest(versions []string, matcher *imagev1.TagPrefixMatcher) (string, error) { if len(versions) == 0 { return "", fmt.Errorf("version list argument cannot be empty") } - sorted := sort.StringSlice(versions) + if matcher != nil { + versions = PrefixMatchFilter(versions, matcher.Include, matcher.Exclude) + } + + var sorted sort.StringSlice + if matcher != nil && matcher.Trim && len(matcher.Include) > 0 { + for _, tag := range versions { + processed := tag + for _, in := range matcher.Include { + processed = strings.TrimPrefix(processed, in) + } + sorted = append(sorted, processed) + } + } else { + sorted = versions + } + if p.Order == AlphabeticalOrderDesc { sort.Sort(sorted) } else { sort.Sort(sort.Reverse(sorted)) } - return sorted[0], nil + latest := sorted[0] + if matcher != nil && matcher.Trim && len(matcher.Include) > 0 { + for _, tag := range versions { + for _, in := range matcher.Include { + if latest == strings.TrimPrefix(tag, in) { + return tag, nil + } + } + } + } + return latest, nil } diff --git a/internal/policy/alphabetical_test.go b/internal/policy/alphabetical_test.go index 97a5de36..30525617 100644 --- a/internal/policy/alphabetical_test.go +++ b/internal/policy/alphabetical_test.go @@ -18,6 +18,8 @@ package policy import ( "testing" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1" ) func TestNewAlphabetical(t *testing.T) { @@ -63,48 +65,72 @@ func TestAlphabetical_Latest(t *testing.T) { label string order string versions []string + match *imagev1.TagPrefixMatcher expectedVersion string expectErr bool }{ { - label: "Ubuntu CalVer", + label: "With Ubuntu CalVer", versions: []string{"16.04", "16.04.1", "16.10", "20.04", "20.10"}, expectedVersion: "20.10", }, - { - label: "Ubuntu CalVer descending", + label: "With Ubuntu CalVer prefix include", + versions: []string{"16.04", "16.04.1", "16.10", "20.04", "20.10"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"16"}}, + expectedVersion: "16.10", + }, + { + label: "With Ubuntu CalVer descending", versions: []string{"16.04", "16.04.1", "16.10", "20.04", "20.10"}, order: AlphabeticalOrderDesc, expectedVersion: "16.04", }, { - label: "Ubuntu code names", + label: "With Ubuntu code names", versions: []string{"xenial", "yakkety", "zesty", "artful", "bionic"}, expectedVersion: "zesty", }, { - label: "Ubuntu code names descending", + label: "With Ubuntu code names descending", versions: []string{"xenial", "yakkety", "zesty", "artful", "bionic"}, order: AlphabeticalOrderDesc, expectedVersion: "artful", }, { - label: "Timestamps", + label: "With Timestamps", versions: []string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}, expectedVersion: "1606364286", }, { - label: "Timestamps desc", + label: "With Timestamps desc", versions: []string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}, order: AlphabeticalOrderDesc, expectedVersion: "1606234201", }, { - label: "Timestamps with prefix", + label: "With Timestamps prefix", versions: []string{"rel-1606234201", "rel-1606364286", "rel-1606334092", "rel-1606334284", "rel-1606334201"}, expectedVersion: "rel-1606364286", }, + { + label: "With prefix include", + versions: []string{"rel-1", "rel-2", "rel-3", "rel-4", "dev-5", "dev-6"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"rel-"}}, + expectedVersion: "rel-4", + }, + { + label: "With prefix exclude", + versions: []string{"rel-1", "rel-2", "rel-3", "rel-4", "dev-5", "dev-6"}, + match: &imagev1.TagPrefixMatcher{Exclude: []string{"rel-"}}, + expectedVersion: "dev-6", + }, + { + label: "With prefix include strip", + versions: []string{"rel-11", "rel-12", "rel-13", "rel-15", "dev-5", "dev-6", "ver-12", "ver-10", "gen-50"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"rel-", "ver-"}, Trim: true}, + expectedVersion: "rel-15", + }, { label: "Empty version list", versions: []string{}, @@ -119,7 +145,7 @@ func TestAlphabetical_Latest(t *testing.T) { t.Fatalf("returned unexpected error: %s", err) } - latest, err := policy.Latest(tt.versions) + latest, err := policy.Latest(tt.versions, tt.match) if tt.expectErr && err == nil { t.Fatalf("expecting error, got nil") } diff --git a/internal/policy/filter.go b/internal/policy/filter.go new file mode 100644 index 00000000..643d8c00 --- /dev/null +++ b/internal/policy/filter.go @@ -0,0 +1,46 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policy + +import "strings" + +// PrefixMatchFilter applies filtering based on the provided lists of included and +// excluded prefixes +func PrefixMatchFilter(list []string, include []string, exclude []string) []string { + var filtered []string + for _, item := range list { + // Keep by default if no include prefixes specified + var keep bool = len(include) == 0 + for _, in := range include { + if strings.HasPrefix(item, in) { + keep = true + } + } + + for _, out := range exclude { + if strings.HasPrefix(item, out) { + keep = false + } + } + + if keep { + filtered = append(filtered, item) + } + } + + return filtered +} diff --git a/internal/policy/filter_test.go b/internal/policy/filter_test.go new file mode 100644 index 00000000..265a653a --- /dev/null +++ b/internal/policy/filter_test.go @@ -0,0 +1,64 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policy + +import ( + "reflect" + "testing" +) + +func TestPrefixMatchFilter(t *testing.T) { + cases := []struct { + label string + tags []string + include []string + exclude []string + expected []string + }{ + { + label: "none", + tags: []string{"a"}, + expected: []string{"a"}, + }, + { + label: "include", + tags: []string{"ver1", "ver2", "ver3", "rel1"}, + include: []string{"ver"}, + expected: []string{"ver1", "ver2", "ver3"}, + }, + { + label: "include", + tags: []string{"ver1", "ver2", "ver3", "rel1"}, + exclude: []string{"rel"}, + expected: []string{"ver1", "ver2", "ver3"}, + }, + { + label: "include and exclude", + tags: []string{"ver1", "ver2", "rel1", "rel2", "patch1", "patch2"}, + exclude: []string{"rel", "patch"}, + expected: []string{"ver1", "ver2"}, + }, + } + for _, tt := range cases { + t.Run(tt.label, func(t *testing.T) { + r := PrefixMatchFilter(tt.tags, tt.include, tt.exclude) + if !reflect.DeepEqual(r, tt.expected) { + t.Errorf("incorrect value returned, got '%s', expected '%s'", r, tt.expected) + } + }) + } +} diff --git a/internal/policy/policer.go b/internal/policy/policer.go index 2530d7f9..d9310e08 100644 --- a/internal/policy/policer.go +++ b/internal/policy/policer.go @@ -16,7 +16,9 @@ limitations under the License. package policy +import imagev1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1" + // Policer is an interface representing a policy implementation type type Policer interface { - Latest([]string) (string, error) + Latest([]string, *imagev1.TagPrefixMatcher) (string, error) } diff --git a/internal/policy/semver.go b/internal/policy/semver.go index 0a2e12c9..3e25a424 100644 --- a/internal/policy/semver.go +++ b/internal/policy/semver.go @@ -18,8 +18,10 @@ package policy import ( "fmt" + "strings" "github.com/Masterminds/semver/v3" + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1" "github.com/fluxcd/pkg/version" ) @@ -44,21 +46,36 @@ func NewSemVer(r string) (*SemVer, error) { } // Latest returns latest version from a provided list of strings -func (p *SemVer) Latest(versions []string) (string, error) { +func (p *SemVer) Latest(versions []string, matcher *imagev1.TagPrefixMatcher) (string, error) { if len(versions) == 0 { return "", fmt.Errorf("version list argument cannot be empty") } + if matcher != nil { + versions = PrefixMatchFilter(versions, matcher.Include, matcher.Exclude) + } + var latestVersion *semver.Version - for _, ver := range versions { - if v, err := version.ParseVersion(ver); err == nil { + var result string + for _, tag := range versions { + processed := tag + if matcher != nil && matcher.Trim { + for _, in := range matcher.Include { + processed = strings.TrimPrefix(processed, in) + } + } + if v, err := version.ParseVersion(processed); err == nil { if p.constraint.Check(v) && (latestVersion == nil || v.GreaterThan(latestVersion)) { latestVersion = v } } + if latestVersion != nil && processed == latestVersion.Original() { + result = tag + } } - if latestVersion != nil { - return latestVersion.Original(), nil + + if result != "" { + return result, nil } return "", fmt.Errorf("unable to determine latest version from provided list") } diff --git a/internal/policy/semver_test.go b/internal/policy/semver_test.go index 2cdcd157..41ace734 100644 --- a/internal/policy/semver_test.go +++ b/internal/policy/semver_test.go @@ -18,6 +18,8 @@ package policy import ( "testing" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1alpha1" ) func TestNewSemVer(t *testing.T) { @@ -57,27 +59,63 @@ func TestSemVer_Latest(t *testing.T) { label string semverRange string versions []string + match *imagev1.TagPrefixMatcher expectedVersion string expectErr bool }{ { - label: "Regular", + label: "With valid format", versions: []string{"1.0.0", "1.0.0.1", "1.0.0p", "1.0.1", "1.2.0", "0.1.0"}, semverRange: "1.0.x", expectedVersion: "1.0.1", }, { - label: "Regular with prefix", + label: "With valid format prefix", versions: []string{"v1.2.3", "v1.0.0", "v0.1.0"}, semverRange: "1.0.x", expectedVersion: "v1.0.0", }, { - label: "With invalid prefix", + label: "With valid format prefix strip", + versions: []string{"v1.2.3", "v1.0.0", "v0.1.0"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"v"}, Trim: true}, + semverRange: "1.0.x", + expectedVersion: "v1.0.0", + }, + { + label: "With invalid format prefix", versions: []string{"b1.2.3", "b1.0.0", "b0.1.0"}, semverRange: "1.0.x", expectErr: true, }, + { + label: "With invalid format prefix strip", + versions: []string{"b1.2.3", "b1.0.0", "b0.1.0"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"b"}, Trim: true}, + semverRange: "1.0.x", + expectedVersion: "b1.0.0", + }, + { + label: "With invalid format prefix include", + versions: []string{"ver1.0.3", "ver1.0.0", "ver0.1.0", "dev-v1.0.2", "dev-v1.0.0"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"dev-"}}, + semverRange: "1.0.x", + expectErr: true, + }, + { + label: "With invalid format prefix include strip", + versions: []string{"ver1.0.3", "ver1.0.0", "ver0.1.0", "dev-v1.0.2", "dev-v1.0.0"}, + match: &imagev1.TagPrefixMatcher{Include: []string{"dev-"}, Trim: true}, + semverRange: "1.0.x", + expectedVersion: "dev-v1.0.2", + }, + { + label: "With valid format prefix exclude", + versions: []string{"v1.0.1", "v2.0.0", "v3.1.0", "v1.1.0"}, + match: &imagev1.TagPrefixMatcher{Exclude: []string{"v3"}}, + semverRange: ">1.0.0", + expectedVersion: "v2.0.0", + }, { label: "With empty list", versions: []string{}, @@ -99,7 +137,7 @@ func TestSemVer_Latest(t *testing.T) { t.Fatalf("returned unexpected error: %s", err) } - latest, err := policy.Latest(tt.versions) + latest, err := policy.Latest(tt.versions, tt.match) if tt.expectErr && err == nil { t.Fatalf("expecting error, got nil") }