From e70af7a3e5b10da5f983680eea31b1ceab499aaa Mon Sep 17 00:00:00 2001 From: Pablo Aguilar Date: Wed, 5 Jun 2024 21:25:06 -0300 Subject: [PATCH] feat: changes git client to resolve semantic versioning tags (#17566) * feat: changes git client to resolve semantic versioning tags Signed-off-by: Pablo Aguilar * docs: update documentation Signed-off-by: Pablo Aguilar * feat: simplify `resolveSemverRevision` method Signed-off-by: Pablo Aguilar * chore: add two more test cases Signed-off-by: Pablo Aguilar * chore: update `resolveSemverRevision` behavior Signed-off-by: Pablo Aguilar * chore: add end to end test Signed-off-by: Pablo Aguilar * chore: fix end to end test Signed-off-by: Pablo Aguilar * chore: improve semver constraint e2e testing Signed-off-by: Pablo Aguilar --------- Signed-off-by: Pablo Aguilar Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com> --- docs/user-guide/tracking_strategies.md | 12 ++- test/e2e/git_test.go | 52 ++++++++++++ util/git/client.go | 77 ++++++++++++++++-- util/git/git_test.go | 107 +++++++++++++++++++------ 4 files changed, 215 insertions(+), 33 deletions(-) create mode 100644 test/e2e/git_test.go diff --git a/docs/user-guide/tracking_strategies.md b/docs/user-guide/tracking_strategies.md index 57dfc5f907b65..9cfc63811b6b4 100644 --- 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 new file mode 100644 index 0000000000000..62258a4b7bdcd --- /dev/null +++ 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 index b356f95639d43..d04482b2ff208 100644 --- 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 index 5a088c241b605..8c9a5d29c3110 100644 --- 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.