Skip to content

Expand Git repo URL normalization #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merenbach merged 13 commits into from
May 5, 2018
Merged

Expand Git repo URL normalization #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
79dd184
Expand unit tests for Git functions
May 4, 2018
6db617a
Update tests for IsSSHUrl
May 4, 2018
fd9dc00
Add TODO
May 4, 2018
114c495
Add EnsureSuffix tests
May 4, 2018
b48ef75
Add EnsureSuffix function
May 4, 2018
92e2fa0
Lowercase repo name in secret name
May 4, 2018
8318ef9
Expand normalization and related tests
May 4, 2018
144c63c
Add tests for EnsurePrefix
May 4, 2018
41b8000
Rm redundant strings.ToLower
May 4, 2018
1f72a0c
Update repository names to fix broken tests
May 4, 2018
6977801
Expand tests some more to include missing .git suffix, thanks @jessesuen
May 5, 2018
65ef380
Add additional repository tests, thanks @jessesuen
May 5, 2018
25f6fb0
Fix typo in comment
May 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions server/repository/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ func (s *Server) Delete(ctx context.Context, q *RepoQuery) (*RepoResponse, error
// repoURLToSecretName hashes repo URL to the secret name using a formula.
// Part of the original repo name is incorporated for debugging purposes
func repoURLToSecretName(repo string) string {
repo = git.NormalizeGitURL(repo)
repo = strings.ToLower(git.NormalizeGitURL(repo))
h := fnv.New32a()
_, _ = h.Write([]byte(repo))
parts := strings.Split(strings.TrimSuffix(repo, ".git"), "/")
return fmt.Sprintf("repo-%s-%v", strings.ToLower(parts[len(parts)-1]), h.Sum32())
return fmt.Sprintf("repo-%s-%v", parts[len(parts)-1], h.Sum32())
}

// repoToStringData converts a repository object to string data for serialization to a secret
Expand Down
9 changes: 5 additions & 4 deletions server/repository/repository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import "testing"

func TestRepoURLToSecretName(t *testing.T) {
tables := map[string]string{
"git://git@github.com:argoproj/ARGO-cd.git": "repo-argo-cd-83273445",
"https://github.com/argoproj/ARGO-cd": "repo-argo-cd-1890113693",
"https://github.com/argoproj/argo-cd": "repo-argo-cd-42374749",
"ssh://git@github.com/argoproj/argo-cd.git": "repo-argo-cd-3569564120",
"git://git@github.com:argoproj/ARGO-cd.git": "repo-argo-cd-593837413",
"https://github.com/argoproj/ARGO-cd": "repo-argo-cd-821842295",
"https://github.com/argoproj/argo-cd": "repo-argo-cd-821842295",
Copy link
Copy Markdown
Member

@jessesuen jessesuen May 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add one where "https://github.com/argoproj/argo-cd.git" also maps to "repo-argo-cd-821842295"

"https://github.com/argoproj/argo-cd.git": "repo-argo-cd-821842295",
"ssh://git@github.com/argoproj/argo-cd.git": "repo-argo-cd-1019298066",
}

for k, v := range tables {
Expand Down
35 changes: 31 additions & 4 deletions util/git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,45 @@ import (
"strings"
)

// EnsurePrefix idempotently ensures that a base string has a given prefix.
func ensurePrefix(s, prefix string) string {
if !strings.HasPrefix(s, prefix) {
s = prefix + s
}
return s
}

// EnsureSuffix idempotently ensures that a base string has a given suffix.
func ensureSuffix(s, suffix string) string {
if !strings.HasSuffix(s, suffix) {
s += suffix
}
return s
}

// NormalizeGitURL normalizes a git URL for lookup and storage
func NormalizeGitURL(repo string) string {
// preprocess
repo = ensureSuffix(repo, ".git")
if IsSshURL(repo) {
repo = ensurePrefix(repo, "ssh://")
}

// process
repoURL, err := url.Parse(repo)
if err != nil {
return strings.ToLower(repo)
return ""
}
return repoURL.String()

// postprocess
repoURL.Host = strings.ToLower(repoURL.Host)
normalized := repoURL.String()
return strings.TrimPrefix(normalized, "ssh://")
}

// IsSshURL returns true is supplied URL is SSH URL
// IsSshURL returns true if supplied URL is SSH URL
func IsSshURL(url string) bool {
return strings.Index(url, "git@") == 0
return strings.HasPrefix(url, "git@") || strings.HasPrefix(url, "ssh://")
}

// GetGitCommandOptions returns URL and env options for git operation
Expand Down
70 changes: 65 additions & 5 deletions util/git/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,74 @@ import (
"github.com/stretchr/testify/assert"
)

func TestReturnsTrueForSSHUrl(t *testing.T) {
assert.True(t, IsSshURL("git@github.com:test.git"))
func TestEnsurePrefix(t *testing.T) {
data := [][]string{
{"world", "hello", "helloworld"},
{"helloworld", "hello", "helloworld"},
{"example.com", "https://", "https://example.com"},
{"https://example.com", "https://", "https://example.com"},
{"cd", "argo", "argocd"},
{"argocd", "argo", "argocd"},
{"", "argocd", "argocd"},
{"argocd", "", "argocd"},
}
for _, table := range data {
result := ensurePrefix(table[0], table[1])
assert.Equal(t, table[2], result)
}
}

func TestReturnsFalseForNonSSHUrl(t *testing.T) {
assert.False(t, IsSshURL("https://github.com/test.git"))
func TestEnsureSuffix(t *testing.T) {
data := [][]string{
{"hello", "world", "helloworld"},
{"helloworld", "world", "helloworld"},
{"repo", ".git", "repo.git"},
{"repo.git", ".git", "repo.git"},
{"", "repo.git", "repo.git"},
{"argo", "cd", "argocd"},
{"argocd", "cd", "argocd"},
{"argocd", "", "argocd"},
{"", "argocd", "argocd"},
}
for _, table := range data {
result := ensureSuffix(table[0], table[1])
assert.Equal(t, table[2], result)
}
}

func TestIsSSHUrl(t *testing.T) {
data := map[string]bool{
"git://github.com/argoproj/test.git": false,
"git@GITHUB.com:argoproj/test.git": true,
"git@github.com:test": true,
"git@github.com:test.git": true,
"https://github.com/argoproj/test": false,
"https://github.com/argoproj/test.git": false,
"ssh://git@GITHUB.com:argoproj/test": true,
"ssh://git@GITHUB.com:argoproj/test.git": true,
"ssh://git@github.com:test.git": true,
}
for k, v := range data {
assert.Equal(t, v, IsSshURL(k))
}
}

func TestNormalizeUrl(t *testing.T) {
assert.Equal(t, NormalizeGitURL("git@GITHUB.com:test.git"), "git@github.com:test.git")
data := map[string]string{
"git@GITHUB.com:argoproj/test": "git@github.com:argoproj/test.git",
"git@GITHUB.com:argoproj/test.git": "git@github.com:argoproj/test.git",
"git@GITHUB.com:test": "git@github.com:test.git",
"git@GITHUB.com:test.git": "git@github.com:test.git",
"https://GITHUB.com/argoproj/test": "https://github.com/argoproj/test.git",
"https://GITHUB.com/argoproj/test.git": "https://github.com/argoproj/test.git",
"https://github.com/TEST": "https://github.com/TEST.git",
"https://github.com/TEST.git": "https://github.com/TEST.git",
"ssh://git@GITHUB.com:argoproj/test": "git@github.com:argoproj/test.git",
"ssh://git@GITHUB.com:argoproj/test.git": "git@github.com:argoproj/test.git",
"ssh://git@GITHUB.com:test.git": "git@github.com:test.git",
"ssh://git@github.com:test": "git@github.com:test.git",
}
for k, v := range data {
assert.Equal(t, v, NormalizeGitURL(k))
}
}