From af566e9194d1fe2344c54238c81f76c2d0281023 Mon Sep 17 00:00:00 2001 From: Michael Hoang Date: Tue, 21 Mar 2023 09:02:05 -0400 Subject: [PATCH] cleanup Signed-off-by: Michael Hoang --- README.md | 4 +- pkg/devfile/parser/parse.go | 6 +- pkg/devfile/parser/parse_test.go | 2 +- pkg/util/git.go | 44 +++++++++-- pkg/util/git_test.go | 106 +++++++++++++++++-------- pkg/util/mock.go | 42 ++++++++++ pkg/util/util.go | 30 +++++-- pkg/util/util_test.go | 130 ++++++++++++++++++++++++++++++- 8 files changed, 310 insertions(+), 54 deletions(-) create mode 100644 pkg/util/mock.go diff --git a/README.md b/README.md index d62a1a43..cf67a1a1 100644 --- a/README.md +++ b/README.md @@ -198,10 +198,10 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g url = "https://gitlab.com//" // Parse the repo url - gitUrl, err := util.ParseGitUrl(url) + gitUrl, err := util.NewGitUrl(url) // Clone the repo to a destination dir - err = util.CloneGitRepo(gitUrl, destDir) + err = util.CloneGitRepo(*gitUrl, destDir) ``` ## Projects using devfile/library diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index f6fde6b7..59c05c9d 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -431,7 +431,7 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D d.Ctx = devfileCtx.NewURLDevfileCtx(newUri) if util.IsGitProviderRepo(newUri) { - gitUrl, err := util.ParseGitUrl(newUri) + gitUrl, err := util.NewGitUrl(newUri) if err != nil { return DevfileObj{}, err } @@ -450,7 +450,7 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D return populateAndParseDevfile(d, newResolveCtx, tool, true) } -func getResourcesFromGit(g util.GitUrl, destDir string, httpTimeout *int, repoToken string) error { +func getResourcesFromGit(g *util.GitUrl, destDir string, httpTimeout *int, repoToken string) error { stackDir, err := ioutil.TempDir(os.TempDir(), fmt.Sprintf("git-resources")) if err != nil { return fmt.Errorf("failed to create dir: %s, error: %v", stackDir, err) @@ -464,7 +464,7 @@ func getResourcesFromGit(g util.GitUrl, destDir string, httpTimeout *int, repoTo } } - err = util.CloneGitRepo(g, stackDir) + err = util.CloneGitRepo(*g, stackDir) if err != nil { return err } diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index cf2ef303..7e78988e 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -4207,7 +4207,7 @@ func Test_getResourcesFromGit(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := getResourcesFromGit(tt.gitUrl, tt.destDir, &httpTimeout, "") + err := getResourcesFromGit(&tt.gitUrl, tt.destDir, &httpTimeout, "") if (err != nil) != tt.wantErr { t.Errorf("Expected error: %t, got error: %t", tt.wantErr, err) } diff --git a/pkg/util/git.go b/pkg/util/git.go index ce548d0b..7d1a6075 100644 --- a/pkg/util/git.go +++ b/pkg/util/git.go @@ -31,6 +31,13 @@ const ( BitbucketHost string = "bitbucket.org" ) +type IGitUrl interface { + ParseGitUrl(fullUrl string) error + GetGitRawFileAPI() string + SetToken(token string, httpTimeout *int) error + IsPublic(httpTimeout *int) bool +} + type GitUrl struct { Protocol string // URL scheme Host string // URL domain name @@ -42,23 +49,30 @@ type GitUrl struct { IsFile bool // defines if the URL points to a file in the repo } +// NewGitUrl creates a GitUrl from a string url +func NewGitUrl(url string) (*GitUrl, error) { + g := &GitUrl{} + if err := g.ParseGitUrl(url); err != nil { + return g, err + } + return g, nil +} + // ParseGitUrl extracts information from a support git url // Only supports git repositories hosted on GitHub, GitLab, and Bitbucket -func ParseGitUrl(fullUrl string) (GitUrl, error) { - var g GitUrl - +func (g *GitUrl) ParseGitUrl(fullUrl string) error { err := ValidateURL(fullUrl) if err != nil { - return g, err + return err } parsedUrl, err := url.Parse(fullUrl) if err != nil { - return g, err + return err } if len(parsedUrl.Path) == 0 { - return g, fmt.Errorf("url path should not be empty") + return fmt.Errorf("url path should not be empty") } if parsedUrl.Host == RawGitHubHost || parsedUrl.Host == GitHubHost { @@ -71,7 +85,7 @@ func ParseGitUrl(fullUrl string) (GitUrl, error) { err = fmt.Errorf("url host should be a valid GitHub, GitLab, or Bitbucket host; received: %s", parsedUrl.Host) } - return g, err + return err } func (g *GitUrl) parseGitHubUrl(url *url.URL) error { @@ -242,6 +256,22 @@ func (g *GitUrl) validateToken(params HTTPRequestParams) error { return nil } +// GetGitRawFileAPI returns the endpoint for the git providers raw file +func (g *GitUrl) GetGitRawFileAPI() string { + var apiRawFile string + + switch g.Host { + case GitHubHost, RawGitHubHost: + apiRawFile = fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", g.Owner, g.Repo, g.Branch, g.Path) + case GitLabHost: + apiRawFile = fmt.Sprintf("https://gitlab.com/api/v4/projects/%s%%2F%s/repository/files/%s/raw", g.Owner, g.Repo, g.Path) + case BitbucketHost: + apiRawFile = fmt.Sprintf("https://api.bitbucket.org/2.0/repositories/%s/%s/src/%s/%s", g.Owner, g.Repo, g.Branch, g.Path) + } + + return apiRawFile +} + // IsGitProviderRepo checks if the url matches a repo from a supported git provider func IsGitProviderRepo(url string) bool { if strings.Contains(url, RawGitHubHost) || strings.Contains(url, GitHubHost) || diff --git a/pkg/util/git_test.go b/pkg/util/git_test.go index 86435e00..7cfe6d7b 100644 --- a/pkg/util/git_test.go +++ b/pkg/util/git_test.go @@ -30,11 +30,11 @@ var ( bitbucketToken = "fake-bitbucket-token" ) -func Test_ParseGitUrl(t *testing.T) { +func Test_NewGitUrl(t *testing.T) { tests := []struct { name string url string - wantUrl GitUrl + wantUrl *GitUrl wantErr string }{ { @@ -51,7 +51,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public GitHub repo with root path", url: "https://github.com/devfile/library", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -70,7 +70,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public GitHub repo with file path", url: "https://github.com/devfile/library/blob/main/devfile.yaml", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -84,7 +84,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public GitHub repo with raw file path", url: "https://raw.githubusercontent.com/devfile/library/main/devfile.yaml", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -108,7 +108,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private GitHub repo with token", url: "https://github.com/fake-owner/fake-private-repo", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "github.com", Owner: "fake-owner", @@ -122,7 +122,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private raw GitHub file path with token", url: "https://raw.githubusercontent.com/fake-owner/fake-private-repo/main/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "fake-owner", @@ -137,7 +137,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public GitLab repo with root path", url: "https://gitlab.com/gitlab-org/gitlab-foss", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "gitlab-org", @@ -156,7 +156,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public GitLab repo with file path", url: "https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "gitlab-org", @@ -180,7 +180,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private GitLab repo with token", url: "https://gitlab.com/fake-owner/fake-private-repo", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "fake-owner", @@ -194,7 +194,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private raw GitLab file path with token", url: "https://gitlab.com/fake-owner/fake-private-repo/-/raw/main/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "fake-owner", @@ -209,7 +209,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public Bitbucket repo with root path", url: "https://bitbucket.org/fake-owner/fake-public-repo", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -228,7 +228,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public Bitbucket repo with file path", url: "https://bitbucket.org/fake-owner/fake-public-repo/src/main/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -242,7 +242,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public Bitbucket file path with nested path", url: "https://bitbucket.org/fake-owner/fake-public-repo/src/main/directory/test.txt", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -256,7 +256,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse public Bitbucket repo with raw file path", url: "https://bitbucket.org/fake-owner/fake-public-repo/raw/main/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -285,7 +285,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private Bitbucket repo with token", url: "https://bitbucket.org/fake-owner/fake-private-repo", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -299,7 +299,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse private raw Bitbucket file path with token", url: "https://bitbucket.org/fake-owner/fake-private-repo/raw/main/README.md", - wantUrl: GitUrl{ + wantUrl: &GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -314,7 +314,7 @@ func Test_ParseGitUrl(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ParseGitUrl(tt.url) + got, err := NewGitUrl(tt.url) if (err != nil) != (tt.wantErr != "") { t.Errorf("Unxpected error: %t, want: %v", err, tt.wantUrl) } else if err == nil && !reflect.DeepEqual(got, tt.wantUrl) { @@ -326,23 +326,63 @@ func Test_ParseGitUrl(t *testing.T) { } } -// todo: try mocking -func Test_SetToken(t *testing.T) { - g := GitUrl{ - Protocol: "https", - Host: "github.com", - Owner: "devfile", - Repo: "library", - Branch: "main", - token: "", +func Test_GetGitRawFileAPI(t *testing.T) { + tests := []struct { + name string + g GitUrl + want string + }{ + { + name: "Github url", + g: GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "devfile", + Repo: "library", + Branch: "main", + Path: "tests/README.md", + }, + want: "https://raw.githubusercontent.com/devfile/library/main/tests/README.md", + }, + { + name: "GitLab url", + g: GitUrl{ + Protocol: "https", + Host: "gitlab.com", + Owner: "gitlab-org", + Repo: "gitlab", + Branch: "master", + Path: "README.md", + }, + want: "https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab/repository/files/README.md/raw", + }, + { + name: "Bitbucket url", + g: GitUrl{ + Protocol: "https", + Host: "bitbucket.org", + Owner: "owner", + Repo: "repo-name", + Branch: "main", + Path: "path/to/file.md", + }, + want: "https://api.bitbucket.org/2.0/repositories/owner/repo-name/src/main/path/to/file.md", + }, + { + name: "Empty GitUrl", + g: GitUrl{}, + want: "", + }, } - httpTimeout := 0 - token := "fake-git-token" - - err := g.SetToken(token, &httpTimeout) - assert.NoError(t, err) - assert.Equal(t, token, g.token) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.g.GetGitRawFileAPI() + if !reflect.DeepEqual(result, tt.want) { + t.Errorf("Got: %v, want: %v", result, tt.want) + } + }) + } } func Test_IsPublic(t *testing.T) { diff --git a/pkg/util/mock.go b/pkg/util/mock.go new file mode 100644 index 00000000..f12a5662 --- /dev/null +++ b/pkg/util/mock.go @@ -0,0 +1,42 @@ +package util + +import "net/http" + +var ( + GetDoFunc func(req *http.Request) (*http.Response, error) + GetParseGitUrlFunc func(url string) error + GetGetGitRawFileAPIFunc func() string + GetSetTokenFunc func(token string, httpTimeout *int) error + GetIsPublicFunc func(httpTimeout *int) bool +) + +type MockClient struct { + DoFunc func(req *http.Request) (*http.Response, error) +} + +func (m *MockClient) Do(req *http.Request) (*http.Response, error) { + return GetDoFunc(req) +} + +type MockGitUrl struct { + ParseGitUrlFunc func(fullUrl string) error + GetGitRawFileAPIFunc func(url string) string + SetTokenFunc func(token string, httpTimeout *int) error + IsPublicFunc func(httpTimeout *int) bool +} + +func (m *MockGitUrl) ParseGitUrl(fullUrl string) error { + return GetParseGitUrlFunc(fullUrl) +} + +func (m *MockGitUrl) GetGitRawFileAPI() string { + return GetGetGitRawFileAPIFunc() +} + +func (m *MockGitUrl) SetToken(token string, httpTimeout *int) error { + return GetSetTokenFunc(token, httpTimeout) +} + +func (m *MockGitUrl) IsPublic(httpTimeout *int) bool { + return GetIsPublicFunc(httpTimeout) +} diff --git a/pkg/util/util.go b/pkg/util/util.go index 3e1f9b84..028e1014 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -54,6 +54,10 @@ import ( "k8s.io/klog" ) +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + const ( HTTPRequestResponseTimeout = 30 * time.Second // HTTPRequestTimeout configures timeout of all HTTP requests ModeReadWriteFile = 0600 // default Permission for a file @@ -1080,29 +1084,43 @@ func DownloadFileInMemory(url string) ([]byte, error) { // DownloadInMemory uses HTTPRequestParams to download the file and return bytes func DownloadInMemory(params HTTPRequestParams) ([]byte, error) { - var httpClient = &http.Client{Transport: &http.Transport{ ResponseHeaderTimeout: HTTPRequestResponseTimeout, }, Timeout: HTTPRequestResponseTimeout} - url := params.URL + var err error + var gitUrl = &GitUrl{} + + if IsGitProviderRepo(params.URL) { + gitUrl, err = NewGitUrl(params.URL) + if err != nil { + return nil, errors.Errorf("failed to parse git repo. error: %v", err) + } + } + + return downloadInMemoryWithClient(params, httpClient, gitUrl) +} + +func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, g IGitUrl) ([]byte, error) { + var url string + url = params.URL req, err := http.NewRequest("GET", url, nil) if err != nil { return nil, err } if IsGitProviderRepo(url) { - g, err := ParseGitUrl(url) + url = g.GetGitRawFileAPI() + req, err = http.NewRequest("GET", url, nil) if err != nil { - return nil, errors.Errorf("failed to parse git repo. error: %v", err) + return nil, err } if !g.IsPublic(params.Timeout) { err = g.SetToken(params.Token, params.Timeout) if err != nil { return nil, err } - bearer := "Bearer " + params.Token - req.Header.Add("Authorization", bearer) + req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", params.Token)) } } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 1de8bb1d..e763489f 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -16,8 +16,11 @@ package util import ( + "bytes" "fmt" "github.com/devfile/library/v2/pkg/testingutil/filesystem" + "github.com/kylelemons/godebug/pretty" + "github.com/stretchr/testify/assert" "io/ioutil" corev1 "k8s.io/api/core/v1" "net" @@ -936,9 +939,132 @@ func TestDownloadFile(t *testing.T) { } } -//todo: +func TestDownloadInMemory_GitRepo(t *testing.T) { + respBody := []byte("test response body") + resp := &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewReader(respBody)), + } + + var Client = &MockClient{} + GetDoFunc = func(req *http.Request) (*http.Response, error) { + if req.Header.Get("Authorization") == "" { + return nil, fmt.Errorf("missing authorization header") + } + return resp, nil + } + + var GitUrlMock = &MockGitUrl{} + GetGetGitRawFileAPIFunc = func() string { + return "" + } + GetSetTokenFunc = func(token string, httpTimeout *int) error { + return nil + } + + tests := []struct { + name string + params HTTPRequestParams + GetIsPublicFunc func(httpTimeout *int) bool + want []byte + wantErr string + }{ + { + name: "Case 1: Private Github repo with token", + params: HTTPRequestParams{ + URL: "https://github.com/myorg/myrepo/file.txt", + Token: "fake-token", + }, + GetIsPublicFunc: func(httpTimeout *int) bool { return false }, + want: []byte("test response body"), + wantErr: "", + }, + { + name: "Case 2: Public Github repo without token", + params: HTTPRequestParams{ + URL: "https://github.com/myorg/myrepo/file.txt", + }, + GetIsPublicFunc: func(httpTimeout *int) bool { return true }, + want: []byte("test response body"), + wantErr: "missing authorization header", + }, + { + name: "Case 3: Non git provider repo", + params: HTTPRequestParams{ + URL: "https://repo.com/myorg/myrepo/file.txt", + Token: "", + }, + want: []byte("test response body"), + wantErr: "missing authorization header", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + GetIsPublicFunc = tt.GetIsPublicFunc + result, err := downloadInMemoryWithClient(tt.params, Client, GitUrlMock) + if (err != nil) != (tt.wantErr != "") { + t.Errorf("Unxpected error: %t, want: %v", err, tt.want) + } else if err == nil && !reflect.DeepEqual(result, tt.want) { + t.Errorf("Expected: %v, received: %v, difference at %v", tt.want, result, pretty.Compare(tt.want, result)) + } else if err != nil { + assert.Regexp(t, tt.wantErr, err.Error(), "Error message should match") + } + }) + } +} + func TestDownloadInMemory(t *testing.T) { - + // Start a local HTTP server + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + // Send response to be tested + _, err := rw.Write([]byte("OK")) + if err != nil { + t.Error(err) + } + })) + + // Close the server when test finishes + defer server.Close() + + tests := []struct { + name string + url string + token string + want []byte + wantErr string + }{ + { + name: "Case 1: Input url is valid", + url: server.URL, + want: []byte{79, 75}, + }, + { + name: "Case 2: Input url is invalid", + url: "invalid", + wantErr: "unsupported protocol scheme", + }, + { + name: "Case 3: Git provider with invalid url", + url: "github.com/mike-hoang/invalid-repo", + token: "", + want: []byte(nil), + wantErr: "failed to parse git repo. error:*", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := DownloadInMemory(HTTPRequestParams{URL: tt.url, Token: tt.token}) + if (err != nil) != (tt.wantErr != "") { + t.Errorf("Failed to download file with error: %s", err) + } else if err == nil && !reflect.DeepEqual(data, tt.want) { + t.Errorf("Expected: %v, received: %v, difference at %v", tt.want, string(data[:]), pretty.Compare(tt.want, data)) + } else if err != nil { + assert.Regexp(t, tt.wantErr, err.Error(), "Error message should match") + } + }) + } } func TestValidateK8sResourceName(t *testing.T) {