[Bugfix] Fix TagsWatcher only setup when the image is SemVer compatible (#812)

[Bugfix] Fix TagsWatcher only setup when the image is SemVer compatible (#812)

internal/policy/force.go

@@ -1,5 +1,7 @@
 package policy
 
+import "github.com/keel-hq/keel/types"
+
 type ForcePolicy struct {
 	matchTag bool
 }
@@ -18,6 +20,7 @@ func (fp *ForcePolicy) ShouldUpdate(current, new string) (bool, error) {
 }
 
 func (fp *ForcePolicy) Filter(tags []string) []string {
+	// todo: why is this not sorting?
 	return append([]string{}, tags...)
 }
 
@@ -25,4 +28,6 @@ func (fp *ForcePolicy) Name() string {
 	return "force"
 }
 
-func (fp *ForcePolicy) Type() PolicyType { return PolicyTypeForce }
+func (fp *ForcePolicy) Type() types.PolicyType { return types.PolicyTypeForce }
+
+func (fp *ForcePolicy) KeepTag() bool { return fp.matchTag }

internal/policy/glob.go

@@ -2,6 +2,7 @@ package policy
 
 import (
 	"fmt"
+	"github.com/keel-hq/keel/types"
 	"sort"
 	"strings"
 
@@ -27,8 +28,8 @@ func NewGlobPolicy(policy string) (*GlobPolicy, error) {
 	return nil, fmt.Errorf("invalid glob policy: %s", policy)
 }
 
-func (p *GlobPolicy) ShouldUpdate(current, new string) (bool, error) {
-	return glob.Glob(p.pattern, new), nil
+func (p *GlobPolicy) ShouldUpdate(current string, new string) (bool, error) {
+	return (glob.Glob(p.pattern, new) && strings.Compare(new, current) > 0), nil
 }
 
 func (p *GlobPolicy) Filter(tags []string) []string {
@@ -48,5 +49,6 @@ func (p *GlobPolicy) Filter(tags []string) []string {
 	return filtered
 }
 
-func (p *GlobPolicy) Name() string     { return p.policy }
-func (p *GlobPolicy) Type() PolicyType { return PolicyTypeGlob }
+func (p *GlobPolicy) Name() string           { return p.policy }
+func (p *GlobPolicy) Type() types.PolicyType { return types.PolicyTypeGlob }
+func (p *GlobPolicy) KeepTag() bool          { return false }

internal/policy/glob_test.go

@@ -22,7 +22,7 @@ func TestGlobPolicy_ShouldUpdate(t *testing.T) {
 			name:    "test glob latest",
 			fields:  fields{pattern: "latest"},
 			args:    args{current: "latest", new: "latest"},
-			want:    true,
+			want:    false,
 			wantErr: false,
 		},
 		{
@@ -33,12 +33,26 @@ func TestGlobPolicy_ShouldUpdate(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name:    "test glob with *",
+			name:    "test glob with lat*",
 			fields:  fields{pattern: "lat*"},
 			args:    args{current: "latest", new: "latest"},
+			want:    false,
+			wantErr: false,
+		},
+		{
+			name:    "test glob with latest.*",
+			fields:  fields{pattern: "latest.*"},
+			args:    args{current: "latest.20241321", new: "latest.20251321"},
 			want:    true,
 			wantErr: false,
 		},
+		{
+			name:    "test glob with latest.* reverse",
+			fields:  fields{pattern: "latest.*"},
+			args:    args{current: "latest.20251321", new: "latest.20241321"},
+			want:    false,
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/policy/policy.go

@@ -3,34 +3,29 @@ package policy
 import (
 	"strings"
 
+	"github.com/keel-hq/keel/util/image"
+	"github.com/keel-hq/keel/util/version"
+
 	"github.com/keel-hq/keel/types"
 
 	log "github.com/sirupsen/logrus"
 )
 
-type PolicyType int
-
-const (
-	PolicyTypeNone PolicyType = iota
-	PolicyTypeSemver
-	PolicyTypeForce
-	PolicyTypeGlob
-	PolicyTypeRegexp
-)
-
 type Policy interface {
 	ShouldUpdate(current, new string) (bool, error)
 	Name() string
-	Type() PolicyType
+	Type() types.PolicyType
 	Filter(tags []string) []string
+	KeepTag() bool
 }
 
 type NilPolicy struct{}
 
 func (np *NilPolicy) ShouldUpdate(c, n string) (bool, error) { return false, nil }
 func (np *NilPolicy) Name() string                           { return "nil policy" }
-func (np *NilPolicy) Type() PolicyType                       { return PolicyTypeNone }
+func (np *NilPolicy) Type() types.PolicyType                 { return types.PolicyTypeNone }
 func (np *NilPolicy) Filter(tags []string) []string          { return append([]string{}, tags...) }
+func (np *NilPolicy) KeepTag() bool                          { return false }
 
 // GetPolicyFromLabelsOrAnnotations - gets policy from k8s labels or annotations
 func GetPolicyFromLabelsOrAnnotations(labels map[string]string, annotations map[string]string) Policy {
@@ -143,3 +138,19 @@ func getMatchPreRelease(labels map[string]string) bool {
 	// Default to true for backward compatibility
 	return true
 }
+
+// LegacyPolicyPopulate creates a policy based on the image tag
+func LegacyPolicyPopulate(ref *image.Reference) Policy {
+	_, err := version.GetVersion(ref.Tag())
+	var policy Policy
+	if err == nil {
+		policy = NewSemverPolicy(SemverPolicyTypeAll, true)
+	} else {
+		policy = NewForcePolicy(true)
+	}
+	log.WithFields(log.Fields{
+		"image":  ref.Name(),
+		"policy": policy.Type(),
+	}).Info("trigger.poll.watcher: image policy was not configured. Automatic policy was set.")
+	return policy
+}

internal/policy/regexp.go

@@ -2,6 +2,7 @@ package policy
 
 import (
 	"fmt"
+	"github.com/keel-hq/keel/types"
 	"regexp"
 	"sort"
 	"strings"
@@ -60,5 +61,6 @@ func (p *RegexpPolicy) Filter(tags []string) []string {
 	return filtered
 }
 
-func (p *RegexpPolicy) Name() string     { return p.policy }
-func (p *RegexpPolicy) Type() PolicyType { return PolicyTypeRegexp }
+func (p *RegexpPolicy) Name() string           { return p.policy }
+func (p *RegexpPolicy) Type() types.PolicyType { return types.PolicyTypeRegexp }
+func (p *RegexpPolicy) KeepTag() bool          { return false }

internal/policy/semver.go

@@ -6,6 +6,8 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/keel-hq/keel/types"
+
 	"github.com/Masterminds/semver"
 )
 
@@ -62,7 +64,9 @@ func (sp *SemverPolicy) Name() string {
 	return sp.spt.String()
 }
 
-func (sp *SemverPolicy) Type() PolicyType { return PolicyTypeSemver }
+func (sp *SemverPolicy) Type() types.PolicyType { return types.PolicyTypeSemver }
+
+func (sp *SemverPolicy) KeepTag() bool { return false }
 
 func shouldUpdate(spt SemverPolicyType, matchPreRelease bool, current, new string) (bool, error) {
 	if current == "latest" {

provider/helm3/updates.go

@@ -1,7 +1,6 @@
 package helm3
 
 import (
-	"github.com/keel-hq/keel/internal/policy"
 	"github.com/keel-hq/keel/types"
 	"github.com/keel-hq/keel/util/image"
 
@@ -52,7 +51,7 @@ func checkRelease(repo *types.Repository, namespace, name string, chart *hapi_ch
 	}
 	log.Infof("policy for release %s/%s parsed: %s", namespace, name, keelCfg.Plc.Name())
 
-	if keelCfg.Plc.Type() == policy.PolicyTypeNone {
+	if keelCfg.Plc.Type() == types.PolicyTypeNone {
 		// policy is not set, ignoring release
 		return plan, false, nil
 	}

provider/kubernetes/kubernetes.go

@@ -221,7 +221,7 @@ func (p *Provider) TrackedImages() ([]*types.TrackedImage, error) {
 
 		// ignoring unlabelled deployments
 		plc := policy.GetPolicyFromLabelsOrAnnotations(labels, annotations)
-		if plc.Type() == policy.PolicyTypeNone {
+		if plc.Type() == types.PolicyTypeNone {
 			continue
 		}
 
@@ -493,7 +493,7 @@ func (p *Provider) createUpdatePlans(repo *types.Repository) ([]*UpdatePlan, err
 		annotations := resource.GetAnnotations()
 
 		plc := policy.GetPolicyFromLabelsOrAnnotations(labels, annotations)
-		if plc.Type() == policy.PolicyTypeNone {
+		if plc.Type() == types.PolicyTypeNone {
 			continue
 		}
 

trigger/poll/manager_test.go

@@ -2,6 +2,7 @@ package poll
 
 import (
 	"context"
+	"github.com/keel-hq/keel/internal/policy"
 	"log"
 	"os"
 	"path/filepath"
@@ -59,6 +60,7 @@ func TestCheckDeployment(t *testing.T) {
 				Trigger:      types.TriggerTypePoll,
 				Provider:     "fp",
 				PollSchedule: types.KeelPollDefaultSchedule,
+				Policy:       policy.LegacyPolicyPopulate(imgA),
 			},
 
 			{
@@ -66,6 +68,7 @@ func TestCheckDeployment(t *testing.T) {
 				Image:        imgB,
 				Provider:     "fp",
 				PollSchedule: types.KeelPollDefaultSchedule,
+				Policy:       policy.LegacyPolicyPopulate(imgB),
 			},
 		},
 	}

trigger/poll/multi_tags_watcher.go

@@ -90,17 +90,31 @@ func (j *WatchRepositoryTagsJob) computeEvents(tags []string) ([]types.Event, er
 
 	events := []types.Event{}
 
-	if j.details.trackedImage.Policy != nil {
-		tags = j.details.trackedImage.Policy.Filter(tags)
-	}
+	// This contains all tracked images that share the same imageIdentifier and thus, the same watcher
+	allRelatedTrackedImages := getRelatedTrackedImages(j.details.trackedImage, trackedImages)
+
+	for _, trackedImage := range allRelatedTrackedImages {
+
+		filteredTags := tags
+
+		// The fact that they are related, does not mean they share the exact same Policy configuration, so wee need
+		// to calculate the tags here for each image.
+		filteredTags = j.details.trackedImage.Policy.Filter(tags)
+
+		for _, tag := range filteredTags {
 
-	for _, trackedImage := range getRelatedTrackedImages(j.details.trackedImage, trackedImages) {
-		for _, tag := range tags {
 			update, err := trackedImage.Policy.ShouldUpdate(trackedImage.Image.Tag(), tag)
 			if err != nil {
 				continue
 			}
-			if update && !exists(tag, events) {
+			if update == false {
+				continue
+			}
+			// When using tags watcher we rely completely on tag names to deal with updates.
+			if trackedImage.Image.Tag() == tag {
+				break
+			}
+			if !exists(tag, events) {
 				event := types.Event{
 					Repository: types.Repository{
 						Name: trackedImage.Image.Repository(),
@@ -134,11 +148,10 @@ func exists(tag string, events []types.Event) bool {
 func getRelatedTrackedImages(ours *types.TrackedImage, all []*types.TrackedImage) []*types.TrackedImage {
 	b := all[:0]
 	for _, x := range all {
-		if x.Image.Repository() == ours.Image.Repository() {
+		if getImageIdentifier(x.Image, x.Policy.KeepTag()) == getImageIdentifier(ours.Image, ours.Policy.KeepTag()) {
 			b = append(b, x)
 		}
 	}
-
 	return b
 }
 

trigger/poll/watcher.go

@@ -6,11 +6,12 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/keel-hq/keel/util/image"
+
 	"github.com/keel-hq/keel/extension/credentialshelper"
 	"github.com/keel-hq/keel/provider"
 	"github.com/keel-hq/keel/registry"
 	"github.com/keel-hq/keel/types"
-	"github.com/keel-hq/keel/util/image"
 	"github.com/keel-hq/keel/util/version"
 	"github.com/rusenask/cron"
 
@@ -50,8 +51,7 @@ type watchDetails struct {
 	digest       string // image digest
 	latest       string // latest tag
 	schedule     string
-
-	mu sync.RWMutex
+	mu           sync.RWMutex
 }
 
 // RepositoryWatcher - repository watcher cron
@@ -90,33 +90,22 @@ func (w *RepositoryWatcher) Start(ctx context.Context) {
 	}()
 }
 
+// This identifier is used to key the watchers, so that only a watcher
+// is setup per identifier
 func getImageIdentifier(ref *image.Reference, keepTag bool) string {
-	_, err := version.GetVersion(ref.Tag())
-	// if failed to parse version, will need to watch digest
-	if err != nil || keepTag == true {
+	if keepTag == true {
 		return ref.Registry() + "/" + ref.ShortName() + ":" + ref.Tag()
 	}
-
 	return ref.Registry() + "/" + ref.ShortName()
 }
 
 // Unwatch - stop watching for changes
-func (w *RepositoryWatcher) Unwatch(imageName string) error {
-	imageRef, err := image.Parse(imageName)
-	if err != nil {
-		log.WithFields(log.Fields{
-			"error":      err,
-			"image_name": imageName,
-		}).Error("trigger.poll.RepositoryWatcher.Unwatch: failed to parse image")
-		return err
-	}
-	key := getImageIdentifier(imageRef, false)
-	_, ok := w.watched[key]
+func (w *RepositoryWatcher) Unwatch(imageIdentifier string) error {
+	_, ok := w.watched[imageIdentifier]
 	if ok {
-		w.cron.DeleteJob(key)
-		delete(w.watched, key)
+		w.cron.DeleteJob(imageIdentifier)
+		delete(w.watched, imageIdentifier)
 	}
-
 	return nil
 }
 
@@ -183,13 +172,11 @@ func (w *RepositoryWatcher) watch(image *types.TrackedImage) (string, error) {
 		return "", fmt.Errorf("invalid cron schedule: %s", err)
 	}
 
-	keepTag := image.Policy != nil && image.Policy.Name() == "force"
-	key := getImageIdentifier(image.Image, keepTag)
+	key := getImageIdentifier(image.Image, image.Policy.KeepTag())
 
 	// checking whether it's already being watched
 	details, ok := w.watched[key]
 	if !ok {
-		// err = w.addJob(imageRef, registryUsername, registryPassword, schedule)
 		err = w.addJob(image, image.PollSchedule)
 		if err != nil {
 			log.WithFields(log.Fields{
@@ -202,6 +189,8 @@ func (w *RepositoryWatcher) watch(image *types.TrackedImage) (string, error) {
 	}
 
 	// checking schedule
+	// todo: this is not right, we are using the last seen schedule, which might not be the most frequent
+	// the most frequent schedule should be used for the shared watcher
 	if details.schedule != image.PollSchedule {
 		err := w.cron.UpdateJob(key, image.PollSchedule)
 		if err != nil {
@@ -249,8 +238,8 @@ func (w *RepositoryWatcher) addJob(ti *types.TrackedImage, schedule string) erro
 		return err
 	}
 
-	keepTag := ti.Policy != nil && ti.Policy.Name() == "force"
-	key := getImageIdentifier(ti.Image, keepTag)
+	key := getImageIdentifier(ti.Image, ti.Policy.KeepTag())
+
 	details := &watchDetails{
 		trackedImage: ti,
 		digest:       digest, // current image digest
@@ -261,15 +250,9 @@ func (w *RepositoryWatcher) addJob(ti *types.TrackedImage, schedule string) erro
 	// adding job to internal map
 	w.watched[key] = details
 
-	// checking tag type:
-	//  - for versioned (semver) tags:
-	//    - we setup a watch all tags job (default)
-	//    - if "force" to follow a floating tag, a single tag watcher is
-	//      setup, which checks digest
-	//  - for non-semver types we create a single tag watcher which
-	// checks digest
-	_, err = version.GetVersion(ti.Image.Tag())
-	if err != nil || keepTag == true {
+	// read the docs several times, the only legit case when want a tag watcher
+	// is when policy is force and keel.sh/match-tag=true.
+	if ti.Policy.KeepTag() {
 		// adding new job
 		job := NewWatchTagJob(w.providers, w.registryClient, details)
 		log.WithFields(log.Fields{
@@ -281,7 +264,6 @@ func (w *RepositoryWatcher) addJob(ti *types.TrackedImage, schedule string) erro
 
 		// running it now
 		job.Run()
-
 		return w.cron.AddJob(key, schedule, job)
 	}
 
@@ -293,10 +275,6 @@ func (w *RepositoryWatcher) addJob(ti *types.TrackedImage, schedule string) erro
 		"digest":   digest,
 		"schedule": schedule,
 	}).Info("trigger.poll.RepositoryWatcher: new watch repository tags job added")
-
-	// running it now
 	job.Run()
-
 	return w.cron.AddJob(key, schedule, job)
-
 }

trigger/poll/watcher_test.go

@@ -25,6 +25,7 @@ func mustParse(img string, schedule string) *types.TrackedImage {
 		Image:        ref,
 		PollSchedule: schedule,
 		Trigger:      types.TriggerTypePoll,
+		Policy:       policy.LegacyPolicyPopulate(ref),
 	}
 }
 

types/policytype_jsonenums.go

@@ -1,6 +1,6 @@
 // generated by jsonenums -type=PolicyType; DO NOT EDIT
 
-package policy
+package types
 
 import (
 	"encoding/json"

types/tracked_images.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"fmt"
-
 	"github.com/keel-hq/keel/util/image"
 )
 
@@ -31,6 +30,8 @@ type Policy interface {
 	ShouldUpdate(current, new string) (bool, error)
 	Name() string
 	Filter(tags []string) []string
+	Type() PolicyType
+	KeepTag() bool
 }
 
 func (i TrackedImage) String() string {

types/types.go

@@ -378,3 +378,13 @@ func (t ProviderType) String() string {
 		return ""
 	}
 }
+
+type PolicyType int
+
+const (
+	PolicyTypeNone PolicyType = iota
+	PolicyTypeSemver
+	PolicyTypeForce
+	PolicyTypeGlob
+	PolicyTypeRegexp
+)