From dbe8bcecd4c5702f560aafa2dc35935292777239 Mon Sep 17 00:00:00 2001 From: david may <1301201+wass3r@users.noreply.github.com> Date: Fri, 19 Jan 2024 15:14:53 +0000 Subject: [PATCH] fix(scm): prefer starlark file for starlark pipeline (#1043) * fix(scm): prefer starlark file for starlark pipeline fixes https://github.com/go-vela/community/issues/488 * cuddlemania --------- Co-authored-by: Easton Crupper <65553218+ecrupper@users.noreply.github.com> --- scm/github/repo.go | 6 +- scm/github/repo_test.go | 164 ++++++++++++++++------ scm/github/testdata/pipeline.star | 14 ++ scm/github/testdata/py.json | 4 +- scm/github/testdata/star.json | 2 +- scm/github/testdata/yml_bad_encoding.json | 18 +++ 6 files changed, 157 insertions(+), 51 deletions(-) create mode 100644 scm/github/testdata/pipeline.star create mode 100644 scm/github/testdata/yml_bad_encoding.json diff --git a/scm/github/repo.go b/scm/github/repo.go index b89900f90..11d5ee002 100644 --- a/scm/github/repo.go +++ b/scm/github/repo.go @@ -57,10 +57,12 @@ func (c *client) Config(ctx context.Context, u *library.User, r *library.Repo, r // create GitHub OAuth client with user's token client := c.newClientToken(*u.Token) + // default pipeline file names files := []string{".vela.yml", ".vela.yaml"} + // starlark support - prefer .star/.py, use default as fallback if strings.EqualFold(r.GetPipelineType(), constants.PipelineTypeStarlark) { - files = append(files, ".vela.star", ".vela.py") + files = append([]string{".vela.star", ".vela.py"}, files...) } // set the reference for the options to capture the pipeline configuration @@ -575,8 +577,8 @@ func (c *client) GetBranch(ctx context.Context, u *library.User, r *library.Repo client := c.newClientToken(u.GetToken()) maxRedirects := 3 - data, _, err := client.Repositories.GetBranch(ctx, r.GetOrg(), r.GetName(), branch, maxRedirects) + data, _, err := client.Repositories.GetBranch(ctx, r.GetOrg(), r.GetName(), branch, maxRedirects) if err != nil { return "", "", err } diff --git a/scm/github/repo_test.go b/scm/github/repo_test.go index afca2bb4a..997c793d6 100644 --- a/scm/github/repo_test.go +++ b/scm/github/repo_test.go @@ -27,14 +27,14 @@ func TestGithub_Config_YML(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - if c.Param("path") == ".vela.yaml" { - c.Status(http.StatusNotFound) + if c.Param("path") == ".vela.yml" { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/yml.json") return } - c.Header("Content-Type", "application/json") - c.Status(http.StatusOK) - c.File("testdata/yml.json") + c.Status(http.StatusNotFound) }) s := httptest.NewServer(engine) @@ -79,16 +79,23 @@ func TestGithub_ConfigBackoff_YML(t *testing.T) { resp := httptest.NewRecorder() _, engine := gin.CreateTestContext(resp) + // counter for api calls + count := 0 + // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - if c.Param("path") == ".vela.yaml" { - c.Status(http.StatusNotFound) + // load the yml file on the second api call + if c.Param("path") == ".vela.yml" && count != 0 { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/yml.json") return } - c.Header("Content-Type", "application/json") - c.Status(http.StatusOK) - c.File("testdata/yml.json") + c.Status(http.StatusNotFound) + + // increment api call counter + count++ }) s := httptest.NewServer(engine) @@ -96,7 +103,7 @@ func TestGithub_ConfigBackoff_YML(t *testing.T) { want, err := os.ReadFile("testdata/pipeline.yml") if err != nil { - t.Errorf("Config reading file returned err: %v", err) + t.Errorf("ConfigBackoff reading file returned err: %v", err) } // setup types @@ -111,22 +118,22 @@ func TestGithub_ConfigBackoff_YML(t *testing.T) { client, _ := NewTest(s.URL) // run test - got, err := client.Config(context.TODO(), u, r, "") + got, err := client.ConfigBackoff(context.TODO(), u, r, "") if resp.Code != http.StatusOK { - t.Errorf("Config returned %v, want %v", resp.Code, http.StatusOK) + t.Errorf("ConfigBackoff returned %v, want %v", resp.Code, http.StatusOK) } if err != nil { - t.Errorf("Config returned err: %v", err) + t.Errorf("ConfigBackoff returned err: %v", err) } if !reflect.DeepEqual(got, want) { - t.Errorf("Config is %v, want %v", got, want) + t.Errorf("ConfigBackoff is %v, want %v", got, want) } } -func TestGithub_Config_YML_BadRequest(t *testing.T) { +func TestGithub_Config_YAML(t *testing.T) { // setup context gin.SetMode(gin.TestMode) @@ -135,12 +142,24 @@ func TestGithub_Config_YML_BadRequest(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - c.Status(http.StatusBadRequest) + if c.Param("path") == ".vela.yaml" { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/yaml.json") + return + } + + c.Status(http.StatusNotFound) }) s := httptest.NewServer(engine) defer s.Close() + want, err := os.ReadFile("testdata/pipeline.yml") + if err != nil { + t.Errorf("Config reading file returned err: %v", err) + } + // setup types u := new(library.User) u.SetName("foo") @@ -159,16 +178,16 @@ func TestGithub_Config_YML_BadRequest(t *testing.T) { t.Errorf("Config returned %v, want %v", resp.Code, http.StatusOK) } - if err == nil { - t.Error("Config should have returned err") + if err != nil { + t.Errorf("Config returned err: %v", err) } - if got != nil { - t.Errorf("Config is %v, want nil", got) + if !reflect.DeepEqual(got, want) { + t.Errorf("Config is %v, want %v", got, want) } } -func TestGithub_Config_YAML(t *testing.T) { +func TestGithub_Config_Star(t *testing.T) { // setup context gin.SetMode(gin.TestMode) @@ -177,20 +196,20 @@ func TestGithub_Config_YAML(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - if c.Param("path") == ".vela.yml" { - c.Status(http.StatusNotFound) + if c.Param("path") == ".vela.star" { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/star.json") return } - c.Header("Content-Type", "application/json") - c.Status(http.StatusOK) - c.File("testdata/yaml.json") + c.Status(http.StatusNotFound) }) s := httptest.NewServer(engine) defer s.Close() - want, err := os.ReadFile("testdata/pipeline.yml") + want, err := os.ReadFile("testdata/pipeline.star") if err != nil { t.Errorf("Config reading file returned err: %v", err) } @@ -203,6 +222,7 @@ func TestGithub_Config_YAML(t *testing.T) { r := new(library.Repo) r.SetOrg("foo") r.SetName("bar") + r.SetPipelineType(constants.PipelineTypeStarlark) client, _ := NewTest(s.URL) @@ -222,7 +242,7 @@ func TestGithub_Config_YAML(t *testing.T) { } } -func TestGithub_Config_Star(t *testing.T) { +func TestGithub_Config_Star_Prefer(t *testing.T) { // setup context gin.SetMode(gin.TestMode) @@ -231,20 +251,25 @@ func TestGithub_Config_Star(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - if c.Param("path") == ".vela.yml" { + // repo has .vela.yml and .vela.star + switch c.Param("path") { + case ".vela.yml": + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/yml.json") + case ".vela.star": + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/star.json") + default: c.Status(http.StatusNotFound) - return } - - c.Header("Content-Type", "application/json") - c.Status(http.StatusOK) - c.File("testdata/star.json") }) s := httptest.NewServer(engine) defer s.Close() - want, err := os.ReadFile("testdata/pipeline.yml") + want, err := os.ReadFile("testdata/pipeline.star") if err != nil { t.Errorf("Config reading file returned err: %v", err) } @@ -286,20 +311,20 @@ func TestGithub_Config_Py(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { - if c.Param("path") == ".vela.yml" { - c.Status(http.StatusNotFound) + if c.Param("path") == ".vela.py" { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/py.json") return } - c.Header("Content-Type", "application/json") - c.Status(http.StatusOK) - c.File("testdata/py.json") + c.Status(http.StatusNotFound) }) s := httptest.NewServer(engine) defer s.Close() - want, err := os.ReadFile("testdata/pipeline.yml") + want, err := os.ReadFile("testdata/pipeline.star") if err != nil { t.Errorf("Config reading file returned err: %v", err) } @@ -341,11 +366,13 @@ func TestGithub_Config_YAML_BadRequest(t *testing.T) { // setup mock server engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { + // first default not found if c.Param("path") == ".vela.yml" { c.Status(http.StatusNotFound) return } + // second default (.vela.yaml) causes bad request c.Status(http.StatusBadRequest) }) @@ -421,6 +448,55 @@ func TestGithub_Config_NotFound(t *testing.T) { } } +func TestGithub_Config_BadEncoding(t *testing.T) { + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + _, engine := gin.CreateTestContext(resp) + + // setup mock server + engine.GET("/api/v3/repos/foo/bar/contents/:path", func(c *gin.Context) { + if c.Param("path") == ".vela.yml" { + c.Header("Content-Type", "application/json") + c.Status(http.StatusOK) + c.File("testdata/yml_bad_encoding.json") + return + } + + c.Status(http.StatusNotFound) + }) + + s := httptest.NewServer(engine) + defer s.Close() + + // setup types + u := new(library.User) + u.SetName("foo") + u.SetToken("bar") + + r := new(library.Repo) + r.SetOrg("foo") + r.SetName("bar") + + client, _ := NewTest(s.URL) + + // run test + got, err := client.Config(context.TODO(), u, r, "") + + if resp.Code != http.StatusOK { + t.Errorf("Config returned %v, want %v", resp.Code, http.StatusOK) + } + + if err == nil { + t.Error("Config should have returned err") + } + + if got != nil { + t.Errorf("Config is %v, want nil", got) + } +} + func TestGithub_Disable(t *testing.T) { // setup context gin.SetMode(gin.TestMode) @@ -1277,7 +1353,6 @@ func TestGithub_ListUserRepos(t *testing.T) { // run test got, err := client.ListUserRepos(context.TODO(), u) - if err != nil { t.Errorf("Status returned err: %v", err) } @@ -1315,7 +1390,6 @@ func TestGithub_ListUserRepos_Ineligible(t *testing.T) { // run test got, err := client.ListUserRepos(context.TODO(), u) - if err != nil { t.Errorf("Status returned err: %v", err) } @@ -1360,7 +1434,6 @@ func TestGithub_GetPullRequest(t *testing.T) { // run test gotCommit, gotBranch, gotBaseRef, gotHeadRef, err := client.GetPullRequest(context.TODO(), u, r, 1) - if err != nil { t.Errorf("Status returned err: %v", err) } @@ -1417,7 +1490,6 @@ func TestGithub_GetBranch(t *testing.T) { // run test gotBranch, gotCommit, err := client.GetBranch(context.TODO(), u, r, "main") - if err != nil { t.Errorf("Status returned err: %v", err) } diff --git a/scm/github/testdata/pipeline.star b/scm/github/testdata/pipeline.star new file mode 100644 index 000000000..c55266655 --- /dev/null +++ b/scm/github/testdata/pipeline.star @@ -0,0 +1,14 @@ +def main(ctx): + return { + 'version': '1', + 'steps': [ + { + 'name': 'build', + 'image': 'golang:latest', + 'commands': [ + 'go build', + 'go test', + ] + }, + ], +} diff --git a/scm/github/testdata/py.json b/scm/github/testdata/py.json index cd33aa29e..79d4a3d5b 100644 --- a/scm/github/testdata/py.json +++ b/scm/github/testdata/py.json @@ -4,7 +4,7 @@ "size": 5362, "name": ".vela.py", "path": ".vela.py", - "content": "LS0tCnZlcnNpb246ICIxIgoKbWV0YWRhdGE6CiAgb3M6IGxpbnV4CgpzdGVwczoKICAtIG5hbWU6IGJ1aWxkCiAgICBpbWFnZTogb3BlbmpkazpsYXRlc3QKICAgIHB1bGw6IHRydWUKICAgIGVudmlyb25tZW50OgogICAgICBHUkFETEVfVVNFUl9IT01FOiAuZ3JhZGxlCiAgICAgIEdSQURMRV9PUFRTOiAtRG9yZy5ncmFkbGUuZGFlbW9uPWZhbHNlIC1Eb3JnLmdyYWRsZS53b3JrZXJzLm1heD0xIC1Eb3JnLmdyYWRsZS5wYXJhbGxlbD1mYWxzZQogICAgY29tbWFuZHM6CiAgICAgIC0gLi9ncmFkbGV3IGJ1aWxkIGRpc3RUYXIK", + "content": "ZGVmIG1haW4oY3R4KToKICByZXR1cm4gewogICAgJ3ZlcnNpb24nOiAnMScsCiAgICAnc3RlcHMnOiBbCiAgICAgIHsKICAgICAgICAnbmFtZSc6ICdidWlsZCcsCiAgICAgICAgJ2ltYWdlJzogJ2dvbGFuZzpsYXRlc3QnLAogICAgICAgICdjb21tYW5kcyc6IFsKICAgICAgICAgICdnbyBidWlsZCcsCiAgICAgICAgICAnZ28gdGVzdCcsCiAgICAgICAgXQogICAgICB9LAogICAgXSwKfQo=", "sha": "3d21ec53a331a6f037a91c368710b99387d012c1", "url": "https://api.github.com/repos/octokit/octokit.rb/contents/.vela.py", "git_url": "https://api.github.com/repos/octokit/octokit.rb/git/blobs/3d21ec53a331a6f037a91c368710b99387d012c1", @@ -15,4 +15,4 @@ "self": "https://api.github.com/repos/octokit/octokit.rb/contents/.vela.py", "html": "https://github.com/octokit/octokit.rb/blob/main/.vela.py" } -} +} \ No newline at end of file diff --git a/scm/github/testdata/star.json b/scm/github/testdata/star.json index b6d909593..5832c7e9d 100644 --- a/scm/github/testdata/star.json +++ b/scm/github/testdata/star.json @@ -4,7 +4,7 @@ "size": 5362, "name": ".vela.star", "path": ".vela.star", - "content": "LS0tCnZlcnNpb246ICIxIgoKbWV0YWRhdGE6CiAgb3M6IGxpbnV4CgpzdGVwczoKICAtIG5hbWU6IGJ1aWxkCiAgICBpbWFnZTogb3BlbmpkazpsYXRlc3QKICAgIHB1bGw6IHRydWUKICAgIGVudmlyb25tZW50OgogICAgICBHUkFETEVfVVNFUl9IT01FOiAuZ3JhZGxlCiAgICAgIEdSQURMRV9PUFRTOiAtRG9yZy5ncmFkbGUuZGFlbW9uPWZhbHNlIC1Eb3JnLmdyYWRsZS53b3JrZXJzLm1heD0xIC1Eb3JnLmdyYWRsZS5wYXJhbGxlbD1mYWxzZQogICAgY29tbWFuZHM6CiAgICAgIC0gLi9ncmFkbGV3IGJ1aWxkIGRpc3RUYXIK", + "content": "ZGVmIG1haW4oY3R4KToKICByZXR1cm4gewogICAgJ3ZlcnNpb24nOiAnMScsCiAgICAnc3RlcHMnOiBbCiAgICAgIHsKICAgICAgICAnbmFtZSc6ICdidWlsZCcsCiAgICAgICAgJ2ltYWdlJzogJ2dvbGFuZzpsYXRlc3QnLAogICAgICAgICdjb21tYW5kcyc6IFsKICAgICAgICAgICdnbyBidWlsZCcsCiAgICAgICAgICAnZ28gdGVzdCcsCiAgICAgICAgXQogICAgICB9LAogICAgXSwKfQo=", "sha": "3d21ec53a331a6f037a91c368710b99387d012c1", "url": "https://api.github.com/repos/octokit/octokit.rb/contents/.vela.star", "git_url": "https://api.github.com/repos/octokit/octokit.rb/git/blobs/3d21ec53a331a6f037a91c368710b99387d012c1", diff --git a/scm/github/testdata/yml_bad_encoding.json b/scm/github/testdata/yml_bad_encoding.json new file mode 100644 index 000000000..66029bb15 --- /dev/null +++ b/scm/github/testdata/yml_bad_encoding.json @@ -0,0 +1,18 @@ +{ + "type": "file", + "encoding": "none", + "size": 5362, + "name": ".vela.yml", + "path": ".vela.yml", + "content": "LS0tCnZlcnNpb246ICIxIgoKbWV0YWRhdGE6CiAgb3M6IGxpbnV4CgpzdGVwczoKICAtIG5hbWU6IGJ1aWxkCiAgICBpbWFnZTogb3BlbmpkazpsYXRlc3QKICAgIHB1bGw6IHRydWUKICAgIGVudmlyb25tZW50OgogICAgICBHUkFETEVfVVNFUl9IT01FOiAuZ3JhZGxlCiAgICAgIEdSQURMRV9PUFRTOiAtRG9yZy5ncmFkbGUuZGFlbW9uPWZhbHNlIC1Eb3JnLmdyYWRsZS53b3JrZXJzLm1heD0xIC1Eb3JnLmdyYWRsZS5wYXJhbGxlbD1mYWxzZQogICAgY29tbWFuZHM6CiAgICAgIC0gLi9ncmFkbGV3IGJ1aWxkIGRpc3RUYXIK", + "sha": "3d21ec53a331a6f037a91c368710b99387d012c1", + "url": "https://api.github.com/repos/octokit/octokit.rb/contents/.vela.yml", + "git_url": "https://api.github.com/repos/octokit/octokit.rb/git/blobs/3d21ec53a331a6f037a91c368710b99387d012c1", + "html_url": "https://github.com/octokit/octokit.rb/blob/main/.vela.yml", + "download_url": "https://raw.githubusercontent.com/octokit/octokit.rb/main/.vela.yml", + "_links": { + "git": "https://api.github.com/repos/octokit/octokit.rb/git/blobs/3d21ec53a331a6f037a91c368710b99387d012c1", + "self": "https://api.github.com/repos/octokit/octokit.rb/contents/.vela.yml", + "html": "https://github.com/octokit/octokit.rb/blob/main/.vela.yml" + } +} \ No newline at end of file