diff --git a/internal/policy/force.go b/internal/policy/force.go index 77d15558..b337f522 100644 --- a/internal/policy/force.go +++ b/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 } diff --git a/internal/policy/glob.go b/internal/policy/glob.go index d0f5ec7c..844d4666 100644 --- a/internal/policy/glob.go +++ b/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 } diff --git a/internal/policy/glob_test.go b/internal/policy/glob_test.go index 68d4646b..a86951f2 100644 --- a/internal/policy/glob_test.go +++ b/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) { diff --git a/internal/policy/policy.go b/internal/policy/policy.go index fb77bb29..2a3aff98 100644 --- a/internal/policy/policy.go +++ b/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 +} diff --git a/internal/policy/regexp.go b/internal/policy/regexp.go index e28c8549..aa9fcdeb 100644 --- a/internal/policy/regexp.go +++ b/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 } diff --git a/internal/policy/semver.go b/internal/policy/semver.go index c0d7faa4..7120a916 100644 --- a/internal/policy/semver.go +++ b/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" { diff --git a/provider/helm3/updates.go b/provider/helm3/updates.go index b30e1a6d..02ca52b2 100644 --- a/provider/helm3/updates.go +++ b/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 } diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go index 1fca7249..8965c00f 100644 --- a/provider/kubernetes/kubernetes.go +++ b/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 } diff --git a/trigger/poll/manager_test.go b/trigger/poll/manager_test.go index 638a1c12..96b8b895 100644 --- a/trigger/poll/manager_test.go +++ b/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), }, }, } diff --git a/trigger/poll/multi_tags_watcher.go b/trigger/poll/multi_tags_watcher.go index 542bdd8d..9794e4f6 100644 --- a/trigger/poll/multi_tags_watcher.go +++ b/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 } diff --git a/trigger/poll/watcher.go b/trigger/poll/watcher.go index 3591e2d6..c78a3ee7 100644 --- a/trigger/poll/watcher.go +++ 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) - } diff --git a/trigger/poll/watcher_test.go b/trigger/poll/watcher_test.go index 1d6e053e..5322f4bc 100644 --- a/trigger/poll/watcher_test.go +++ b/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), } } diff --git a/internal/policy/policytype_jsonenums.go b/types/policytype_jsonenums.go similarity index 99% rename from internal/policy/policytype_jsonenums.go rename to types/policytype_jsonenums.go index c810538e..4c40715a 100644 --- a/internal/policy/policytype_jsonenums.go +++ b/types/policytype_jsonenums.go @@ -1,6 +1,6 @@ // generated by jsonenums -type=PolicyType; DO NOT EDIT -package policy +package types import ( "encoding/json" diff --git a/types/tracked_images.go b/types/tracked_images.go index 0dc65f04..460f5cac 100644 --- a/types/tracked_images.go +++ b/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 { diff --git a/types/types.go b/types/types.go index 95bbe856..950c1efb 100644 --- a/types/types.go +++ b/types/types.go @@ -378,3 +378,13 @@ func (t ProviderType) String() string { return "" } } + +type PolicyType int + +const ( + PolicyTypeNone PolicyType = iota + PolicyTypeSemver + PolicyTypeForce + PolicyTypeGlob + PolicyTypeRegexp +)