[fix] split query param to ref|tag|sha

cmd/commands/app.go

@@ -170,9 +170,7 @@ func setAppOptsDefaults(ctx context.Context, repofs fs.FS, opts *AppCreateOption
 		// local directory
 		fsys = fs.Create(osfs.New(opts.AppOpts.AppSpecifier))
 	} else {
-		host, orgRepo, p, _, _, _, _ := util.ParseGitUrl(opts.AppOpts.AppSpecifier)
-		url := host + orgRepo
-		log.G().Infof("cloning repo: '%s', to infer app type from path '%s'", url, p)
+		log.G().Infof("trying to infer application type from '%s'", opts.AppOpts.AppSpecifier)
 		cloneOpts := &git.CloneOptions{
 			Repo: opts.AppOpts.AppSpecifier,
 			Auth: opts.CloneOpts.Auth,

cmd/commands/repo.go

@@ -165,7 +165,7 @@ func NewRepoBootstrapCommand() *cobra.Command {
 
 	<BIN> repo bootstrap --repo https://github.com/example/repo/path/to/installation_root
 `),
-		PreRun: func(cmd *cobra.Command, args []string) { cloneOpts.Parse() },
+		PreRun: func(_ *cobra.Command, _ []string) { cloneOpts.Parse() },
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return RunRepoBootstrap(cmd.Context(), &RepoBootstrapOptions{
 				AppSpecifier:     appSpecifier,
@@ -264,6 +264,7 @@ func RunRepoCreate(ctx context.Context, opts *RepoCreateOptions) (*git.CloneOpti
 			Password: opts.Token,
 		},
 	}
+
 	co.Parse()
 	return co, nil
 }

cmd/commands/repo_test.go

@@ -56,7 +56,7 @@ func TestRunRepoCreate(t *testing.T) {
 					Repo: "https://github.com/owner/name/path?ref=revision",
 				}
 				expected.Parse()
-				assert.Equal(t, "https://github.com/owner/name", cloneOpts.URL())
+				assert.Equal(t, "https://github.com/owner/name.git", cloneOpts.URL())
 				assert.Equal(t, "revision", cloneOpts.Revision())
 				assert.Equal(t, "path", cloneOpts.Path())
 				mp.AssertCalled(t, "CreateRepository", mock.Anything, mock.Anything)
@@ -247,7 +247,7 @@ func Test_buildBootstrapManifests(t *testing.T) {
 
 				argocdApp := &argocdv1alpha1.Application{}
 				assert.NoError(t, yaml.Unmarshal(b.argocdApp, argocdApp))
-				assert.Equal(t, "https://github.com/foo/bar", argocdApp.Spec.Source.RepoURL)
+				assert.Equal(t, "https://github.com/foo/bar.git", argocdApp.Spec.Source.RepoURL)
 				assert.Equal(t, filepath.Join("installation1", store.Default.BootsrtrapDir, store.Default.ArgoCDName), argocdApp.Spec.Source.Path)
 				assert.Equal(t, "main", argocdApp.Spec.Source.TargetRevision)
 				assert.Equal(t, 0, len(argocdApp.ObjectMeta.Finalizers))
@@ -256,7 +256,7 @@ func Test_buildBootstrapManifests(t *testing.T) {
 
 				bootstrapApp := &argocdv1alpha1.Application{}
 				assert.NoError(t, yaml.Unmarshal(b.bootstrapApp, bootstrapApp))
-				assert.Equal(t, "https://github.com/foo/bar", bootstrapApp.Spec.Source.RepoURL)
+				assert.Equal(t, "https://github.com/foo/bar.git", bootstrapApp.Spec.Source.RepoURL)
 				assert.Equal(t, filepath.Join("installation1", store.Default.BootsrtrapDir), bootstrapApp.Spec.Source.Path)
 				assert.Equal(t, "main", bootstrapApp.Spec.Source.TargetRevision)
 				assert.NotEqual(t, 0, len(bootstrapApp.ObjectMeta.Finalizers))
@@ -265,7 +265,7 @@ func Test_buildBootstrapManifests(t *testing.T) {
 
 				rootApp := &argocdv1alpha1.Application{}
 				assert.NoError(t, yaml.Unmarshal(b.rootApp, rootApp))
-				assert.Equal(t, "https://github.com/foo/bar", rootApp.Spec.Source.RepoURL)
+				assert.Equal(t, "https://github.com/foo/bar.git", rootApp.Spec.Source.RepoURL)
 				assert.Equal(t, filepath.Join("installation1", store.Default.ProjectsDir), rootApp.Spec.Source.Path)
 				assert.Equal(t, "main", rootApp.Spec.Source.TargetRevision)
 				assert.NotEqual(t, 0, len(rootApp.ObjectMeta.Finalizers))
@@ -296,7 +296,6 @@ func Test_buildBootstrapManifests(t *testing.T) {
 	for tname, tt := range tests {
 		t.Run(tname, func(t *testing.T) {
 			tt.args.cloneOpts.Parse()
-
 			b, ret := buildBootstrapManifests(
 				tt.args.namespace,
 				tt.args.appSpecifier,

pkg/application/application.go

@@ -10,10 +10,10 @@ import (
 	"reflect"
 
 	"github.com/argoproj-labs/argocd-autopilot/pkg/fs"
+	"github.com/argoproj-labs/argocd-autopilot/pkg/git"
 	"github.com/argoproj-labs/argocd-autopilot/pkg/kube"
 	"github.com/argoproj-labs/argocd-autopilot/pkg/log"
 	"github.com/argoproj-labs/argocd-autopilot/pkg/store"
-	"github.com/argoproj-labs/argocd-autopilot/pkg/util"
 
 	"github.com/ghodss/yaml"
 	billyUtils "github.com/go-git/go-billy/v5/util"
@@ -388,17 +388,18 @@ func newDirApp(opts *CreateOptions) *dirApp {
 	app := &dirApp{
 		baseApp: baseApp{opts},
 	}
-
-	host, orgRepo, path, gitRef, _, _, _ := util.ParseGitUrl(opts.AppSpecifier)
-	url := host + orgRepo
+	cloneOpts := &git.CloneOptions{
+		Repo: opts.AppSpecifier,
+	}
+	cloneOpts.Parse()
 	app.config = &Config{
 		AppName:           opts.AppName,
 		UserGivenName:     opts.AppName,
 		DestNamespace:     opts.DestNamespace,
 		DestServer:        opts.DestServer,
-		SrcRepoURL:        url,
-		SrcPath:           path,
-		SrcTargetRevision: gitRef,
+		SrcRepoURL:        cloneOpts.URL(),
+		SrcPath:           cloneOpts.Path(),
+		SrcTargetRevision: cloneOpts.Revision(),
 	}
 
 	return app

pkg/application/application_test.go

@@ -605,7 +605,7 @@ func Test_newDirApp(t *testing.T) {
 					UserGivenName:     "fooapp",
 					DestNamespace:     "fizz",
 					DestServer:        "buzz",
-					SrcRepoURL:        "https://github.com/foo/bar",
+					SrcRepoURL:        "https://github.com/foo/bar.git",
 					SrcTargetRevision: "v0.1.2",
 					SrcPath:           "somepath/in/repo",
 				},

pkg/git/repository.go

@@ -113,10 +113,11 @@ func (o *CloneOptions) Parse() {
 	var (
 		host    string
 		orgRepo string
+		suffix  string
 	)
 
-	host, orgRepo, o.path, o.revision, _, _, _ = util.ParseGitUrl(o.Repo)
-	o.url = host + orgRepo
+	host, orgRepo, o.path, o.revision, suffix = util.ParseGitUrl(o.Repo)
+	o.url = host + orgRepo + suffix
 }
 
 func (o *CloneOptions) Clone(ctx context.Context) (Repository, fs.FS, error) {
@@ -153,7 +154,7 @@ func (o *CloneOptions) URL() string {
 }
 
 func (o *CloneOptions) Revision() string {
-	return o.revision
+	return plumbing.ReferenceName(o.revision).Short()
 }
 
 func (o *CloneOptions) Path() string {
@@ -208,7 +209,7 @@ var clone = func(ctx context.Context, opts *CloneOptions) (*repo, error) {
 	}
 
 	if opts.revision != "" {
-		cloneOpts.ReferenceName = plumbing.NewBranchReferenceName(opts.revision)
+		cloneOpts.ReferenceName = plumbing.ReferenceName(opts.revision)
 	}
 
 	log.G(ctx).WithFields(log.Fields{

pkg/git/repository_test.go

@@ -247,7 +247,7 @@ func Test_clone(t *testing.T) {
 				Repo: "https://github.com/owner/name",
 			},
 			expectedOpts: &gg.CloneOptions{
-				URL:      "https://github.com/owner/name",
+				URL:      "https://github.com/owner/name.git",
 				Auth:     nil,
 				Depth:    1,
 				Progress: os.Stderr,
@@ -266,7 +266,7 @@ func Test_clone(t *testing.T) {
 				},
 			},
 			expectedOpts: &gg.CloneOptions{
-				URL: "https://github.com/owner/name",
+				URL: "https://github.com/owner/name.git",
 				Auth: &http.BasicAuth{
 					Username: "asd",
 					Password: "123",
@@ -284,7 +284,7 @@ func Test_clone(t *testing.T) {
 				Repo: "https://github.com/owner/name",
 			},
 			expectedOpts: &gg.CloneOptions{
-				URL:      "https://github.com/owner/name",
+				URL:      "https://github.com/owner/name.git",
 				Depth:    1,
 				Progress: os.Stderr,
 				Tags:     gg.NoTags,
@@ -300,7 +300,7 @@ func Test_clone(t *testing.T) {
 				Repo: "https://github.com/owner/name?ref=test",
 			},
 			expectedOpts: &gg.CloneOptions{
-				URL:           "https://github.com/owner/name",
+				URL:           "https://github.com/owner/name.git",
 				Depth:         1,
 				Progress:      os.Stderr,
 				Tags:          gg.NoTags,

pkg/util/repospec.go

@@ -8,7 +8,6 @@ package util
 
 import (
 	"net/url"
-	"strconv"
 	"strings"
 	"time"
 )
@@ -21,42 +20,43 @@ const (
 // From strings like [email protected]:someOrg/someRepo.git or
 // https://github.com/someOrg/someRepo?ref=someHash, extract
 // the parts.
-func ParseGitUrl(n string) (
-	host string, orgRepo string, path string, gitRef string, gitSubmodules bool, gitSuff string, gitTimeout time.Duration) {
-
+func ParseGitUrl(n string) (host, orgRepo, path, ref, gitSuff string) {
 	if strings.Contains(n, gitDelimiter) {
 		index := strings.Index(n, gitDelimiter)
 		// Adding _git/ to host
 		host = normalizeGitHostSpec(n[:index+len(gitDelimiter)])
 		orgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0]
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):])
+		path, ref = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):])
 		return
 	}
+
 	host, n = parseHostSpec(n)
 	gitSuff = gitSuffix
 	if strings.Contains(n, gitSuffix) {
 		index := strings.Index(n, gitSuffix)
 		orgRepo = n[0:index]
 		n = n[index+len(gitSuffix):]
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
+		path, ref = peelQuery(n)
 		return
 	}
 
 	i := strings.Index(n, "/")
 	if i < 1 {
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
+		path, ref = peelQuery(n)
 		return
 	}
+
 	j := strings.Index(n[i+1:], "/")
 	if j >= 0 {
 		j += i + 1
 		orgRepo = n[:j]
-		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[j+1:])
+		path, ref = peelQuery(n[j+1:])
 		return
 	}
+
 	path = ""
-	orgRepo, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
-	return host, orgRepo, path, gitRef, gitSubmodules, gitSuff, gitTimeout
+	orgRepo, ref = peelQuery(n)
+	return
 }
 
 // Clone git submodules by default.
@@ -65,45 +65,25 @@ const defaultSubmodules = true
 // Arbitrary, but non-infinite, timeout for running commands.
 const defaultTimeout = 27 * time.Second
 
-func peelQuery(arg string) (string, string, time.Duration, bool) {
-	// Parse the given arg into a URL. In the event of a parse failure, return
-	// our defaults.
+func peelQuery(arg string) (path, ref string) {
 	parsed, err := url.Parse(arg)
 	if err != nil {
-		return arg, "", defaultTimeout, defaultSubmodules
-	}
-	values := parsed.Query()
-
-	// ref is the desired git ref to target. Can be specified by in a git URL
-	// with ?ref=<string> or ?version=<string>, although ref takes precedence.
-	ref := values.Get("version")
-	if queryValue := values.Get("ref"); queryValue != "" {
-		ref = queryValue
-	}
-
-	// depth is the desired git exec timeout. Can be specified by in a git URL
-	// with ?timeout=<duration>.
-	duration := defaultTimeout
-	if queryValue := values.Get("timeout"); queryValue != "" {
-		// Attempt to first parse as a number of integer seconds (like "61"),
-		// and then attempt to parse as a suffixed duration (like "61s").
-		if intValue, err := strconv.Atoi(queryValue); err == nil && intValue > 0 {
-			duration = time.Duration(intValue) * time.Second
-		} else if durationValue, err := time.ParseDuration(queryValue); err == nil && durationValue > 0 {
-			duration = durationValue
-		}
+		return path, ""
 	}
 
-	// submodules indicates if git submodule cloning is desired. Can be
-	// specified by in a git URL with ?submodules=<bool>.
-	submodules := defaultSubmodules
-	if queryValue := values.Get("submodules"); queryValue != "" {
-		if boolValue, err := strconv.ParseBool(queryValue); err == nil {
-			submodules = boolValue
-		}
+	values := parsed.Query()
+	branch := values.Get("ref")
+	tag := values.Get("tag")
+	sha := values.Get("sha")
+	if sha != "" {
+		ref = sha
+	} else if tag != "" {
+		ref = "refs/tags/" + tag
+	} else if branch != "" {
+		ref = "refs/heads/" + branch
 	}
 
-	return parsed.Path, ref, duration, submodules
+	return parsed.Path, ref
 }
 
 func parseHostSpec(n string) (string, string) {