diff --git a/.gitignore b/.gitignore index 7aaeea90..e488f949 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ deployment.yml .test_env.sh cmd/keel/release +cmd/keel/keel hack/deployment-norbac.yaml hack/deployment-rbac.yaml hack/deployment-norbac-helm.yaml diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 7bbfacde..eacfd240 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -35,7 +35,7 @@ func GetPolicyFromLabelsOrAnnotations(labels map[string]string, annotations map[ policyNameA, ok := getPolicyFromLabels(annotations) if ok { - return GetPolicy(policyNameA, &Options{MatchTag: getMatchTag(annotations)}) + return GetPolicy(policyNameA, &Options{MatchTag: getMatchTag(annotations), MatchPreRelease: getMatchPreRelease(annotations)}) } policyNameL, ok := getPolicyFromLabels(labels) @@ -43,12 +43,13 @@ func GetPolicyFromLabelsOrAnnotations(labels map[string]string, annotations map[ return &NilPolicy{} } - return GetPolicy(policyNameL, &Options{MatchTag: getMatchTag(labels)}) + return GetPolicy(policyNameL, &Options{MatchTag: getMatchTag(labels), MatchPreRelease: getMatchPreRelease(labels)}) } // Options - additional options when parsing policy type Options struct { - MatchTag bool + MatchTag bool + MatchPreRelease bool } // GetPolicy - policy getter used by Helm config @@ -79,7 +80,7 @@ func GetPolicy(policyName string, options *Options) Policy { switch policyName { case "all", "major", "minor", "patch": - return ParseSemverPolicy(policyName) + return ParseSemverPolicy(policyName, options.MatchPreRelease) case "force": return NewForcePolicy(options.MatchTag) case "", "never": @@ -92,16 +93,16 @@ func GetPolicy(policyName string, options *Options) Policy { } // ParseSemverPolicy - parse policy type -func ParseSemverPolicy(policy string) Policy { +func ParseSemverPolicy(policy string, matchPreRelease bool) Policy { switch policy { case "all": - return NewSemverPolicy(SemverPolicyTypeAll) + return NewSemverPolicy(SemverPolicyTypeAll, matchPreRelease) case "major": - return NewSemverPolicy(SemverPolicyTypeMajor) + return NewSemverPolicy(SemverPolicyTypeMajor, matchPreRelease) case "minor": - return NewSemverPolicy(SemverPolicyTypeMinor) + return NewSemverPolicy(SemverPolicyTypeMinor, matchPreRelease) case "patch": - return NewSemverPolicy(SemverPolicyTypePatch) + return NewSemverPolicy(SemverPolicyTypePatch, matchPreRelease) // case "force": // return PolicyTypeForce default: @@ -130,3 +131,13 @@ func getMatchTag(labels map[string]string) bool { return false } + +func getMatchPreRelease(labels map[string]string) bool { + mt, ok := labels[types.KeelMatchPreReleaseAnnotation] + if ok { + return mt == "true" + } + + // Default to true for backward compatibility + return true +} diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 477f024c..f9f9d93e 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -70,7 +70,7 @@ func TestGetPolicy(t *testing.T) { { name: "patch", args: args{policyName: "patch", options: &Options{}}, - want: NewSemverPolicy(SemverPolicyTypePatch), + want: NewSemverPolicy(SemverPolicyTypePatch, false), }, { name: "glob:foo-*", @@ -108,15 +108,34 @@ func TestGetPolicyFromLabelsOrAnnotations(t *testing.T) { labels: map[string]string{"foo": "bar"}, annotations: map[string]string{types.KeelPolicyLabel: "all"}, }, - want: NewSemverPolicy(SemverPolicyTypeAll), + want: NewSemverPolicy(SemverPolicyTypeAll, true), }, { - name: "annotations overides labels", + name: "annotations overrides labels", args: args{ - labels: map[string]string{types.KeelPolicyLabel: "patch"}, + // The "annotations overrides labels" can be quite mis-leading for end-users (here the default value of MatchPreRelease) + // is taken from the annotations section, along with the policy... + // Shouldn't we rather merge both labels and annotations, with priority given to annotation (and a warning)? + labels: map[string]string{types.KeelPolicyLabel: "patch", types.KeelMatchPreReleaseAnnotation: "false"}, annotations: map[string]string{types.KeelPolicyLabel: "all"}, }, - want: NewSemverPolicy(SemverPolicyTypeAll), + want: NewSemverPolicy(SemverPolicyTypeAll, true), + }, + { + name: "label matchPreRelease set to false", + args: args{ + labels: map[string]string{types.KeelPolicyLabel: "minor", types.KeelMatchPreReleaseAnnotation: "false"}, + annotations: map[string]string{"foo": "bar"}, + }, + want: NewSemverPolicy(SemverPolicyTypeMinor, false), + }, + { + name: "annotation matchPreRelease set to false", + args: args{ + labels: map[string]string{"foo": "bar"}, + annotations: map[string]string{types.KeelPolicyLabel: "minor", types.KeelMatchPreReleaseAnnotation: "false"}, + }, + want: NewSemverPolicy(SemverPolicyTypeMinor, false), }, } for _, tt := range tests { diff --git a/internal/policy/semver.go b/internal/policy/semver.go index a9743182..50f65cd5 100644 --- a/internal/policy/semver.go +++ b/internal/policy/semver.go @@ -41,18 +41,20 @@ func (t SemverPolicyType) String() string { } } -func NewSemverPolicy(spt SemverPolicyType) *SemverPolicy { +func NewSemverPolicy(spt SemverPolicyType, matchPreRelease bool) *SemverPolicy { return &SemverPolicy{ - spt: spt, + spt: spt, + matchPreRelease: matchPreRelease, } } type SemverPolicy struct { - spt SemverPolicyType + spt SemverPolicyType + matchPreRelease bool } func (sp *SemverPolicy) ShouldUpdate(current, new string) (bool, error) { - return shouldUpdate(sp.spt, current, new) + return shouldUpdate(sp.spt, sp.matchPreRelease, current, new) } func (sp *SemverPolicy) Name() string { @@ -61,7 +63,7 @@ func (sp *SemverPolicy) Name() string { func (sp *SemverPolicy) Type() PolicyType { return PolicyTypeSemver } -func shouldUpdate(spt SemverPolicyType, current, new string) (bool, error) { +func shouldUpdate(spt SemverPolicyType, matchPreRelease bool, current, new string) (bool, error) { if current == "latest" { return true, nil } @@ -81,7 +83,10 @@ func shouldUpdate(spt SemverPolicyType, current, new string) (bool, error) { return false, fmt.Errorf("failed to parse new version: %s", err) } - if currentVersion.Prerelease() != newVersion.Prerelease() && spt != SemverPolicyTypeAll { + // Do not enforce pre-release match when either: + // - All policy + // - matchPreRelease set to false + if currentVersion.Prerelease() != newVersion.Prerelease() && spt != SemverPolicyTypeAll && matchPreRelease { return false, nil } diff --git a/internal/policy/semver_test.go b/internal/policy/semver_test.go index 626811e3..5222050f 100644 --- a/internal/policy/semver_test.go +++ b/internal/policy/semver_test.go @@ -6,9 +6,10 @@ import ( func Test_shouldUpdate(t *testing.T) { type args struct { - spt SemverPolicyType - current string - new string + spt SemverPolicyType + current string + new string + preReleaseMatch bool } tests := []struct { name string @@ -129,9 +130,10 @@ func Test_shouldUpdate(t *testing.T) { { name: "prerelease patch increase, policy minor, no prerelease", args: args{ - current: "1.4.5", - new: "1.4.5-xx", - spt: SemverPolicyTypeMinor, + current: "1.4.5", + new: "1.4.5-xx", + spt: SemverPolicyTypeMinor, + preReleaseMatch: true, }, want: false, wantErr: false, @@ -139,9 +141,10 @@ func Test_shouldUpdate(t *testing.T) { { name: "parsed prerelease patch increase, policy minor, no prerelease", args: args{ - current: "v1.0.0", - new: "v1.0.1-metadata", - spt: SemverPolicyTypeMinor, + current: "v1.0.0", + new: "v1.0.1-metadata", + spt: SemverPolicyTypeMinor, + preReleaseMatch: true, }, want: false, wantErr: false, @@ -149,9 +152,10 @@ func Test_shouldUpdate(t *testing.T) { { name: "parsed prerelease minor increase, policy minor, both have metadata", args: args{ - current: "v1.0.0-metadata", - new: "v1.0.1-metadata", - spt: SemverPolicyTypeMinor, + current: "v1.0.0-metadata", + new: "v1.0.1-metadata", + spt: SemverPolicyTypeMinor, + preReleaseMatch: true, }, want: true, wantErr: false, @@ -159,9 +163,10 @@ func Test_shouldUpdate(t *testing.T) { { name: "prerelease patch increase, policy minor", args: args{ - current: "1.4.5-xx", - new: "1.4.6-xx", - spt: SemverPolicyTypeMinor, + current: "1.4.5-xx", + new: "1.4.6-xx", + spt: SemverPolicyTypeMinor, + preReleaseMatch: true, }, want: true, wantErr: false, @@ -169,9 +174,10 @@ func Test_shouldUpdate(t *testing.T) { { name: "patch increase, policy minor, wrong prerelease", args: args{ - current: "1.4.5-xx", - new: "1.4.6-yy", - spt: SemverPolicyTypeMinor, + current: "1.4.5-xx", + new: "1.4.6-yy", + spt: SemverPolicyTypeMinor, + preReleaseMatch: true, }, want: false, wantErr: false, @@ -186,10 +192,31 @@ func Test_shouldUpdate(t *testing.T) { want: false, wantErr: true, }, + { + name: "pre-release increase, policy All", + args: args{ + current: "1.4.5-xx", + new: "1.4.5-yy", + spt: SemverPolicyTypeAll, + }, + want: true, + wantErr: false, + }, + { + name: "pre-release increase, policy Patch, do NOT match on pre-release", + args: args{ + current: "1.4.5-xx", + new: "1.4.5-yy", + spt: SemverPolicyTypePatch, + preReleaseMatch: false, + }, + want: true, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := shouldUpdate(tt.args.spt, tt.args.current, tt.args.new) + got, err := shouldUpdate(tt.args.spt, tt.args.preReleaseMatch, tt.args.current, tt.args.new) if (err != nil) != tt.wantErr { t.Errorf("shouldUpdate() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/provider/helm/common_test.go b/provider/helm/common_test.go index 6aa674cb..905a8336 100644 --- a/provider/helm/common_test.go +++ b/provider/helm/common_test.go @@ -60,7 +60,7 @@ func Test_getImages(t *testing.T) { &types.TrackedImage{ Image: img, Trigger: types.TriggerTypePoll, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, wantErr: false, @@ -74,17 +74,17 @@ func Test_getImages(t *testing.T) { &types.TrackedImage{ Image: mustParse("quay.io/prometheus/alertmanager:v0.16.2"), Trigger: types.TriggerTypePoll, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, &types.TrackedImage{ Image: mustParse("quay.io/coreos/prometheus-operator:v0.29.0"), Trigger: types.TriggerTypePoll, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, &types.TrackedImage{ Image: mustParse("quay.io/prometheus/prometheus:v2.7.2"), Trigger: types.TriggerTypePoll, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, wantErr: false, diff --git a/provider/helm/helm.go b/provider/helm/helm.go index dae1ba7e..36d659e8 100644 --- a/provider/helm/helm.go +++ b/provider/helm/helm.go @@ -104,6 +104,7 @@ type Root struct { type KeelChartConfig struct { Policy string `json:"policy"` MatchTag bool `json:"matchTag"` + MatchPreRelease bool `json:"matchPreRelease"` Trigger types.TriggerType `json:"trigger"` PollSchedule string `json:"pollSchedule"` Approvals int `json:"approvals"` // Minimum required approvals @@ -448,6 +449,8 @@ func getKeelConfig(vals chartutil.Values) (*KeelChartConfig, error) { } var r Root + // Default MatchPreRelease to true if not present (backward compatibility) + r.Keel.MatchPreRelease = true err = yaml.Unmarshal([]byte(yamlFull), &r) if err != nil { return nil, fmt.Errorf("failed to parse keel config: %s", err) @@ -459,7 +462,7 @@ func getKeelConfig(vals chartutil.Values) (*KeelChartConfig, error) { cfg := r.Keel - cfg.Plc = policy.GetPolicy(cfg.Policy, &policy.Options{MatchTag: cfg.MatchTag}) + cfg.Plc = policy.GetPolicy(cfg.Policy, &policy.Options{MatchTag: cfg.MatchTag, MatchPreRelease: cfg.MatchPreRelease}) return &cfg, nil } diff --git a/provider/helm/helm_test.go b/provider/helm/helm_test.go index 98e1fc27..a00f7d64 100644 --- a/provider/helm/helm_test.go +++ b/provider/helm/helm_test.go @@ -560,6 +560,25 @@ keel: ` valuesPoll, _ := chartutil.ReadValues([]byte(valuesPollStr)) + var valuesNoMatchPreReleaseStr = ` +name: al Rashid +where: + city: Basrah + title: caliph +image: + repository: gcr.io/v2-namespace/hello-world + tag: 1.1.0 + +keel: + policy: all + matchPreRelease: false + images: + - repository: image.repository + tag: image.tag + +` + valuesNoMatchPreRelease, _ := chartutil.ReadValues([]byte(valuesNoMatchPreReleaseStr)) + type args struct { vals chartutil.Values } @@ -573,12 +592,13 @@ keel: name: "correct config", args: args{vals: valuesBasic}, want: &KeelChartConfig{ - Policy: "all", - Trigger: types.TriggerTypeDefault, + Policy: "all", + MatchPreRelease: true, + Trigger: types.TriggerTypeDefault, Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag"}, }, - Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, { @@ -586,25 +606,40 @@ keel: args: args{vals: valuesChannels}, want: &KeelChartConfig{ Policy: "all", + MatchPreRelease: true, Trigger: types.TriggerTypeDefault, NotificationChannels: []string{"chan1", "chan2"}, Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag"}, }, - Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, { name: "correct polling config", args: args{vals: valuesPoll}, want: &KeelChartConfig{ - Policy: "major", - Trigger: types.TriggerTypePoll, - PollSchedule: "@every 30m", + Policy: "major", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, + PollSchedule: "@every 30m", Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag", ImagePullSecret: "such-secret"}, }, - Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true), + }, + }, + { + name: "disable matchPreRelease", + args: args{vals: valuesNoMatchPreRelease}, + want: &KeelChartConfig{ + Policy: "all", + MatchPreRelease: false, + Trigger: types.TriggerTypeDefault, + Images: []ImageDetails{ + ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag"}, + }, + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, false), }, }, } diff --git a/provider/helm/updates_test.go b/provider/helm/updates_test.go index c20a7906..50cd4922 100644 --- a/provider/helm/updates_test.go +++ b/provider/helm/updates_test.go @@ -107,8 +107,9 @@ keel: CurrentVersion: "1.1.0", NewVersion: "latest", Config: &KeelChartConfig{ - Policy: "force", - Trigger: types.TriggerTypePoll, + Policy: "force", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, Images: []ImageDetails{ ImageDetails{ RepositoryPath: "image.repository", @@ -139,8 +140,9 @@ keel: NewVersion: "1.2.0", ReleaseNotes: []string{"https://github.com/keel-hq/keel/releases"}, Config: &KeelChartConfig{ - Policy: "force", - Trigger: types.TriggerTypePoll, + Policy: "force", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, Images: []ImageDetails{ ImageDetails{ RepositoryPath: "image.repository", @@ -318,12 +320,13 @@ image: NewVersion: "1.1.2", CurrentVersion: "1.1.0", Config: &KeelChartConfig{ - Policy: "all", - Trigger: types.TriggerTypePoll, + Policy: "all", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag"}, }, - Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, wantShouldUpdateRelease: true, @@ -374,8 +377,9 @@ image: NewVersion: "1.1.0", CurrentVersion: "alpha", Config: &KeelChartConfig{ - Policy: "force", - Trigger: types.TriggerTypePoll, + Policy: "force", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository", TagPath: "image.tag"}, }, @@ -417,12 +421,13 @@ image: NewVersion: "1.1.0", CurrentVersion: "1.0.0", Config: &KeelChartConfig{ - Policy: "major", - Trigger: types.TriggerTypePoll, + Policy: "major", + MatchPreRelease: true, + Trigger: types.TriggerTypePoll, Images: []ImageDetails{ ImageDetails{RepositoryPath: "image.repository"}, }, - Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), + Plc: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true), }, }, wantShouldUpdateRelease: true, diff --git a/provider/kubernetes/updates_test.go b/provider/kubernetes/updates_test.go index e1372f2e..cc18cfc3 100644 --- a/provider/kubernetes/updates_test.go +++ b/provider/kubernetes/updates_test.go @@ -11,7 +11,7 @@ import ( "github.com/keel-hq/keel/util/timeutil" apps_v1 "k8s.io/api/apps/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -674,7 +674,7 @@ func TestProvider_checkForUpdateSemver(t *testing.T) { { name: "standard version bump", args: args{ - policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.2"}, resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, @@ -740,7 +740,7 @@ func TestProvider_checkForUpdateSemver(t *testing.T) { name: "staging pre-release", args: args{ - policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMinor), + policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMinor, true), repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-prerelease", Tag: "v1.1.2-staging"}, resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, @@ -777,7 +777,7 @@ func TestProvider_checkForUpdateSemver(t *testing.T) { name: "normal new tag while there's pre-release", args: args{ - policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMinor), + policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMinor, true), repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-prerelease", Tag: "v1.1.2"}, resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, @@ -814,7 +814,7 @@ func TestProvider_checkForUpdateSemver(t *testing.T) { name: "standard ignore version bump", args: args{ - policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.1"}, resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, @@ -849,7 +849,7 @@ func TestProvider_checkForUpdateSemver(t *testing.T) { { name: "multiple containers, version bump one", args: args{ - policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.2"}, resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, diff --git a/trigger/poll/multi_tags_watcher.go b/trigger/poll/multi_tags_watcher.go index b34b891a..ac36ab00 100644 --- a/trigger/poll/multi_tags_watcher.go +++ b/trigger/poll/multi_tags_watcher.go @@ -2,13 +2,13 @@ package poll import ( "sort" + "strings" + "github.com/Masterminds/semver" "github.com/keel-hq/keel/extension/credentialshelper" - "github.com/keel-hq/keel/internal/policy" "github.com/keel-hq/keel/provider" "github.com/keel-hq/keel/registry" "github.com/keel-hq/keel/types" - "github.com/keel-hq/keel/util/version" "github.com/prometheus/client_golang/prometheus" @@ -90,32 +90,47 @@ func (j *WatchRepositoryTagsJob) computeEvents(tags []string) ([]types.Event, er events := []types.Event{} - // collapse removes all non-semver tags and only takes - // the highest versions of each prerelease + the main version that doesn't have - // any prereleases - tags = collapse(tags) + // Keep only semver tags, sorted desc (to optimize process) + versions := semverSort(tags) for _, trackedImage := range getRelatedTrackedImages(j.details.trackedImage, trackedImages) { + // Current version tag might not be a valid semver one + currentVersion, invalidCurrentVersion := semver.NewVersion(trackedImage.Image.Tag()) // matches, going through tags - for _, tag := range tags { - update, err := trackedImage.Policy.ShouldUpdate(trackedImage.Image.Tag(), tag) + for _, version := range versions { + if invalidCurrentVersion == nil && currentVersion.GreaterThan(version) { + // Current tag is a valid semver, and is bigger than currently tested one + // -> we can stop now, nothing will be worth upgrading in the rest of the sorted list + break + } + update, err := trackedImage.Policy.ShouldUpdate(trackedImage.Image.Tag(), version.Original()) + // log.WithFields(log.Fields{ + // "current_tag": j.details.trackedImage.Image.Tag(), + // "image_name": j.details.trackedImage.Image.Remote(), + // }).Debug("trigger.poll.WatchRepositoryTagsJob: tag: ", version.Original(), "; update: ", update, "; err:", err) if err != nil { continue } - if update && !exists(tag, events) { + if update && !exists(version.Original(), events) { event := types.Event{ Repository: types.Repository{ Name: j.details.trackedImage.Image.Repository(), - Tag: tag, + Tag: version.Original(), }, TriggerName: types.TriggerTypePoll.String(), } events = append(events, event) + // Only keep first match per image (should be the highest usable version) + break } } } + log.WithFields(log.Fields{ + "current_tag": j.details.trackedImage.Image.Tag(), + "image_name": j.details.trackedImage.Image.Remote(), + }).Debug("trigger.poll.WatchRepositoryTagsJob: events: ", events) return events, nil } @@ -129,41 +144,24 @@ func exists(tag string, events []types.Event) bool { return false } -// collapse gets latest available tags for main version and pre-releases -// example: -// [1.0.0, 1.5.0, 1.3.0-dev, 1.4.5-dev] would become [1.5.0, 1.4.5-dev] -func collapse(tags []string) []string { - r := map[string]string{} - p := policy.NewSemverPolicy(policy.SemverPolicyTypeAll) +// Filter and sort tags according to semver, desc +func semverSort(tags []string) []*semver.Version { + var versions []*semver.Version for _, t := range tags { - v, err := version.GetVersion(t) - // v, err := semver.NewVersion(tag) + if len(strings.SplitN(t, ".", 3)) < 3 { + // Keep only X.Y.Z+ semver + continue + } + v, err := semver.NewVersion(t) + // Filter out non semver tags if err != nil { continue } - stored, ok := r[v.PreRelease] - if !ok { - r[v.PreRelease] = t - continue - } - higher, err := p.ShouldUpdate(stored, t) - if err != nil { - continue - } - if higher { - r[v.PreRelease] = t - } + versions = append(versions, v) } - - result := []string{} - for _, tag := range r { - result = append(result, tag) - } - - // always sort, for test purposes - sort.Strings(result) - - return result + // Sort desc, following semver + sort.Slice(versions, func(i, j int) bool { return versions[j].LessThan(versions[i]) }) + return versions } func getRelatedTrackedImages(ours *types.TrackedImage, all []*types.TrackedImage) []*types.TrackedImage { diff --git a/trigger/poll/multi_tags_watcher_test.go b/trigger/poll/multi_tags_watcher_test.go index b0640d6d..cc20cf21 100644 --- a/trigger/poll/multi_tags_watcher_test.go +++ b/trigger/poll/multi_tags_watcher_test.go @@ -2,10 +2,13 @@ package poll import ( "reflect" + "strconv" "strings" "testing" + "github.com/Masterminds/semver" "github.com/keel-hq/keel/approvals" + // "github.com/keel-hq/keel/cache/memory" "github.com/keel-hq/keel/internal/policy" "github.com/keel-hq/keel/provider" @@ -23,7 +26,7 @@ func TestWatchMultipleTagsWithSemver(t *testing.T) { Trigger: types.TriggerTypePoll, Provider: "fp", PollSchedule: types.KeelPollDefaultSchedule, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, } @@ -57,16 +60,24 @@ func TestWatchMultipleTagsWithSemver(t *testing.T) { } } -func TestWatchAllTagsJobWithSemver(t *testing.T) { +type runTestCase struct { + currentTag string + expectedTag string + bumpPolicy policy.Policy +} - reference, _ := image.Parse("foo/bar:1.1.0") +// Helper function to factorize code +func testRunHelper(testCases []runTestCase, availableTags []string, t *testing.T) { + var testImages []*types.TrackedImage + for _, testCase := range testCases { + reference, _ := image.Parse("foo/bar:" + testCase.currentTag) + testImages = append(testImages, &types.TrackedImage{ + Image: reference, + Policy: testCase.bumpPolicy, + }) + } fp := &fakeProvider{ - images: []*types.TrackedImage{ - &types.TrackedImage{ - Image: reference, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), - }, - }, + images: testImages, } store, teardown := newTestingUtils() defer teardown() @@ -77,7 +88,7 @@ func TestWatchAllTagsJobWithSemver(t *testing.T) { providers := provider.New([]provider.Provider{fp}, am) frc := &fakeRegistryClient{ - tagsToReturn: []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"}, + tagsToReturn: availableTags, } details := &watchDetails{ @@ -88,247 +99,95 @@ func TestWatchAllTagsJobWithSemver(t *testing.T) { job.Run() + // Compute number of expected events (version bump expected) + var nbEvents = 0 + for _, testCase := range testCases { + if testCase.currentTag != testCase.expectedTag { + nbEvents++ + } + } // checking whether new job was submitted - - if len(fp.submitted) != 1 { + if len(fp.submitted) != nbEvents { tags := []string{} for _, s := range fp.submitted { tags = append(tags, s.Repository.Tag) } - t.Errorf("expected 1 events, got: %d [%s]", len(fp.submitted), strings.Join(tags, ", ")) - } - - submitted := fp.submitted[0] - - if submitted.Repository.Name != "index.docker.io/foo/bar" { - t.Errorf("unexpected event repository name: %s", submitted.Repository.Name) - } - - if submitted.Repository.Tag != "1.5.0" { - t.Errorf("expected event repository tag 1.5.0, but got: %s", submitted.Repository.Tag) + t.Errorf("expected "+strconv.Itoa(nbEvents)+" events, got: %d [%s]", len(fp.submitted), strings.Join(tags, ", ")) + } else { + for i, testCase := range testCases { + submitted := fp.submitted[i] + + if submitted.Repository.Name != "index.docker.io/foo/bar" { + t.Errorf("unexpected event repository name: %s", submitted.Repository.Name) + } + + if submitted.Repository.Tag != testCase.expectedTag { + t.Errorf("expected event repository tag "+testCase.expectedTag+", but got: %s", submitted.Repository.Tag) + } + } } +} +func TestWatchAllTagsJobWithSemver(t *testing.T) { + availableTags := []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"} + testCases := []runTestCase{{"1.1.0", "1.5.0", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true)}} + testRunHelper(testCases, availableTags, t) } func TestWatchAllTagsPrerelease(t *testing.T) { + availableTags := []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"} + testCases := []runTestCase{{"1.2.0-dev", "1.3.0-dev", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true)}} + testRunHelper(testCases, availableTags, t) +} - referenceB, _ := image.Parse("foo/bar:1.2.0-dev") +// Full Semver, including pre-releases +func TestWatchAllTagsFullSemver(t *testing.T) { + availableTags := []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"} + testCases := []runTestCase{{"1.2.0-dev", "1.8.0-alpha", policy.NewSemverPolicy(policy.SemverPolicyTypeMinor, false)}} + testRunHelper(testCases, availableTags, t) - fp := &fakeProvider{ - images: []*types.TrackedImage{ - &types.TrackedImage{ - Image: referenceB, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), - }, - }, - } - store, teardown := newTestingUtils() - defer teardown() - am := approvals.New(&approvals.Opts{ - Store: store, - }) + // Test simulating linuxserver tagging strategy + availableTags = []string{"v0.1.2-ls1", "v0.1.2-ls2", "v0.1.3-ls1", "v0.1.3-ls2", "v0.2.0-ls2", "v0.2.0-ls3"} + testCases = []runTestCase{{"v0.1.0-ls1", "v0.2.0-ls3", policy.NewSemverPolicy(policy.SemverPolicyTypeMinor, false)}} + testRunHelper(testCases, availableTags, t) - providers := provider.New([]provider.Provider{fp}, am) +} - frc := &fakeRegistryClient{ - tagsToReturn: []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"}, - } - - details := &watchDetails{ - trackedImage: fp.images[0], - } - - job := NewWatchRepositoryTagsJob(providers, frc, details) - - job.Run() - - // checking whether new job was submitted - - if len(fp.submitted) != 1 { - tags := []string{} - for _, s := range fp.submitted { - tags = append(tags, s.Repository.Tag) - } - t.Errorf("expected 1 events, got: %d [%s]", len(fp.submitted), strings.Join(tags, ", ")) - } - - submitted := fp.submitted[0] - - if submitted.Repository.Name != "index.docker.io/foo/bar" { - t.Errorf("unexpected event repository name: %s", submitted.Repository.Name) - } - - if submitted.Repository.Tag != "1.3.0-dev" { - t.Errorf("expected event repository tag 1.3.0-dev, but got: %s", submitted.Repository.Tag) - } +// Bug #490: new major version "hiding" minor one +func TestWatchAllTagsHiddenMinor(t *testing.T) { + availableTags := []string{"1.3.0", "1.5.0", "2.0.0", "1.2.1"} + testRunHelper([]runTestCase{{"1.2.0", "1.2.1", policy.NewSemverPolicy(policy.SemverPolicyTypePatch, false)}}, availableTags, t) + testRunHelper([]runTestCase{{"1.2.0", "1.5.0", policy.NewSemverPolicy(policy.SemverPolicyTypeMinor, false)}}, availableTags, t) + testRunHelper([]runTestCase{{"1.2.0", "2.0.0", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, false)}}, availableTags, t) } func TestWatchAllTagsMixed(t *testing.T) { - - referenceA, _ := image.Parse("foo/bar:1.0.0") - referenceB, _ := image.Parse("foo/bar:1.2.0-dev") - - fp := &fakeProvider{ - images: []*types.TrackedImage{ - &types.TrackedImage{ - Image: referenceB, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), - }, - &types.TrackedImage{ - Image: referenceA, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), - }, - }, - } - store, teardown := newTestingUtils() - defer teardown() - am := approvals.New(&approvals.Opts{ - Store: store, - }) - - providers := provider.New([]provider.Provider{fp}, am) - - frc := &fakeRegistryClient{ - tagsToReturn: []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"}, - } - - details := &watchDetails{ - trackedImage: fp.images[0], - } - - job := NewWatchRepositoryTagsJob(providers, frc, details) - - job.Run() - - // checking whether new job was submitted - - if len(fp.submitted) != 2 { - tags := []string{} - for _, s := range fp.submitted { - tags = append(tags, s.Repository.Tag) - } - t.Errorf("expected 1 events, got: %d [%s]", len(fp.submitted), strings.Join(tags, ", ")) - } - - submitted := fp.submitted[0] - submitted2 := fp.submitted[1] - - if submitted.Repository.Name != "index.docker.io/foo/bar" { - t.Errorf("unexpected event repository name: %s", submitted.Repository.Name) - } - - if submitted.Repository.Tag != "1.3.0-dev" { - t.Errorf("expected event repository tag 1.3.0-dev, but got: %s", submitted.Repository.Tag) - } - - if submitted2.Repository.Tag != "1.5.0" { - t.Errorf("expected event repository tag 1.5.0, but got: %s", submitted2.Repository.Tag) - } + availableTags := []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"} + testCases := []runTestCase{ + {"1.0.0", "1.5.0", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true)}, + {"1.2.0-dev", "1.3.0-dev", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true)}} + testRunHelper(testCases, availableTags, t) } func TestWatchAllTagsMixedPolicyAll(t *testing.T) { - - referenceA, _ := image.Parse("foo/bar:1.0.0") - referenceB, _ := image.Parse("foo/bar:1.6.0-alpha") - - fp := &fakeProvider{ - images: []*types.TrackedImage{ - &types.TrackedImage{ - Image: referenceB, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), - }, - &types.TrackedImage{ - Image: referenceA, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), - }, - }, - } - store, teardown := newTestingUtils() - defer teardown() - am := approvals.New(&approvals.Opts{ - Store: store, - }) - - providers := provider.New([]provider.Provider{fp}, am) - - frc := &fakeRegistryClient{ - tagsToReturn: []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"}, - } - - details := &watchDetails{ - trackedImage: fp.images[0], - } - - job := NewWatchRepositoryTagsJob(providers, frc, details) - - job.Run() - - // checking whether new job was submitted - - if len(fp.submitted) != 2 { - tags := []string{} - for _, s := range fp.submitted { - tags = append(tags, s.Repository.Tag) - } - t.Errorf("expected 1 events, got: %d [%s]", len(fp.submitted), strings.Join(tags, ", ")) - } - - submitted := fp.submitted[0] - submitted2 := fp.submitted[1] - - if submitted.Repository.Name != "index.docker.io/foo/bar" { - t.Errorf("unexpected event repository name: %s", submitted.Repository.Name) - } - - if submitted.Repository.Tag != "1.8.0-alpha" { - t.Errorf("expected event repository tag 1.8.0-alpha, but got: %s", submitted.Repository.Tag) - } - - if submitted2.Repository.Tag != "1.5.0" { - t.Errorf("expected event repository tag 1.5.0, but got: %s", submitted2.Repository.Tag) - } + availableTags := []string{"1.3.0-dev", "1.5.0", "1.8.0-alpha"} + testCases := []runTestCase{ + {"1.0.0", "1.5.0", policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true)}, + {"1.6.0-alpha", "1.8.0-alpha", policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true)}} + testRunHelper(testCases, availableTags, t) } -func Test_collapse(t *testing.T) { - type args struct { - tags []string +func Test_semverSort(t *testing.T) { + tags := []string{"1.3.0", "aa1.0.0", "zzz", "1.3.0-dev", "1.5.0", "2.0.0-alpha", "1.3.0-dev1", "1.8.0-alpha", "1.3.1-dev", "123", "1.2.3-rc.1.2+meta"} + expectedTags := []string{"2.0.0-alpha", "1.8.0-alpha", "1.5.0", "1.3.1-dev", "1.3.0", "1.3.0-dev1", "1.3.0-dev", "1.2.3-rc.1.2+meta"} + expectedVersions := make([]*semver.Version, len(expectedTags)) + for i, tag := range expectedTags { + v, _ := semver.NewVersion(tag) + expectedVersions[i] = v } - tests := []struct { - name string - args args - want []string - }{ - { - name: "single version", - args: args{tags: []string{"1.0.0"}}, - want: []string{"1.0.0"}, - }, - { - name: "multi", - args: args{tags: []string{"1.0.0", "1.4.0"}}, - want: []string{"1.4.0"}, - }, - { - name: "prerelease", - args: args{tags: []string{"1.0.0-dev", "1.4.0-dev"}}, - want: []string{"1.4.0-dev"}, - }, - { - name: "prerelease multi", - args: args{tags: []string{"1.3.0-bb", "1.0.0-dev", "1.4.0-dev"}}, - want: []string{"1.3.0-bb", "1.4.0-dev"}, - }, - { - name: "prerelease multi, mixed", - args: args{tags: []string{"1.2.0", "1.3.0-bb", "1.0.0-dev", "1.4.0-dev"}}, - want: []string{"1.2.0", "1.3.0-bb", "1.4.0-dev"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := collapse(tt.args.tags); !reflect.DeepEqual(got, tt.want) { - t.Errorf("collapse() = %v, want %v", got, tt.want) - } - }) + sortedTags := semverSort(tags) + + if !reflect.DeepEqual(sortedTags, expectedVersions) { + t.Errorf("Invalid sorted tags; expected: %s; got: %s", expectedVersions, sortedTags) } } diff --git a/trigger/poll/watcher_test.go b/trigger/poll/watcher_test.go index cc0fae09..756856cc 100644 --- a/trigger/poll/watcher_test.go +++ b/trigger/poll/watcher_test.go @@ -179,7 +179,7 @@ func TestWatchAllTagsJob(t *testing.T) { images: []*types.TrackedImage{ &types.TrackedImage{ Image: reference, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeAll, true), }, }, } @@ -270,7 +270,7 @@ func TestWatchMultipleTags(t *testing.T) { Provider: "fp", PollSchedule: types.KeelPollDefaultSchedule, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true), }, &types.TrackedImage{ @@ -278,7 +278,7 @@ func TestWatchMultipleTags(t *testing.T) { Image: imgB, Provider: "fp", PollSchedule: types.KeelPollDefaultSchedule, - Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor), + Policy: policy.NewSemverPolicy(policy.SemverPolicyTypeMajor, true), }, &types.TrackedImage{ diff --git a/types/types.go b/types/types.go index 25eab6db..942b9f92 100644 --- a/types/types.go +++ b/types/types.go @@ -32,6 +32,9 @@ const KeelTriggerLabel = "keel.sh/trigger" const KeelForceTagMatchLegacyLabel = "keel.sh/match-tag" const KeelForceTagMatchLabel = "keel.sh/matchTag" +// KeelMatchPreReleaseAnnotation - label or annotation to set pre-release matching for SemVer, defaults to true for backward compatibility +const KeelMatchPreReleaseAnnotation = "keel.sh/matchPreRelease" + // KeelPollScheduleAnnotation - optional variable to setup custom schedule for polling, defaults to @every 10m const KeelPollScheduleAnnotation = "keel.sh/pollSchedule"