Skip to content
This repository has been archived by the owner on Jun 16, 2020. It is now read-only.

Commit

Permalink
Merge branch 'bugfix/gitlab_url_encoding'
Browse files Browse the repository at this point in the history
  • Loading branch information
@nbedos
nbedos committed Jan 10, 2020
2 parents 7e4080b + 977cadb commit 74b6b21
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 93 deletions.
4 changes: 2 additions & 2 deletions docs/cistern.man.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<meta name="generator" content="pandoc" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=yes" />
<meta name="author" content="Nicolas Bedos" />
<title>CISTERN(1) | version 0.2.0dev6-5-g52ca5f9-dirty-linux-amd64</title>
<title>CISTERN(1) | version 0.2.0dev0-6-g0c92981-dirty-linux-amd64</title>
<style>
code{white-space: pre-wrap;}
span.smallcaps{font-variant: small-caps;}
Expand All @@ -19,7 +19,7 @@
</head>
<body>
<header id="title-block-header">
<h1 class="title">CISTERN(1) | version 0.2.0dev6-5-g52ca5f9-dirty-linux-amd64</h1>
<h1 class="title">CISTERN(1) | version 0.2.0dev0-6-g0c92981-dirty-linux-amd64</h1>
<p class="author">Nicolas Bedos</p>
</header>
<h1 id="name">NAME</h1>
Expand Down
9 changes: 8 additions & 1 deletion providers/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,19 @@ func (c GitHubClient) ID() string {
}

func (c GitHubClient) parseRepositoryURL(url string) (string, string, error) {
host, owner, repo, err := utils.RepoHostOwnerAndName(url)
host, slug, err := utils.RepositoryHostAndSlug(url)
expectedHost := strings.TrimPrefix(c.client.BaseURL.Hostname(), "api.")
if err != nil || !strings.Contains(host, expectedHost) {
return "", "", ErrUnknownRepositoryURL
}

components := strings.FieldsFunc(slug, func(c rune) bool { return c == '/' })
if len(components) < 2 {
return "", "", fmt.Errorf("invalid repository path: %q (expected at least two components)", slug)
}
owner := components[0]
repo := components[1]

return owner, repo, nil
}

Expand Down
17 changes: 8 additions & 9 deletions providers/gitlab.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,12 @@ func (c GitLabClient) Commit(ctx context.Context, repo string, ref string) (Comm
return Commit{}, ErrUnknownRepositoryURL
}

ref = url.PathEscape(ref)

select {
case <-c.rateLimiter:
case <-ctx.Done():
return Commit{}, ctx.Err()
}
gitlabCommit, _, err := c.remote.Commits.GetCommit(slug, url.PathEscape(ref), gitlab.WithContext(ctx))
gitlabCommit, _, err := c.remote.Commits.GetCommit(slug, ref, gitlab.WithContext(ctx))
if err != nil {
if err, ok := err.(*gitlab.ErrorResponse); ok {
switch err.Response.StatusCode {
Expand Down Expand Up @@ -224,12 +222,11 @@ func (c GitLabClient) BuildFromURL(ctx context.Context, u string) (Pipeline, err
}

func (c GitLabClient) parseRepositoryURL(u string) (string, error) {
host, owner, repo, err := utils.RepoHostOwnerAndName(u)
host, slug, err := utils.RepositoryHostAndSlug(u)
if err != nil || host != c.remote.BaseURL().Hostname() {
return "", ErrUnknownRepositoryURL
}

slug := fmt.Sprintf("%s/%s", url.PathEscape(owner), url.PathEscape(repo))
return slug, nil
}

Expand All @@ -243,14 +240,16 @@ func (c GitLabClient) parsePipelineURL(u string) (string, int, error) {
return "", 0, ErrUnknownPipelineURL
}

// url format: https://gitlab.com/nbedos/cistern/pipelines/97604657
// URL format:
// https://gitlab.com/nbedos/cistern/pipelines/97604657
// OR https://gitlab.com/namespace/nbedos/cistern/pipelines/97604657
pathComponents := strings.FieldsFunc(v.EscapedPath(), func(c rune) bool { return c == '/' })
if len(pathComponents) < 4 || pathComponents[2] != "pipelines" {
if len(pathComponents) < 4 || pathComponents[len(pathComponents)-2] != "pipelines" {
return "", 0, ErrUnknownPipelineURL
}

slug := fmt.Sprintf("%s/%s", pathComponents[0], pathComponents[1])
id, err := strconv.Atoi(pathComponents[3])
slug := strings.Join(pathComponents[:len(pathComponents)-2], "/")
id, err := strconv.Atoi(pathComponents[len(pathComponents)-1])
if err != nil {
return "", 0, err
}
Expand Down
118 changes: 84 additions & 34 deletions providers/gitlab_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,73 @@ import (
)

func TestParsePipelineURL(t *testing.T) {
c, err := NewGitLabClient("gitlab", "gitlab", "", "", 1000)
if err != nil {
t.Fatal(err)
testCases := []struct {
name string
url string
expectedSlug string
expectedID int
}{
{
name: "repository path without namespace",
url: "https://gitlab.com/nbedos/cistern/pipelines/97604657",
expectedSlug: "nbedos/cistern",
expectedID: 97604657,
},
{
name: "repository path with namespace (issue #16)",
url: "https://gitlab.com/namespace/nbedos/cistern/pipelines/97604657",
expectedSlug: "namespace/nbedos/cistern",
expectedID: 97604657,
},
{
name: "repository path with long namespace (issue #16)",
url: "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/97604657",
expectedSlug: "long/namespace/nbedos/cistern",
expectedID: 97604657,
},
}

slug, id, err := c.parsePipelineURL("https://gitlab.com/nbedos/cistern/pipelines/97604657")
if err != nil {
t.Fatal(err)
}
for _, testCase := range testCases {
c, err := NewGitLabClient("gitlab", "gitlab", "", "", 1000)
if err != nil {
t.Fatal(err)
}

if slug != "nbedos/cistern" || id != 97604657 {
t.Fail()
slug, id, err := c.parsePipelineURL(testCase.url)
if err != nil {
t.Fatal(err)
}

if slug != testCase.expectedSlug {
t.Fatalf("expected slug %q but got %q", testCase.expectedSlug, slug)
}

if id != testCase.expectedID {
t.Fatalf("expected id %d but got %d", testCase.expectedID, id)
}
}
}

func setupGitLabTestServer(t *testing.T) (GitLabClient, string, func()) {
func setupGitLabTestServer() (GitLabClient, string, func(), error) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
filename := ""

switch r.URL.Path {
case "/api/v4/projects/nbedos/cistern/pipelines/103230300":
case "/api/v4/projects/long/namespace/nbedos/cistern/pipelines/103230300":
filename = "gitlab_pipeline.json"
case "/api/v4/projects/nbedos/cistern/pipelines/103230300/jobs":
case "/api/v4/projects/long/namespace/nbedos/cistern/pipelines/103230300/jobs":
w.Header().Add("X-Total-Pages", "1")
filename = "gitlab_jobs.json"
case "/api/v4/projects/nbedos/cistern/jobs/42/trace":
case "/api/v4/projects/long/namespace/nbedos/cistern/jobs/42/trace":
filename = "gitlab_log"
case "/api/v4/projects/owner/repo/repository/commits/master":
case "/api/v4/projects/long/namespace/owner/repo/repository/commits/master":
filename = "gitlab_commit.json"
case "/api/v4/projects/owner/repo/repository/commits/a24840cf94b395af69da4a1001d32e3694637e20/refs":
case "/api/v4/projects/long/namespace/owner/repo/repository/commits/a24840cf94b395af69da4a1001d32e3694637e20/refs":
filename = "gitlab_refs.json"
case "/api/v4/projects/nbedos/cistern/pipelines":
case "/api/v4/projects/long/namespace/nbedos/cistern/pipelines":
filename = "gitlab_pipelines.json"
case "/api/v4/projects/nbedos/cistern/repository/commits/a24840cf94b395af69da4a1001d32e3694637e20/statuses":
case "/api/v4/projects/long/namespace/nbedos/cistern/repository/commits/a24840cf94b395af69da4a1001d32e3694637e20/statuses":
filename = "gitlab_statuses.json"
default:
w.WriteHeader(404)
return
Expand All @@ -70,22 +104,26 @@ func setupGitLabTestServer(t *testing.T) (GitLabClient, string, func()) {

gitlabClient := gitlab.NewClient(ts.Client(), "token")
if err := gitlabClient.SetBaseURL(ts.URL); err != nil {
t.Fatal(err)
ts.Close()
return GitLabClient{}, "", nil, err
}

client := GitLabClient{
remote: gitlabClient,
rateLimiter: time.Tick(time.Millisecond),
}

return client, ts.URL, func() { ts.Close() }
return client, ts.URL, func() { ts.Close() }, nil
}

func TestGitLabClient_BuildFromURL(t *testing.T) {
client, testURL, teardown := setupGitLabTestServer(t)
client, testURL, teardown, err := setupGitLabTestServer()
if err != nil {
t.Fatal(err)
}
defer teardown()

pipelineURL := testURL + "/nbedos/cistern/pipelines/103230300"
pipelineURL := testURL + "/long/namespace/nbedos/cistern/pipelines/103230300"
pipeline, err := client.BuildFromURL(context.Background(), pipelineURL)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -117,7 +155,7 @@ func TestGitLabClient_BuildFromURL(t *testing.T) {
},
WebURL: utils.NullString{
Valid: true,
String: "https://gitlab.com/nbedos/cistern/pipelines/103230300",
String: "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103230300",
},
Children: []Step{
{
Expand All @@ -140,7 +178,7 @@ func TestGitLabClient_BuildFromURL(t *testing.T) {
},
WebURL: utils.NullString{
Valid: true,
String: "https://gitlab.com/nbedos/cistern/pipelines/103230300",
String: "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103230300",
},
Children: []Step{
{
Expand All @@ -161,8 +199,8 @@ func TestGitLabClient_BuildFromURL(t *testing.T) {
Valid: true,
Duration: time.Minute + 31*time.Second,
},
WebURL: utils.NullString{Valid: true, String: "https://gitlab.com/nbedos/cistern/-/jobs/379869167"},
Log: Log{Key: "nbedos/cistern"},
WebURL: utils.NullString{Valid: true, String: "https://gitlab.com/long/namespace/nbedos/cistern/-/jobs/379869167"},
Log: Log{Key: "long/namespace/nbedos/cistern"},
},
},
},
Expand All @@ -175,13 +213,16 @@ func TestGitLabClient_BuildFromURL(t *testing.T) {
}

func TestGitLabClient_Log(t *testing.T) {
client, _, teardown := setupGitLabTestServer(t)
client, _, teardown, err := setupGitLabTestServer()
if err != nil {
t.Fatal(err)
}
defer teardown()

step := Step{
ID: "42",
Log: Log{
Key: "nbedos/cistern",
Key: "long/namespace/nbedos/cistern",
},
}
log, err := client.Log(context.Background(), step)
Expand All @@ -196,10 +237,13 @@ func TestGitLabClient_Log(t *testing.T) {

func TestGitLabClient_Commit(t *testing.T) {
t.Run("existing reference", func(t *testing.T) {
client, testURL, teardown := setupGitLabTestServer(t)
client, testURL, teardown, err:= setupGitLabTestServer()
if err != nil {
t.Fatal(err)
}
defer teardown()

commit, err := client.Commit(context.Background(), testURL+"/owner/repo", "master")
commit, err := client.Commit(context.Background(), testURL+"/long/namespace/owner/repo", "master")
if err != nil {
t.Fatal(err)
}
Expand All @@ -221,27 +265,33 @@ func TestGitLabClient_Commit(t *testing.T) {
})

t.Run("non existing commit", func(t *testing.T) {
client, testURL, teardown := setupGitLabTestServer(t)
client, testURL, teardown, err := setupGitLabTestServer()
if err != nil {
t.Fatal(err)
}
defer teardown()

_, err := client.Commit(context.Background(), testURL+"/owner/repo", "0000000")
_, err = client.Commit(context.Background(), testURL+"/long/namespace/owner/repo", "0000000")
if err != ErrUnknownGitReference {
t.Fatal(err)
}
})
}

func TestGitLabClient_RefStatuses(t *testing.T) {
client, testURL, teardown := setupGitLabTestServer(t)
client, testURL, teardown, err := setupGitLabTestServer()
if err != nil {
t.Fatal(err)
}
defer teardown()

statuses, err := client.RefStatuses(context.Background(), testURL+"/nbedos/cistern", "", "a24840cf94b395af69da4a1001d32e3694637e20")
statuses, err := client.RefStatuses(context.Background(), testURL+"/long/namespace/nbedos/cistern", "", "a24840cf94b395af69da4a1001d32e3694637e20")
if err != nil {
t.Fatal(err)
}

expectedStatuses := []string{
"https://gitlab.com/nbedos/cistern/pipelines/103494597",
"https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103494597",
}
sort.Strings(expectedStatuses)
sort.Strings(statuses)
Expand Down
33 changes: 32 additions & 1 deletion providers/test_data/gitlab/gitlab_commit.json
View file
Original file line number Diff line number Diff line change
@@ -1 +1,32 @@
{"id":"a24840cf94b395af69da4a1001d32e3694637e20","short_id":"a24840cf","created_at":"2019-12-16T19:06:43.000+01:00","parent_ids":["78813c9dc24828fd29d1ee1f7b8d0df79ab7b2b5"],"title":"Fix typos","message":"Fix typos\n","author_name":"nbedos","author_email":"[email protected]","authored_date":"2019-12-16T19:06:43.000+01:00","committer_name":"nbedos","committer_email":"[email protected]","committed_date":"2019-12-16T19:06:43.000+01:00","stats":{"additions":5,"deletions":5,"total":10},"status":"success","last_pipeline":{"id":103494597,"sha":"a24840cf94b395af69da4a1001d32e3694637e20","ref":"master","status":"success","created_at":"2019-12-16T18:06:38.656Z","updated_at":"2019-12-16T18:08:12.278Z","web_url":"https://gitlab.com/nbedos/cistern/pipelines/103494597"},"project_id":13620164}
{
"id": "a24840cf94b395af69da4a1001d32e3694637e20",
"short_id": "a24840cf",
"created_at": "2019-12-16T19:06:43.000+01:00",
"parent_ids": [
"78813c9dc24828fd29d1ee1f7b8d0df79ab7b2b5"
],
"title": "Fix typos",
"message": "Fix typos\n",
"author_name": "nbedos",
"author_email": "[email protected]",
"authored_date": "2019-12-16T19:06:43.000+01:00",
"committer_name": "nbedos",
"committer_email": "[email protected]",
"committed_date": "2019-12-16T19:06:43.000+01:00",
"stats": {
"additions": 5,
"deletions": 5,
"total": 10
},
"status": "success",
"last_pipeline": {
"id": 103494597,
"sha": "a24840cf94b395af69da4a1001d32e3694637e20",
"ref": "master",
"status": "success",
"created_at": "2019-12-16T18:06:38.656Z",
"updated_at": "2019-12-16T18:08:12.278Z",
"web_url": "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103494597"
},
"project_id": 13620164
}
4 changes: 2 additions & 2 deletions providers/test_data/gitlab/gitlab_jobs.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@
"status": "success",
"created_at": "2019-12-15T21:46:40.694Z",
"updated_at": "2019-12-15T21:48:13.077Z",
"web_url": "https://gitlab.com/nbedos/cistern/pipelines/103230300"
"web_url": "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103230300"
},
"web_url": "https://gitlab.com/nbedos/cistern/-/jobs/379869167",
"web_url": "https://gitlab.com/long/namespace/nbedos/cistern/-/jobs/379869167",
"artifacts": [
{
"file_type": "trace",
Expand Down
4 changes: 2 additions & 2 deletions providers/test_data/gitlab/gitlab_pipeline.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"status": "success",
"created_at": "2019-12-15T21:46:40.694Z",
"updated_at": "2019-12-15T21:48:13.077Z",
"web_url": "https://gitlab.com/nbedos/cistern/pipelines/103230300",
"web_url": "https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103230300",
"before_sha": "0e04997502c99369e87b7822ffcc2f744cc7b5bb",
"tag": false,
"yaml_errors": null,
Expand All @@ -29,7 +29,7 @@
"group": "success",
"tooltip": "passed",
"has_details": true,
"details_path": "/nbedos/cistern/pipelines/103230300",
"details_path": "/long/namespace/nbedos/cistern/pipelines/103230300",
"illustration": null,
"favicon": "https://gitlab.com/assets/ci_favicons/favicon_status_success-8451333011eee8ce9f2ab25dc487fe24a8758c694827a582f17f42b0a90446a2.png"
}
Expand Down
2 changes: 1 addition & 1 deletion providers/test_data/gitlab/gitlab_pipelines.json
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[{"id":103494597,"sha":"a24840cf94b395af69da4a1001d32e3694637e20","ref":"master","status":"success","created_at":"2019-12-16T18:06:38.656Z","updated_at":"2019-12-16T18:08:12.278Z","web_url":"https://gitlab.com/nbedos/cistern/pipelines/103494597"}]
[{"id":103494597,"sha":"a24840cf94b395af69da4a1001d32e3694637e20","ref":"master","status":"success","created_at":"2019-12-16T18:06:38.656Z","updated_at":"2019-12-16T18:08:12.278Z","web_url":"https://gitlab.com/long/namespace/nbedos/cistern/pipelines/103494597"}]
Loading

0 comments on commit 74b6b21

Please sign in to comment.