From 77d52fe3615ec0c6076e4ae442129835a607ae07 Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Thu, 11 Feb 2021 01:56:22 +0200 Subject: [PATCH] Implement numerical ordering policy Converts the given list of tags to floats and compares them based on the ordering rule. If a tag is not convertible, it will err and fail to compute the latest version. Fixes #102 Signed-off-by: Aurel Canciu --- api/v1alpha1/imagepolicy_types.go | 14 ++ api/v1alpha1/zz_generated.deepcopy.go | 20 +++ ...image.toolkit.fluxcd.io_imagepolicies.yaml | 17 +- ...e.toolkit.fluxcd.io_imagerepositories.yaml | 2 +- internal/policy/alphabetical.go | 3 +- internal/policy/factory.go | 2 + internal/policy/numerical.go | 79 ++++++++++ internal/policy/numerical_test.go | 147 ++++++++++++++++++ 8 files changed, 281 insertions(+), 3 deletions(-) create mode 100644 internal/policy/numerical.go create mode 100644 internal/policy/numerical_test.go diff --git a/api/v1alpha1/imagepolicy_types.go b/api/v1alpha1/imagepolicy_types.go index e53ac938..7b94f483 100644 --- a/api/v1alpha1/imagepolicy_types.go +++ b/api/v1alpha1/imagepolicy_types.go @@ -52,6 +52,9 @@ type ImagePolicyChoice struct { // Alphabetical set of rules to use for alphabetical ordering of the tags. // +optional Alphabetical *AlphabeticalPolicy `json:"alphabetical,omitempty"` + // Numerical set of rules to use for numerical ordering of the tags. + // +optional + Numerical *NumericalPolicy `json:"numerical,omitempty"` } // SemVerPolicy specifices a semantic version policy. @@ -73,6 +76,17 @@ type AlphabeticalPolicy struct { Order string `json:"order,omitempty"` } +// NumericalPolicy specifices a numerical ordering policy. +type NumericalPolicy struct { + // Order specifies the sorting order of the tags. Given the integer values + // from 0 to 9 as tags, ascending order would select 9, and descending order + // would select 0. + // +kubebuilder:default:="asc" + // +kubebuilder:validation:Enum=asc;desc + // +optional + Order string `json:"order,omitempty"` +} + // TagFilter enables filtering tags based on a set of defined rules type TagFilter struct { // Pattern specifies a regular expression pattern used to filter for image diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2c065758..6d84c888 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -81,6 +81,11 @@ func (in *ImagePolicyChoice) DeepCopyInto(out *ImagePolicyChoice) { *out = new(AlphabeticalPolicy) **out = **in } + if in.Numerical != nil { + in, out := &in.Numerical, &out.Numerical + *out = new(NumericalPolicy) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImagePolicyChoice. @@ -287,6 +292,21 @@ func (in *ImageRepositoryStatus) DeepCopy() *ImageRepositoryStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NumericalPolicy) DeepCopyInto(out *NumericalPolicy) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NumericalPolicy. +func (in *NumericalPolicy) DeepCopy() *NumericalPolicy { + if in == nil { + return nil + } + out := new(NumericalPolicy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ScanResult) DeepCopyInto(out *ScanResult) { *out = *in diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml index 9f47bfbd..4f8190a2 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.1 + controller-gen.kubebuilder.io/version: v0.3.0 creationTimestamp: null name: imagepolicies.image.toolkit.fluxcd.io spec: @@ -84,6 +84,21 @@ spec: - desc type: string type: object + numerical: + description: Numerical set of rules to use for numerical ordering + of the tags. + properties: + order: + default: asc + description: Order specifies the sorting order of the tags. + Given the integer values from 0 to 9 as tags, ascending + order would select 9, and descending order would select + 0. + enum: + - asc + - desc + type: string + type: object semver: description: SemVer gives a semantic version range to check against the tags available. diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml index 7f72a01f..69762789 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.1 + controller-gen.kubebuilder.io/version: v0.3.0 creationTimestamp: null name: imagerepositories.image.toolkit.fluxcd.io spec: diff --git a/internal/policy/alphabetical.go b/internal/policy/alphabetical.go index 4b66be9d..f588f66c 100644 --- a/internal/policy/alphabetical.go +++ b/internal/policy/alphabetical.go @@ -33,7 +33,8 @@ type Alphabetical struct { Order string } -// NewAlphabetical constructs a Alphabetical object validating the provided semver constraint +// NewAlphabetical constructs a Alphabetical object validating the provided +// order argument func NewAlphabetical(order string) (*Alphabetical, error) { switch order { case "": diff --git a/internal/policy/factory.go b/internal/policy/factory.go index 88fb176b..2aa4c01b 100644 --- a/internal/policy/factory.go +++ b/internal/policy/factory.go @@ -32,6 +32,8 @@ func PolicerFromSpec(choice imagev1.ImagePolicyChoice) (Policer, error) { p, err = NewSemVer(choice.SemVer.Range) case choice.Alphabetical != nil: p, err = NewAlphabetical(strings.ToUpper(choice.Alphabetical.Order)) + case choice.Numerical != nil: + p, err = NewNumerical(strings.ToUpper(choice.Numerical.Order)) default: return nil, fmt.Errorf("given ImagePolicyChoice object is invalid") } diff --git a/internal/policy/numerical.go b/internal/policy/numerical.go new file mode 100644 index 00000000..721ef359 --- /dev/null +++ b/internal/policy/numerical.go @@ -0,0 +1,79 @@ +/* +Copyright 2021 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 ( + "fmt" + "strconv" +) + +const ( + // NumericalOrderAsc ascending order + NumericalOrderAsc = "ASC" + // NumericalOrderDesc descending order + NumericalOrderDesc = "DESC" +) + +// Numerical representes a Numerical ordering policy +type Numerical struct { + Order string +} + +// NewNumerical constructs a Numerical object validating the provided +// order argument +func NewNumerical(order string) (*Numerical, error) { + switch order { + case "": + order = NumericalOrderAsc + case NumericalOrderAsc, NumericalOrderDesc: + break + default: + return nil, fmt.Errorf("invalid order argument provided: '%s', must be one of: %s, %s", order, NumericalOrderAsc, NumericalOrderDesc) + } + + return &Numerical{ + Order: order, + }, nil +} + +// Latest returns latest version from a provided list of strings +func (p *Numerical) Latest(versions []string) (string, error) { + if len(versions) == 0 { + return "", fmt.Errorf("version list argument cannot be empty") + } + + var latest string + var pv float64 + for i, version := range versions { + cv, err := strconv.ParseFloat(version, 64) + if err != nil { + return "", fmt.Errorf("failed to parse invalid numeric value '%s'", version) + } + + switch { + case i == 0: + break // First iteration, nothing to compare + case p.Order == NumericalOrderAsc && cv < pv, p.Order == NumericalOrderDesc && cv > pv: + continue + } + + latest = version + pv = cv + } + + return latest, nil +} diff --git a/internal/policy/numerical_test.go b/internal/policy/numerical_test.go new file mode 100644 index 00000000..784e05fd --- /dev/null +++ b/internal/policy/numerical_test.go @@ -0,0 +1,147 @@ +/* +Copyright 2021 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 ( + "math/rand" + "testing" + "time" +) + +func TestNewNumerical(t *testing.T) { + cases := []struct { + label string + order string + expectErr bool + }{ + { + label: "With valid empty order", + order: "", + }, + { + label: "With valid asc order", + order: NumericalOrderAsc, + }, + { + label: "With valid desc order", + order: NumericalOrderDesc, + }, + { + label: "With invalid order", + order: "invalid", + expectErr: true, + }, + } + + for _, tt := range cases { + t.Run(tt.label, func(t *testing.T) { + _, err := NewNumerical(tt.order) + if tt.expectErr && err == nil { + t.Fatalf("expecting error, got nil") + } + if !tt.expectErr && err != nil { + t.Fatalf("returned unexpected error: %s", err) + } + }) + } +} + +func TestNumerical_Latest(t *testing.T) { + cases := []struct { + label string + order string + versions []string + expectedVersion string + expectErr bool + }{ + { + label: "With unordered list of integers ascending", + versions: shuffle([]string{"-62", "-88", "73", "72", "15", "16", "15", "29", "-33", "-91"}), + expectedVersion: "73", + }, + { + label: "With unordered list of integers descending", + versions: shuffle([]string{"5", "-8", "-78", "25", "70", "-4", "80", "92", "-20", "-24"}), + order: NumericalOrderDesc, + expectedVersion: "-78", + }, + { + label: "With unordered list of floats ascending", + versions: shuffle([]string{"47.40896403322944", "-27.8520927455902", "-27.930666514224427", "-31.352485948094568", "-50.41072694704882", "-21.962849842263736", "24.71884721436865", "-39.99177354004344", "53.47333823144817", "3.2008658570411086"}), + expectedVersion: "53.47333823144817", + }, + { + label: "With unordered list of floats descending", + versions: shuffle([]string{"-65.27202780220686", "57.82948329142309", "22.40184684363291", "-86.36934305697784", "-90.29082099756083", "-12.041712603564264", "77.70488240399305", "-38.98425003883552", "16.06867070412028", "53.735674335181216"}), + order: NumericalOrderDesc, + expectedVersion: "-90.29082099756083", + }, + { + label: "With Unix Timestamps ascending", + versions: shuffle([]string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}), + expectedVersion: "1606364286", + }, + { + label: "With Unix Timestamps descending", + versions: shuffle([]string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}), + order: NumericalOrderDesc, + expectedVersion: "1606234201", + }, + { + label: "With single value ascending", + versions: []string{"1"}, + expectedVersion: "1", + }, + { + label: "With single value descending", + versions: []string{"1"}, + order: NumericalOrderDesc, + expectedVersion: "1", + }, + { + label: "Empty version list", + versions: []string{}, + expectErr: true, + }, + } + + for _, tt := range cases { + t.Run(tt.label, func(t *testing.T) { + policy, err := NewNumerical(tt.order) + if err != nil { + t.Fatalf("returned unexpected error: %s", err) + } + latest, err := policy.Latest(tt.versions) + if tt.expectErr && err == nil { + t.Fatalf("expecting error, got nil") + } + if !tt.expectErr && err != nil { + t.Fatalf("returned unexpected error: %s", err) + } + + if latest != tt.expectedVersion { + t.Errorf("incorrect computed version returned, got '%s', expected '%s'", latest, tt.expectedVersion) + } + }) + } +} + +func shuffle(list []string) []string { + rand.Seed(time.Now().UnixNano()) + rand.Shuffle(len(list), func(i, j int) { list[i], list[j] = list[j], list[i] }) + return list +}