diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index b0042bfd49781..128b1be78b161 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + "github.com/Masterminds/semver/v3" "github.com/go-playground/webhooks/v6/azuredevops" "github.com/go-playground/webhooks/v6/bitbucket" bitbucketserver "github.com/go-playground/webhooks/v6/bitbucket-server" @@ -413,11 +414,33 @@ func sourceRevisionHasChanged(source v1alpha1.ApplicationSource, revision string targetRevisionHasPrefixList := []string{"refs/heads/", "refs/tags/"} for _, prefix := range targetRevisionHasPrefixList { if strings.HasPrefix(source.TargetRevision, prefix) { - return revision == targetRev + return compareRevisions(revision, targetRev) } } - return source.TargetRevision == revision + return compareRevisions(revision, source.TargetRevision) +} + +func compareRevisions(revision string, targetRevision string) bool { + if revision == targetRevision { + return true + } + + // If basic equality checking fails, it might be that the target revision is + // a semver version constraint + constraint, err := semver.NewConstraint(targetRevision) + if err != nil { + // The target revision is not a constraint + return false + } + + version, err := semver.NewVersion(revision) + if err != nil { + // The new revision is not a valid semver version, so it can't match the constraint. + return false + } + + return constraint.Check(version) } func sourceUsesURL(source v1alpha1.ApplicationSource, webURL string, repoRegexp *regexp.Regexp) bool { diff --git a/util/webhook/webhook_test.go b/util/webhook/webhook_test.go index 33ca167ccc865..dc99b42f6bd94 100644 --- a/util/webhook/webhook_test.go +++ b/util/webhook/webhook_test.go @@ -466,8 +466,15 @@ func TestAppRevisionHasChanged(t *testing.T) { {"dev target revision, dev, did not touch head", getSource("dev"), "dev", false, true}, {"refs/heads/dev target revision, master, touched head", getSource("refs/heads/dev"), "master", true, false}, {"refs/heads/dev target revision, dev, did not touch head", getSource("refs/heads/dev"), "dev", false, true}, + {"refs/tags/dev target revision, dev, did not touch head", getSource("refs/tags/dev"), "dev", false, true}, {"env/test target revision, env/test, did not touch head", getSource("env/test"), "env/test", false, true}, {"refs/heads/env/test target revision, env/test, did not touch head", getSource("refs/heads/env/test"), "env/test", false, true}, + {"refs/tags/env/test target revision, env/test, did not touch head", getSource("refs/tags/env/test"), "env/test", false, true}, + {"three/part/rev target revision, rev, did not touch head", getSource("three/part/rev"), "rev", false, false}, + {"1.* target revision (matching), 1.1.0, did not touch head", getSource("1.*"), "1.1.0", false, true}, + {"refs/tags/1.* target revision (matching), 1.1.0, did not touch head", getSource("refs/tags/1.*"), "1.1.0", false, true}, + {"1.* target revision (not matching), 2.0.0, did not touch head", getSource("1.*"), "2.0.0", false, false}, + {"1.* target revision, dev (not semver), did not touch head", getSource("1.*"), "dev", false, false}, } for _, tc := range testCases {