Skip to content

Commit 13a7b5a

Browse files
david-garcia-garciadavid-garcia-garcia
authored
[Bugfix] Fix TagsWatcher only setup when the image is SemVer compatible (#812)
[Bugfix] Fix TagsWatcher only setup when the image is SemVer compatible (#812)
1 parent c7e3710 commit 13a7b5a

File tree

15 files changed

+119
-76
lines changed

15 files changed

+119
-76
lines changed

‎internal/policy/force.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package policy
22

3+
import "github.com/keel-hq/keel/types"
4+
35
type ForcePolicy struct {
46
matchTag bool
57
}
@@ -18,11 +20,14 @@ func (fp *ForcePolicy) ShouldUpdate(current, new string) (bool, error) {
1820
}
1921

2022
func (fp *ForcePolicy) Filter(tags []string) []string {
23+
// todo: why is this not sorting?
2124
return append([]string{}, tags...)
2225
}
2326

2427
func (fp *ForcePolicy) Name() string {
2528
return "force"
2629
}
2730

28-
func (fp *ForcePolicy) Type() PolicyType { return PolicyTypeForce }
31+
func (fp *ForcePolicy) Type() types.PolicyType { return types.PolicyTypeForce }
32+
33+
func (fp *ForcePolicy) KeepTag() bool { return fp.matchTag }

‎internal/policy/glob.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package policy
22

33
import (
44
"fmt"
5+
"github.com/keel-hq/keel/types"
56
"sort"
67
"strings"
78

@@ -27,8 +28,8 @@ func NewGlobPolicy(policy string) (*GlobPolicy, error) {
2728
return nil, fmt.Errorf("invalid glob policy: %s", policy)
2829
}
2930

30-
func (p *GlobPolicy) ShouldUpdate(current, new string) (bool, error) {
31-
return glob.Glob(p.pattern, new), nil
31+
func (p *GlobPolicy) ShouldUpdate(current string, new string) (bool, error) {
32+
return (glob.Glob(p.pattern, new) && strings.Compare(new, current) > 0), nil
3233
}
3334

3435
func (p *GlobPolicy) Filter(tags []string) []string {
@@ -48,5 +49,6 @@ func (p *GlobPolicy) Filter(tags []string) []string {
4849
return filtered
4950
}
5051

51-
func (p *GlobPolicy) Name() string { return p.policy }
52-
func (p *GlobPolicy) Type() PolicyType { return PolicyTypeGlob }
52+
func (p *GlobPolicy) Name() string { return p.policy }
53+
func (p *GlobPolicy) Type() types.PolicyType { return types.PolicyTypeGlob }
54+
func (p *GlobPolicy) KeepTag() bool { return false }

‎internal/policy/glob_test.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestGlobPolicy_ShouldUpdate(t *testing.T) {
2222
name: "test glob latest",
2323
fields: fields{pattern: "latest"},
2424
args: args{current: "latest", new: "latest"},
25-
want: true,
25+
want: false,
2626
wantErr: false,
2727
},
2828
{
@@ -33,12 +33,26 @@ func TestGlobPolicy_ShouldUpdate(t *testing.T) {
3333
wantErr: false,
3434
},
3535
{
36-
name: "test glob with *",
36+
name: "test glob with lat*",
3737
fields: fields{pattern: "lat*"},
3838
args: args{current: "latest", new: "latest"},
39+
want: false,
40+
wantErr: false,
41+
},
42+
{
43+
name: "test glob with latest.*",
44+
fields: fields{pattern: "latest.*"},
45+
args: args{current: "latest.20241321", new: "latest.20251321"},
3946
want: true,
4047
wantErr: false,
4148
},
49+
{
50+
name: "test glob with latest.* reverse",
51+
fields: fields{pattern: "latest.*"},
52+
args: args{current: "latest.20251321", new: "latest.20241321"},
53+
want: false,
54+
wantErr: false,
55+
},
4256
}
4357
for _, tt := range tests {
4458
t.Run(tt.name, func(t *testing.T) {

‎internal/policy/policy.go‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,29 @@ package policy
33
import (
44
"strings"
55

6+
"github.com/keel-hq/keel/util/image"
7+
"github.com/keel-hq/keel/util/version"
8+
69
"github.com/keel-hq/keel/types"
710

811
log "github.com/sirupsen/logrus"
912
)
1013

11-
type PolicyType int
12-
13-
const (
14-
PolicyTypeNone PolicyType = iota
15-
PolicyTypeSemver
16-
PolicyTypeForce
17-
PolicyTypeGlob
18-
PolicyTypeRegexp
19-
)
20-
2114
type Policy interface {
2215
ShouldUpdate(current, new string) (bool, error)
2316
Name() string
24-
Type() PolicyType
17+
Type() types.PolicyType
2518
Filter(tags []string) []string
19+
KeepTag() bool
2620
}
2721

2822
type NilPolicy struct{}
2923

3024
func (np *NilPolicy) ShouldUpdate(c, n string) (bool, error) { return false, nil }
3125
func (np *NilPolicy) Name() string { return "nil policy" }
32-
func (np *NilPolicy) Type() PolicyType { return PolicyTypeNone }
26+
func (np *NilPolicy) Type() types.PolicyType { return types.PolicyTypeNone }
3327
func (np *NilPolicy) Filter(tags []string) []string { return append([]string{}, tags...) }
28+
func (np *NilPolicy) KeepTag() bool { return false }
3429

3530
// GetPolicyFromLabelsOrAnnotations - gets policy from k8s labels or annotations
3631
func GetPolicyFromLabelsOrAnnotations(labels map[string]string, annotations map[string]string) Policy {
@@ -143,3 +138,19 @@ func getMatchPreRelease(labels map[string]string) bool {
143138
// Default to true for backward compatibility
144139
return true
145140
}
141+
142+
// LegacyPolicyPopulate creates a policy based on the image tag
143+
func LegacyPolicyPopulate(ref *image.Reference) Policy {
144+
_, err := version.GetVersion(ref.Tag())
145+
var policy Policy
146+
if err == nil {
147+
policy = NewSemverPolicy(SemverPolicyTypeAll, true)
148+
} else {
149+
policy = NewForcePolicy(true)
150+
}
151+
log.WithFields(log.Fields{
152+
"image": ref.Name(),
153+
"policy": policy.Type(),
154+
}).Info("trigger.poll.watcher: image policy was not configured. Automatic policy was set.")
155+
return policy
156+
}

‎internal/policy/regexp.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package policy
22

33
import (
44
"fmt"
5+
"github.com/keel-hq/keel/types"
56
"regexp"
67
"sort"
78
"strings"
@@ -60,5 +61,6 @@ func (p *RegexpPolicy) Filter(tags []string) []string {
6061
return filtered
6162
}
6263

63-
func (p *RegexpPolicy) Name() string { return p.policy }
64-
func (p *RegexpPolicy) Type() PolicyType { return PolicyTypeRegexp }
64+
func (p *RegexpPolicy) Name() string { return p.policy }
65+
func (p *RegexpPolicy) Type() types.PolicyType { return types.PolicyTypeRegexp }
66+
func (p *RegexpPolicy) KeepTag() bool { return false }

‎internal/policy/semver.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"sort"
77
"strings"
88

9+
"github.com/keel-hq/keel/types"
10+
911
"github.com/Masterminds/semver"
1012
)
1113

@@ -62,7 +64,9 @@ func (sp *SemverPolicy) Name() string {
6264
return sp.spt.String()
6365
}
6466

65-
func (sp *SemverPolicy) Type() PolicyType { return PolicyTypeSemver }
67+
func (sp *SemverPolicy) Type() types.PolicyType { return types.PolicyTypeSemver }
68+
69+
func (sp *SemverPolicy) KeepTag() bool { return false }
6670

6771
func shouldUpdate(spt SemverPolicyType, matchPreRelease bool, current, new string) (bool, error) {
6872
if current == "latest" {

‎provider/helm3/updates.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package helm3
22

33
import (
4-
"github.com/keel-hq/keel/internal/policy"
54
"github.com/keel-hq/keel/types"
65
"github.com/keel-hq/keel/util/image"
76

@@ -52,7 +51,7 @@ func checkRelease(repo *types.Repository, namespace, name string, chart *hapi_ch
5251
}
5352
log.Infof("policy for release %s/%s parsed: %s", namespace, name, keelCfg.Plc.Name())
5453

55-
if keelCfg.Plc.Type() == policy.PolicyTypeNone {
54+
if keelCfg.Plc.Type() == types.PolicyTypeNone {
5655
// policy is not set, ignoring release
5756
return plan, false, nil
5857
}

‎provider/kubernetes/kubernetes.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func (p *Provider) TrackedImages() ([]*types.TrackedImage, error) {
221221

222222
// ignoring unlabelled deployments
223223
plc := policy.GetPolicyFromLabelsOrAnnotations(labels, annotations)
224-
if plc.Type() == policy.PolicyTypeNone {
224+
if plc.Type() == types.PolicyTypeNone {
225225
continue
226226
}
227227

@@ -493,7 +493,7 @@ func (p *Provider) createUpdatePlans(repo *types.Repository) ([]*UpdatePlan, err
493493
annotations := resource.GetAnnotations()
494494

495495
plc := policy.GetPolicyFromLabelsOrAnnotations(labels, annotations)
496-
if plc.Type() == policy.PolicyTypeNone {
496+
if plc.Type() == types.PolicyTypeNone {
497497
continue
498498
}
499499

‎trigger/poll/manager_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package poll
22

33
import (
44
"context"
5+
"github.com/keel-hq/keel/internal/policy"
56
"log"
67
"os"
78
"path/filepath"
@@ -59,13 +60,15 @@ func TestCheckDeployment(t *testing.T) {
5960
Trigger: types.TriggerTypePoll,
6061
Provider: "fp",
6162
PollSchedule: types.KeelPollDefaultSchedule,
63+
Policy: policy.LegacyPolicyPopulate(imgA),
6264
},
6365

6466
{
6567
Trigger: types.TriggerTypePoll,
6668
Image: imgB,
6769
Provider: "fp",
6870
PollSchedule: types.KeelPollDefaultSchedule,
71+
Policy: policy.LegacyPolicyPopulate(imgB),
6972
},
7073
},
7174
}

‎trigger/poll/multi_tags_watcher.go‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,31 @@ func (j *WatchRepositoryTagsJob) computeEvents(tags []string) ([]types.Event, er
9090

9191
events := []types.Event{}
9292

93-
if j.details.trackedImage.Policy != nil {
94-
tags = j.details.trackedImage.Policy.Filter(tags)
95-
}
93+
// This contains all tracked images that share the same imageIdentifier and thus, the same watcher
94+
allRelatedTrackedImages := getRelatedTrackedImages(j.details.trackedImage, trackedImages)
95+
96+
for _, trackedImage := range allRelatedTrackedImages {
97+
98+
filteredTags := tags
99+
100+
// The fact that they are related, does not mean they share the exact same Policy configuration, so wee need
101+
// to calculate the tags here for each image.
102+
filteredTags = j.details.trackedImage.Policy.Filter(tags)
103+
104+
for _, tag := range filteredTags {
96105

97-
for _, trackedImage := range getRelatedTrackedImages(j.details.trackedImage, trackedImages) {
98-
for _, tag := range tags {
99106
update, err := trackedImage.Policy.ShouldUpdate(trackedImage.Image.Tag(), tag)
100107
if err != nil {
101108
continue
102109
}
103-
if update && !exists(tag, events) {
110+
if update == false {
111+
continue
112+
}
113+
// When using tags watcher we rely completely on tag names to deal with updates.
114+
if trackedImage.Image.Tag() == tag {
115+
break
116+
}
117+
if !exists(tag, events) {
104118
event := types.Event{
105119
Repository: types.Repository{
106120
Name: trackedImage.Image.Repository(),
@@ -134,11 +148,10 @@ func exists(tag string, events []types.Event) bool {
134148
func getRelatedTrackedImages(ours *types.TrackedImage, all []*types.TrackedImage) []*types.TrackedImage {
135149
b := all[:0]
136150
for _, x := range all {
137-
if x.Image.Repository() == ours.Image.Repository() {
151+
if getImageIdentifier(x.Image, x.Policy.KeepTag()) == getImageIdentifier(ours.Image, ours.Policy.KeepTag()) {
138152
b = append(b, x)
139153
}
140154
}
141-
142155
return b
143156
}
144157

0 commit comments

Comments
 (0)