refactor(pkguri)!: Parse ref and pkg as query params (#62)

* refactor(pkguri)!: Parse ref and pkg as query params

* fix: tests

* fix: tests and docs

* fix: tests

* fix: Update string representation

README.md

@@ -97,7 +97,7 @@ name = "b"
 
 [[pipeline]]
 name = "example"
-repo_uri = "git@github.com:bluebrown/example.git@main"
+repo_uri = "git@github.com:bluebrown/example.git?ref=main"
 channels = ["a", "b"]
 ```
 
@@ -352,7 +352,7 @@ for github, using the `GITHUB_TOKEN` environment variable, to authenticate.
 [[pipeline]]
 name = "my-github-pr"
 channels = ["example"]
-repo_uri = "git@github.com:bluebrown/foobar.git@main/manifests"
+repo_uri = "git@github.com:bluebrown/foobar.git?ref=main&pkg=manifests"
 dest_branch = "kobold"
 post_hook = "builtin.github-pr@v1"
 ```
@@ -368,12 +368,12 @@ the pull request is merged.
 [[pipeline]]
 name = "stage"
 channels = ["distribution"]
-repo_uri = "http://gitea/dev/test.git@main/stage"
+repo_uri = "http://gitea/dev/test.git?ref=main&pkg=stage"
 
 [[pipeline]]
 name = "prod"
 channels = ["distribution"]
-repo_uri = "http://gitea/dev/test.git@main/prod"
+repo_uri = "http://gitea/dev/test.git?ref=main&pkg=prod"
 dest_branch = "release"
 post_hook = "builtin.gitea-pr@v1"
 ```

config/config_test.go

@@ -28,7 +28,7 @@ func TestReadDir(t *testing.T) {
 					RepoURI: git.PackageURI{
 						Repo: "git@github.com:bluebrown/testing",
 						Ref:  "dev",
-						Pkg:  "/resources",
+						Pkg:  "resources",
 					},
 					Channels: []string{"foo"},
 				}},

config/confix/convert.go

@@ -68,7 +68,7 @@ func MakeConfig(v1 *old.NormalizedConfig) (*config.Config, error) {
 			}
 
 			uri := git.PackageURI{}
-			if err := uri.UnmarshalText([]byte(fmt.Sprintf("%s@%s%s", repo.URL, sub.Branch, scope))); err != nil {
+			if err := uri.UnmarshalText([]byte(fmt.Sprintf("%s?ref=%s&pkg=%s", repo.URL, sub.Branch, strings.TrimPrefix(scope, "/")))); err != nil {
 				fmt.Printf("[WARN] sub=%q repo=%q scope=%q: invalid uri, skipping!\n",
 					sub.Name, repo.Name, scope)
 				continue

config/confix/testdata/want.toml

@@ -18,35 +18,35 @@ decoder = "builtin.lines@v1"
 
 [[pipeline]]
 name = "e2e"
-repo_uri = "http://gitea.local:3000/kobold/kobold-test.git@main"
+repo_uri = "http://gitea.local:3000/kobold/kobold-test.git?ref=main"
 dest_branch = ""
 channels = ["acr", "dockerhub", "distribution", "generic"]
 post_hook = ""
 
 [[pipeline]]
 name = "pr-test"
-repo_uri = "http://gitea.local:3000/kobold/kobold-test.git@master/test"
+repo_uri = "http://gitea.local:3000/kobold/kobold-test.git?ref=master&pkg=test"
 dest_branch = "kobold"
 channels = ["acr"]
 post_hook = ""
 
 [[pipeline]]
 name = "pr-prod"
-repo_uri = "http://gitea.local:3000/kobold/kobold-test.git@master/prod"
+repo_uri = "http://gitea.local:3000/kobold/kobold-test.git?ref=master&pkg=prod"
 dest_branch = "kobold"
 channels = ["acr"]
 post_hook = ""
 
 [[pipeline]]
 name = "azure"
-repo_uri = "https://myorg@dev.azure.com/myorg/myproject/_git/myrepo.git@main"
+repo_uri = "https://myorg@dev.azure.com/myorg/myproject/_git/myrepo.git?ref=main"
 dest_branch = ""
 channels = ["distribution"]
 post_hook = ""
 
 [[pipeline]]
 name = "azure2"
-repo_uri = "https://myorg@dev.azure.com/myorg/myproject/_git/myrepo2.git@main"
+repo_uri = "https://myorg@dev.azure.com/myorg/myproject/_git/myrepo2.git?ref=main"
 dest_branch = "kobold"
 channels = ["acr", "generic"]
 post_hook = "builtin.ado-pr@v1"

config/testdata/split/pipeline.toml

@@ -1,5 +1,5 @@
 version = "2"
 [[pipeline]]
 name = "bar"
-repo_uri = "git@github.com:bluebrown/testing.git@dev/resources"
+repo_uri = "git@github.com:bluebrown/testing.git?ref=dev&pkg=resources"
 channels = ["foo"]

e2e/kobold/kobold.d/repo.toml

@@ -3,11 +3,11 @@ version = "2"
 [[pipeline]]
 name = "stage"
 channels = ["lines", "distribution"]
-repo_uri = "http://gitea/dev/test.git@main/stage"
+repo_uri = "http://gitea/dev/test.git?ref=main&pkg=stage"
 
 [[pipeline]]
 name = "prod"
 channels = ["lines", "distribution"]
-repo_uri = "http://gitea/dev/test.git@main/prod"
+repo_uri = "http://gitea/dev/test.git?ref=main&pkg=prod"
 dest_branch = "release"
 post_hook = "builtin.gitea-pr@v1"

git/pkguri.go

@@ -3,14 +3,14 @@ package git
 import (
 	"database/sql/driver"
 	"fmt"
-	"regexp"
+	"net/url"
 	"strings"
 )
 
 // the package uri is a special uri format to specify a git repo, ref, and
 // package path within the repo under the ref. The uri is of the form:
 //
-//	<repo>@<ref>[<pkg>]
+//	<repo>?=ref=<ref>&pkg=<pkg>]
 //
 // where <repo> is the git repo uri, <ref> is the git ref (branch, tag, commit),
 // and <pkg> is the package path within the repo. If <pkg> is not specified, the
@@ -23,35 +23,34 @@ type PackageURI struct {
 
 // FIXME: appending .git can lead to confusion or invalid URIs.
 func (uri *PackageURI) String() string {
-	return fmt.Sprintf("%s.git@%s%s", uri.Repo, uri.Ref, uri.Pkg)
+	return fmt.Sprintf("%s.git?ref=%s&pkg=%s", uri.Repo, uri.Ref, uri.Pkg)
 }
 
-var pattern = regexp.MustCompile(`^(?P<repo>.*)@(?P<ref>\w+)(?P<pkg>\/.+)?$`)
-
 func (uri *PackageURI) MustUnmarshalText(s string) {
 	if err := uri.UnmarshalText([]byte(s)); err != nil {
 		panic(err)
 	}
 }
 
 func (uri *PackageURI) UnmarshalText(b []byte) error {
-	matches := pattern.FindStringSubmatch(string(b))
-	if len(matches) == 0 {
-		return fmt.Errorf("invalid git package uri: %q", string(b))
+	var packageURIString = string(b)
+	if !strings.Contains(packageURIString, "?") {
+		return fmt.Errorf("invalid git package uri: %q, query params are missing", string(b))
+	}
+	var repoParts = strings.Split(packageURIString, "?")
+	var repo = repoParts[0]
+	var queryParams, err = url.ParseQuery(repoParts[1])
+	if err != nil {
+		return fmt.Errorf("invalid git package uri: %q, could not parse query params", string(b))
 	}
-	for i, name := range pattern.SubexpNames() {
-		if i == 0 {
-			continue
-		}
-		switch name {
-		case "repo":
-			uri.Repo = strings.TrimSuffix(matches[i], ".git")
-		case "ref":
-			uri.Ref = matches[i]
-		case "pkg":
-			uri.Pkg = strings.TrimSuffix(matches[i], "/")
-		}
+	var ref = queryParams.Get("ref")
+	if ref == "" {
+		return fmt.Errorf("invalid git package uri: %q, missing ref query param", string(b))
 	}
+	var pkg = queryParams.Get("pkg")
+	uri.Repo = strings.TrimSuffix(repo, ".git")
+	uri.Ref = ref
+	uri.Pkg = pkg
 	return nil
 }
 

git/pkguri_test.go

@@ -0,0 +1,105 @@
+package git
+
+import "testing"
+
+func TestPackageURI_UnmarshalText(t *testing.T) {
+	type fields struct {
+		Repo string
+		Ref  string
+		Pkg  string
+	}
+	type args struct {
+		b []byte
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "Valid url with package",
+			fields: fields{
+				Repo: "git@github.com:some-org/some-repo",
+				Ref:  "main",
+				Pkg:  "some-pkg",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git?ref=main&pkg=some-pkg"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Valid url with package specified first",
+			fields: fields{
+				Repo: "git@github.com:some-org/some-repo",
+				Ref:  "main",
+				Pkg:  "some-pkg",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git?pkg=some-pkg&ref=main"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Valid url only uses first declaration of query params",
+			fields: fields{
+				Repo: "git@github.com:some-org/some-repo",
+				Ref:  "main",
+				Pkg:  "some-pkg",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git?ref=main&pkg=some-pkg&ref=fail&pkg=fail"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Valid url without package",
+			fields: fields{
+				Repo: "git@github.com:some-org/some-repo",
+				Ref:  "main",
+				Pkg:  "",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git?ref=main"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "Missing query params",
+			fields: fields{
+				Repo: "",
+				Ref:  "",
+				Pkg:  "",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git"),
+			},
+			wantErr: true,
+		},
+		{
+			name: "Missing ref",
+			fields: fields{
+				Repo: "",
+				Ref:  "",
+				Pkg:  "",
+			},
+			args: args{
+				b: []byte("git@github.com:some-org/some-repo.git?pkg=some-pkg"),
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			uri := &PackageURI{
+				Repo: tt.fields.Repo,
+				Ref:  tt.fields.Ref,
+				Pkg:  tt.fields.Pkg,
+			}
+			if err := uri.UnmarshalText(tt.args.b); (err != nil) != tt.wantErr {
+				t.Errorf("UnmarshalText() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

testdata/kobold.toml

@@ -14,14 +14,14 @@ decoder = "builtin.dockerhub@v1"
 [[pipeline]]
 name = "github-prod"
 channels = ["gh", "dockerhub"]
-repo_uri = "git@github.com:bluebrown/foobar.git@test/manifests"
+repo_uri = "git@github.com:bluebrown/foobar.git?ref=test&pkg=manifests"
 dest_branch = "kobold"
 post_hook = "builtin.github-pr@v1"
 
 [[pipeline]]
 name = "github-test"
 channels = ["gh", "dockerhub"]
-repo_uri = "git@github.com:bluebrown/foobar.git@main/manifests"
+repo_uri = "git@github.com:bluebrown/foobar.git?ref=main&pkg=manifests"
 
 [[channel]]
 name = "ado"
@@ -30,11 +30,11 @@ decoder = "builtin.dstribution@v1"
 [[pipeline]]
 name = "azure-devops-stage"
 channels = ["ado"]
-repo_uri = "$ADO_HOST/kobold-test.git@main/stage"
+repo_uri = "$ADO_HOST/kobold-test.git?ref=main&pkg=stage"
 
 [[pipeline]]
 name = "azure-devops-prod"
 channels = ["ado"]
-repo_uri = "$ADO_HOST/kobold-test.git@main/prod"
+repo_uri = "$ADO_HOST/kobold-test.git?ref=main&pkg=prod"
 dest_branch = "kobold"
 post_hook = "builtin.ado-pr@v1"