feat: changes git client to resolve semantic versioning tags (argopro…

…j#17566) * feat: changes git client to resolve semantic versioning tags Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * docs: update documentation Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * feat: simplify `resolveSemverRevision` method Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * chore: add two more test cases Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * chore: update `resolveSemverRevision` behavior Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * chore: add end to end test Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * chore: fix end to end test Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> * chore: improve semver constraint e2e testing Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> --------- Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
rumstead · Jun 6, 2024 · e70af7a · e70af7a
1 parent 236eb95
commit e70af7a
Show file tree

Hide file tree

Showing 4 changed files with 215 additions and 33 deletions.
diff --git a/docs/user-guide/tracking_strategies.md b/docs/user-guide/tracking_strategies.md
@@ -25,14 +25,15 @@ Helm chart versions are [Semantic Versions](https://semver.org/). As a result, y
 
 ## Git
 
-For Git, all versions are Git references:
+For Git, all versions are Git references but tags [Semantic Versions](https://semver.org/) can also be used:
 
 | Use Case | How | Notes |
 |-|-|-|
 | Pin to a version (e.g. in production) | Either (a) tag the commit with (e.g. `v1.2.0`) and use that tag, or (b) using commit SHA. | See [commit pinning](#commit-pinning). |
-| Track patches (e.g. in pre-production) | Tag/re-tag the commit, e.g. (e.g. `v1.2`) and use that tag. | See [tag tracking](#tag-tracking) |
-| Track minor releases (e.g. in QA) | Re-tag the commit as (e.g. `v1`) and use that tag. | See [tag tracking](#tag-tracking) |
-| Use the latest (e.g. in local development) | Use `HEAD` or `master` (assuming `master` is your master branch). | See [HEAD / Branch Tracking](#head-branch-tracking) |
+| Track patches (e.g. in pre-production) | Use a range (e.g. `1.2.*` or `>=1.2.0 <1.3.0`)                                           | See [tag tracking](#tag-tracking) |
+| Track minor releases (e.g. in QA) | Use a range (e.g. `1.*` or `>=1.0.0 <2.0.0`)                                             | See [tag tracking](#tag-tracking) |
+| Use the latest (e.g. in local development) | Use `HEAD` or `master` (assuming `master` is your master branch).                        | See [HEAD / Branch Tracking](#head-branch-tracking) |
+| Use the latest including pre-releases | Use star range with `-0` suffix | `*-0` or `>=0.0.0-0` |
 
 
 ### HEAD / Branch Tracking
@@ -53,6 +54,9 @@ To redeploy an app, the user uses Git to change the meaning of a tag by retaggin
 different commit SHA. Argo CD will detect the new meaning of the tag when performing the
 comparison/sync.
 
+But if you're using semantic versioning you can set the constraint in your service revision
+and Argo CD will get the latest version following the constraint rules.
+
 ### Commit Pinning
 
 If a Git commit SHA is specified, the app is effectively pinned to the manifests defined at

diff --git a/test/e2e/git_test.go b/test/e2e/git_test.go
@@ -0,0 +1,52 @@
+package e2e
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/argoproj/argo-cd/v2/test/e2e/fixture"
+	v1 "k8s.io/api/core/v1"
+
+	. "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	. "github.com/argoproj/argo-cd/v2/test/e2e/fixture/app"
+)
+
+func TestGitSemverResolutionNotUsingConstraint(t *testing.T) {
+	Given(t).
+		Path("deployment").
+		CustomSSHKnownHostsAdded().
+		SSHRepoURLAdded(true).
+		RepoURLType(fixture.RepoURLTypeSSH).
+		Revision("v0.1.0").
+		When().
+		AddTag("v0.1.0").
+		CreateApp().
+		Sync().
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeSynced))
+}
+
+func TestGitSemverResolutionUsingConstraint(t *testing.T) {
+	Given(t).
+		Path("deployment").
+		CustomSSHKnownHostsAdded().
+		SSHRepoURLAdded(true).
+		RepoURLType(fixture.RepoURLTypeSSH).
+		Revision("v0.1.*").
+		When().
+		AddTag("v0.1.0").
+		CreateApp().
+		Sync().
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		When().
+		PatchFile("deployment.yaml", `[
+	{"op": "replace", "path": "/metadata/name", "value": "new-app"},
+	{"op": "replace", "path": "/spec/replicas", "value": 1}
+]`).
+		AddTag("v0.1.2").
+		Sync().
+		Then().
+		Expect(SyncStatusIs(SyncStatusCodeSynced)).
+		Expect(Pod(func(p v1.Pod) bool { return strings.HasPrefix(p.Name, "new-app") }))
+}
diff --git a/util/git/client.go b/util/git/client.go
@@ -2,6 +2,7 @@ package git
 
 import (
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"math"
 	"net/http"
@@ -16,6 +17,8 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/Masterminds/semver/v3"
+
 	argoexec "github.com/argoproj/pkg/exec"
 	"github.com/bmatcuk/doublestar/v4"
 	"github.com/go-git/go-git/v5"
@@ -576,11 +579,11 @@ func (m *nativeGitClient) LsRefs() (*Refs, error) {
 	return sortedRefs, nil
 }
 
-// LsRemote resolves the commit SHA of a specific branch, tag, or HEAD. If the supplied revision
-// does not resolve, and "looks" like a 7+ hexadecimal commit SHA, it return the revision string.
-// Otherwise, it returns an error indicating that the revision could not be resolved. This method
-// runs with in-memory storage and is safe to run concurrently, or to be run without a git
-// repository locally cloned.
+// LsRemote resolves the commit SHA of a specific branch, tag (with semantic versioning or not),
+// or HEAD. If the supplied revision does not resolve, and "looks" like a 7+ hexadecimal commit SHA,
+// it will return the revision string. Otherwise, it returns an error indicating that the revision could
+// not be resolved. This method runs with in-memory storage and is safe to run concurrently,
+// or to be run without a git repository locally cloned.
 func (m *nativeGitClient) LsRemote(revision string) (res string, err error) {
 	for attempt := 0; attempt < maxAttemptsCount; attempt++ {
 		res, err = m.lsRemote(revision)
@@ -607,19 +610,31 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 	}
 
 	refs, err := m.getRefs()
-
 	if err != nil {
 		return "", err
 	}
+
 	if revision == "" {
 		revision = "HEAD"
 	}
+
+	semverSha, err := m.resolveSemverRevision(revision, refs)
+	if err != nil {
+		return "", err
+	}
+
+	if semverSha != "" {
+		return semverSha, nil
+	}
+
 	// refToHash keeps a maps of remote refs to their hash
 	// (e.g. refs/heads/master -> a67038ae2e9cb9b9b16423702f98b41e36601001)
 	refToHash := make(map[string]string)
+
 	// refToResolve remembers ref name of the supplied revision if we determine the revision is a
 	// symbolic reference (like HEAD), in which case we will resolve it from the refToHash map
 	refToResolve := ""
+
 	for _, ref := range refs {
 		refName := ref.Name().String()
 		hash := ref.Hash().String()
@@ -637,6 +652,7 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 			}
 		}
 	}
+
 	if refToResolve != "" {
 		// If refToResolve is non-empty, we are resolving symbolic reference (e.g. HEAD).
 		// It should exist in our refToHash map
@@ -645,16 +661,65 @@ func (m *nativeGitClient) lsRemote(revision string) (string, error) {
 			return hash, nil
 		}
 	}
+
 	// We support the ability to use a truncated commit-SHA (e.g. first 7 characters of a SHA)
 	if IsTruncatedCommitSHA(revision) {
 		log.Debugf("revision '%s' assumed to be commit sha", revision)
 		return revision, nil
 	}
+
 	// If we get here, revision string had non hexadecimal characters (indicating its a branch, tag,
 	// or symbolic ref) and we were unable to resolve it to a commit SHA.
 	return "", fmt.Errorf("Unable to resolve '%s' to a commit SHA", revision)
 }
 
+// resolveSemverRevision is a part of the lsRemote method workflow.
+// When the user configure correctly the Git repository revision and the revision is a valid semver constraint
+// only the for loop in this function will run, otherwise the lsRemote loop will try to resolve the revision.
+// Some examples to illustrate the actual behavior, if:
+// * The revision is "v0.1.*"/"0.1.*" or "v0.1.2"/"0.1.2" and there's a tag matching that constraint only this function loop will run;
+// * The revision is "v0.1.*"/"0.1.*" or "0.1.2"/"0.1.2" and there is no tag matching that constraint this function loop and lsRemote loop will run for backward compatibility;
+// * The revision is "custom-tag" only the lsRemote loop will run because that revision is an invalid semver;
+// * The revision is "master-branch" only the lsRemote loop will run because that revision is an invalid semver;
+func (m *nativeGitClient) resolveSemverRevision(revision string, refs []*plumbing.Reference) (string, error) {
+	constraint, err := semver.NewConstraint(revision)
+	if err != nil {
+		return "", nil
+	}
+
+	maxVersion := semver.New(0, 0, 0, "", "")
+	maxVersionHash := plumbing.ZeroHash
+	for _, ref := range refs {
+		if !ref.Name().IsTag() {
+			continue
+		}
+
+		tag := ref.Name().Short()
+		version, err := semver.NewVersion(tag)
+		if err != nil {
+			if errors.Is(err, semver.ErrInvalidSemVer) {
+				log.Debugf("Invalid semantic version: %s", tag)
+				continue
+			}
+
+			return "", fmt.Errorf("error parsing version for tag: %w", err)
+		}
+
+		if constraint.Check(version) {
+			if version.GreaterThan(maxVersion) {
+				maxVersion = version
+				maxVersionHash = ref.Hash()
+			}
+		}
+	}
+
+	if maxVersionHash.IsZero() {
+		return "", nil
+	}
+
+	return maxVersionHash.String(), nil
+}
+
 // CommitSHA returns current commit sha from `git rev-parse HEAD`
 func (m *nativeGitClient) CommitSHA() (string, error) {
 	out, err := m.runCmd("rev-parse", "HEAD")

diff --git a/util/git/git_test.go b/util/git/git_test.go
@@ -212,34 +212,95 @@ func TestCustomHTTPClient(t *testing.T) {
 func TestLsRemote(t *testing.T) {
 	clnt, err := NewClientExt("https://github.com/argoproj/argo-cd.git", "/tmp", NopCreds{}, false, false, "")
 	assert.NoError(t, err)
-	xpass := []string{
-		"HEAD",
-		"master",
-		"release-0.8",
-		"v0.8.0",
-		"4e22a3cb21fa447ca362a05a505a69397c8a0d44",
-		//"4e22a3c",
+
+	testCases := []struct {
+		name           string
+		revision       string
+		expectedCommit string
+	}{
+		{
+			name:     "should resolve symbolic link reference",
+			revision: "HEAD",
+		},
+		{
+			name:     "should resolve branch name",
+			revision: "master",
+		},
+		{
+			name:           "should resolve tag without semantic versioning",
+			revision:       "release-0.8",
+			expectedCommit: "ff87d8cb9e669d3738434733ecba3c6dd2c64d70",
+		},
+		{
+			name:           "should resolve a pined tag with semantic versioning",
+			revision:       "v0.8.0",
+			expectedCommit: "d7c04ae24c16f8ec611b0331596fbc595537abe9",
+		},
+		{
+			name:           "should resolve a pined tag with semantic versioning without the 'v' prefix",
+			revision:       "0.8.0",
+			expectedCommit: "d7c04ae24c16f8ec611b0331596fbc595537abe9",
+		},
+		{
+			name:           "should resolve a range tag with semantic versioning",
+			revision:       "v0.8.*", // it should resolve to v0.8.2
+			expectedCommit: "e5eefa2b943ae14a3e4491d4e35ef082e1c2a3f4",
+		},
+		{
+			name:           "should resolve a range tag with semantic versioning without the 'v' prefix",
+			revision:       "0.8.*", // it should resolve to v0.8.2
+			expectedCommit: "e5eefa2b943ae14a3e4491d4e35ef082e1c2a3f4",
+		},
+		{
+			name:           "should resolve a conditional range tag with semantic versioning",
+			revision:       ">=v2.9.0 <2.10.4", // it should resolve to v2.10.3
+			expectedCommit: "0fd6344537eb948cff602824a1d060421ceff40e",
+		},
+		{
+			name:     "should resolve a star range tag with semantic versioning",
+			revision: "*",
+		},
+		{
+			name:     "should resolve a star range suffixed tag with semantic versioning",
+			revision: "*-0",
+		},
+		{
+			name:           "should resolve commit sha",
+			revision:       "4e22a3cb21fa447ca362a05a505a69397c8a0d44",
+			expectedCommit: "4e22a3cb21fa447ca362a05a505a69397c8a0d44",
+		},
 	}
-	for _, revision := range xpass {
-		commitSHA, err := clnt.LsRemote(revision)
-		assert.NoError(t, err)
-		assert.True(t, IsCommitSHA(commitSHA))
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			commitSHA, err := clnt.LsRemote(tc.revision)
+			assert.NoError(t, err)
+			assert.True(t, IsCommitSHA(commitSHA))
+			if tc.expectedCommit != "" {
+				assert.Equal(t, tc.expectedCommit, commitSHA)
+			}
+		})
 	}
 
 	// We do not resolve truncated git hashes and return the commit as-is if it appears to be a commit
-	commitSHA, err := clnt.LsRemote("4e22a3c")
-	assert.NoError(t, err)
-	assert.False(t, IsCommitSHA(commitSHA))
-	assert.True(t, IsTruncatedCommitSHA(commitSHA))
+	t.Run("truncated commit", func(t *testing.T) {
+		commitSHA, err := clnt.LsRemote("4e22a3c")
+		assert.NoError(t, err)
+		assert.False(t, IsCommitSHA(commitSHA))
+		assert.True(t, IsTruncatedCommitSHA(commitSHA))
+	})
+
+	t.Run("unresolvable revisions", func(t *testing.T) {
+		xfail := []string{
+			"unresolvable",
+			"4e22a3", // too short (6 characters)
+		}
 
-	xfail := []string{
-		"unresolvable",
-		"4e22a3", // too short (6 characters)
-	}
-	for _, revision := range xfail {
-		_, err := clnt.LsRemote(revision)
-		assert.Error(t, err)
-	}
+		for _, revision := range xfail {
+			_, err := clnt.LsRemote(revision)
+			assert.ErrorContains(t, err, "Unable to resolve")
+		}
+	})
 }
 
 // Running this test requires git-lfs to be installed on your machine.