fix: Check for semver constraint matching in application webhook handler (cherry-pick #21648) (#22507)

Signed-off-by: eadred <[email protected]>
Co-authored-by: Eadred <[email protected]>

Author: gcp-cherry-pick-bot[bot]

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 {

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 {