From 2f735209e68d65bcf9e7c6e4182f8326a0c1bad7 Mon Sep 17 00:00:00 2001 From: Michael Hoang Date: Thu, 27 Apr 2023 16:46:15 -0400 Subject: [PATCH] updating tests Signed-off-by: Michael Hoang --- README.md | 23 +- pkg/devfile/parser/context/context.go | 13 +- pkg/devfile/parser/context/context_test.go | 8 +- pkg/devfile/parser/parse.go | 43 ++- pkg/devfile/parser/parse_test.go | 357 +++++++++++---------- pkg/git/git.go | 37 ++- pkg/git/git_test.go | 92 ++---- pkg/git/mock.go | 27 +- pkg/util/util.go | 5 +- 9 files changed, 313 insertions(+), 292 deletions(-) diff --git a/README.md b/README.md index f5352480..17efc7de 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,9 @@ The Devfile Parser library is a Golang module that: 2. writes to the devfile.yaml with the updated data. 3. generates Kubernetes objects for the various devfile resources. 4. defines util functions for the devfile. +5. downloads resources from a parent devfile if specified in the devfile.yaml -## Private Repository Support +## Private repository support Tokens are required to be set in the following cases: 1. parsing a devfile from a private repository @@ -24,19 +25,28 @@ Set the token for the repository: ```go parser.ParserArgs{ ... - URL: + // URL must point to a devfile.yaml + URL: /devfile.yaml Token: ... } ``` Note: The url must also be set with a supported git provider repo url. +Minimum token scope required: +1. GitHub: Read access to code +2. GitLab: Read repository +3. Bitbucket: Read repository + +Note: To select token scopes for GitHub, a fine-grained token is required. + For more information about personal access tokens: 1. [GitHub docs](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token) 2. [GitLab docs](https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#create-a-personal-access-token) 3. [Bitbucket docs](https://support.atlassian.com/bitbucket-cloud/docs/repository-access-tokens/) [1] Currently, this works under the assumption that the token can authenticate the devfile and the parent devfile; both devfiles are in the same repository. + [2] In this scenario, the token will be used to authenticate the main devfile. ## Usage @@ -199,6 +209,15 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g } ``` +9. When parsing a devfile that contains a parent reference, if the parent uri is a supported git provider repo url with the correct personal access token, all resources from the parent git repo excluding the parent devfile.yaml will be downloaded to the location of the devfile being parsed. **Note: The URL must point to a devfile.yaml** + ```yaml + schemaVersion: 2.2.0 + ... + parent: + uri: /devfile.yaml + ... + ``` + ## Projects using devfile/library The following projects are consuming this library as a Golang dependency diff --git a/pkg/devfile/parser/context/context.go b/pkg/devfile/parser/context/context.go index d0589315..97ab49d3 100644 --- a/pkg/devfile/parser/context/context.go +++ b/pkg/devfile/parser/context/context.go @@ -73,14 +73,6 @@ func NewURLDevfileCtx(url string) DevfileCtx { } } -// NewPrivateURLDevfileCtx returns a new DevfileCtx type object -func NewPrivateURLDevfileCtx(url string, token string) DevfileCtx { - return DevfileCtx{ - url: url, - token: token, - } -} - // NewByteContentDevfileCtx set devfile content from byte data and returns a new DevfileCtx type object and error func NewByteContentDevfileCtx(data []byte) (d DevfileCtx, err error) { err = d.SetDevfileContentFromBytes(data) @@ -166,6 +158,11 @@ func (d *DevfileCtx) GetToken() string { return d.token } +// SetToken sets the token for the devfile +func (d *DevfileCtx) SetToken(token string) { + d.token = token +} + // SetAbsPath sets absolute file path for devfile func (d *DevfileCtx) SetAbsPath() (err error) { // Set devfile absolute path diff --git a/pkg/devfile/parser/context/context_test.go b/pkg/devfile/parser/context/context_test.go index f1dc8292..3f332e88 100644 --- a/pkg/devfile/parser/context/context_test.go +++ b/pkg/devfile/parser/context/context_test.go @@ -88,16 +88,12 @@ func TestNewURLDevfileCtx(t *testing.T) { token = "fake-token" url = "https://github.com/devfile/registry/blob/main/stacks/go/2.0.0/devfile.yaml" ) - - { - d := NewPrivateURLDevfileCtx(url, token) - assert.Equal(t, "https://github.com/devfile/registry/blob/main/stacks/go/2.0.0/devfile.yaml", d.GetURL()) - assert.Equal(t, "fake-token", d.GetToken()) - } { d := NewURLDevfileCtx(url) assert.Equal(t, "https://github.com/devfile/registry/blob/main/stacks/go/2.0.0/devfile.yaml", d.GetURL()) assert.Equal(t, "", d.GetToken()) + d.SetToken(token) + assert.Equal(t, "fake-token", d.GetToken()) } } diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index a88e0e38..e22fa0eb 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "github.com/devfile/library/v2/pkg/git" + "github.com/hashicorp/go-multierror" "io/ioutil" "net/url" "os" @@ -49,34 +50,49 @@ import ( // downloadGitRepoResources is exposed as a global variable for the purpose of running mock tests var downloadGitRepoResources = func(url string, destDir string, httpTimeout *int, token string) error { + var returnedErr error + gitUrl, err := git.NewGitUrlWithURL(url) if err != nil { return err } - if gitUrl.IsGitProviderRepo() && gitUrl.IsFile { - stackDir, err := ioutil.TempDir(os.TempDir(), fmt.Sprintf("git-resources")) + if gitUrl.IsGitProviderRepo() { + if !gitUrl.IsFile || gitUrl.Revision == "" || !strings.Contains(gitUrl.Path, OutputDevfileYamlPath) { + return fmt.Errorf("error getting devfile from url: failed to retrieve %s", url) + } + + stackDir, err := os.MkdirTemp("", fmt.Sprintf("git-resources")) if err != nil { return fmt.Errorf("failed to create dir: %s, error: %v", stackDir, err) } - defer os.RemoveAll(stackDir) + + defer func(path string) { + err := os.RemoveAll(path) + if err != nil { + returnedErr = multierror.Append(returnedErr, err) + } + }(stackDir) if !gitUrl.IsPublic(httpTimeout) { err = gitUrl.SetToken(token, httpTimeout) if err != nil { - return err + returnedErr = multierror.Append(returnedErr, err) + return returnedErr } } err = gitUrl.CloneGitRepo(stackDir) if err != nil { - return err + returnedErr = multierror.Append(returnedErr, err) + return returnedErr } dir := path.Dir(path.Join(stackDir, gitUrl.Path)) err = git.CopyAllDirFiles(dir, destDir) if err != nil { - return err + returnedErr = multierror.Append(returnedErr, err) + return returnedErr } } @@ -163,15 +179,15 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) { } else if args.Path != "" { d.Ctx = devfileCtx.NewDevfileCtx(args.Path) } else if args.URL != "" { - if args.Token != "" { - d.Ctx = devfileCtx.NewPrivateURLDevfileCtx(args.URL, args.Token) - } else { - d.Ctx = devfileCtx.NewURLDevfileCtx(args.URL) - } + d.Ctx = devfileCtx.NewURLDevfileCtx(args.URL) } else { return d, errors.Wrap(err, "the devfile source is not provided") } + if args.Token != "" { + d.Ctx.SetToken(args.Token) + } + tool := resolverTools{ defaultNamespace: args.DefaultNamespace, registryURLs: args.RegistryURLs, @@ -475,10 +491,9 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D } token := curDevfileCtx.GetToken() + d.Ctx = devfileCtx.NewURLDevfileCtx(newUri) if token != "" { - d.Ctx = devfileCtx.NewPrivateURLDevfileCtx(newUri, token) - } else { - d.Ctx = devfileCtx.NewURLDevfileCtx(newUri) + d.Ctx.SetToken(token) } destDir := path.Dir(curDevfileCtx.GetAbsPath()) diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index 6412ea59..0971f8d9 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -16,6 +16,7 @@ package parser import ( + "bytes" "context" "fmt" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" @@ -26,6 +27,7 @@ import ( "net/http/httptest" "os" "path" + "path/filepath" "reflect" "strings" "testing" @@ -2859,7 +2861,7 @@ func Test_parseParentAndPluginFromURI(t *testing.T) { tt.args.devFileObj.Data.AddComponents(plugincomp) } - downloadGitRepoResources = mockDownloadGitRepoResources(&git.Url{}) + downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") err := parseParentAndPlugin(tt.args.devFileObj, &resolutionContextTree{}, resolverTools{}) // Unexpected error @@ -3079,6 +3081,7 @@ func Test_parseParentAndPlugin_RecursivelyReference(t *testing.T) { httpTimeout: &httpTimeout, } + downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") err := parseParentAndPlugin(devFileObj, &resolutionContextTree{}, tool) // devfile has a cycle in references: main devfile -> uri: http://127.0.0.1:8080 -> name: testcrd, namespace: defaultnamespace -> uri: http://127.0.0.1:8090 -> uri: http://127.0.0.1:8080 expectedErr := fmt.Sprintf("devfile has an cycle in references: main devfile -> uri: %s%s -> name: %s, namespace: %s -> uri: %s%s -> uri: %s%s", httpPrefix, uri1, name, namespace, @@ -4149,7 +4152,7 @@ func Test_parseFromURI(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - downloadGitRepoResources = mockDownloadGitRepoResources(&git.Url{}) + downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") got, err := parseFromURI(tt.importReference, tt.curDevfileCtx, &resolutionContextTree{}, resolverTools{}) if (err != nil) != (tt.wantErr != nil) { t.Errorf("Test_parseFromURI() unexpected error: %v, wantErr %v", err, tt.wantErr) @@ -4169,77 +4172,30 @@ func Test_parseFromURI_GitProviders(t *testing.T) { invalidRevision = "invalid-revision" ) - minimalDevfileContent := fmt.Sprintf("schemaVersion: 2.2.0") + minimalDevfileContent := fmt.Sprintf("schemaVersion: 2.2.0\nmetadata:\n name: devfile") server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { _, err := rw.Write([]byte(minimalDevfileContent)) if err != nil { t.Error(err) } })) - - parentDevfileContent := fmt.Sprintf("schemaVersion: 2.2.0\nmetadata:\n name: parent-devfile\nparent:\n uri: \"%s\"", server.URL) - parent := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - _, err := rw.Write([]byte(parentDevfileContent)) - if err != nil { - t.Error(err) - } - })) - - devfileContent := fmt.Sprintf("schemaVersion: 2.2.0\nmetadata:\n name: nested-devfile\nparent:\n uri: \"%s\"", parent.URL) - nestedParent := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - _, err := rw.Write([]byte(devfileContent)) - if err != nil { - t.Error(err) - } - })) - - // Close the server when test finishes defer server.Close() - defer parent.Close() - defer nestedParent.Close() - - httpTimeout := 0 minimalDevfile := DevfileObj{ - Ctx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), - Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevfileHeader: devfilepkg.DevfileHeader{ - SchemaVersion: schemaVersion, - }, - }, - }, - } - - parentDevfile := DevfileObj{ Ctx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), Data: &v2.DevfileV2{ Devfile: v1.Devfile{ DevfileHeader: devfilepkg.DevfileHeader{ SchemaVersion: schemaVersion, Metadata: devfilepkg.DevfileMetadata{ - Name: "parent-devfile", + Name: "devfile", }, }, }, }, } - nestParentDevfile := DevfileObj{ - Ctx: devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath), - Data: &v2.DevfileV2{ - Devfile: v1.Devfile{ - DevfileHeader: devfilepkg.DevfileHeader{ - SchemaVersion: schemaVersion, - Metadata: devfilepkg.DevfileMetadata{ - Name: "nested-devfile", - }, - }, - }, - }, - } - - publicGitUrl := &git.Url{ + validGitUrl := &git.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4249,137 +4205,156 @@ func Test_parseFromURI_GitProviders(t *testing.T) { IsFile: true, } - privateGitUrlInvalidRevision := &git.Url{ - Protocol: "https", - Host: "raw.githubusercontent.com", - Owner: "devfile", - Repo: "library", - Revision: invalidRevision, - Path: "devfile.yaml", - IsFile: true, - } - privateGitUrlInvalidRevision.SetToken(validToken, &httpTimeout) - - privateBitbucketGitUrl := &git.Url{ - Protocol: "https", - Host: "bitbucket.org", - Owner: "devfile", - Repo: "registry", - Revision: "main", - Path: "stacks/go/1.0.2/devfile.yaml", - IsFile: true, - } - privateBitbucketGitUrl.SetToken(validToken, &httpTimeout) - - privateGitUrl := &git.Url{ - Protocol: "https", - Host: "github.com", - Owner: "mock-owner", - Repo: "mock-repo", - Revision: "mock-branch", - Path: "mock/stacks/go/1.0.2/devfile.yaml", - IsFile: true, - } - privateGitUrl.SetToken(validToken, &httpTimeout) - - curDevfileContextWithValidToken := devfileCtx.NewPrivateURLDevfileCtx(OutputDevfileYamlPath, validToken) - curDevfileContextWithInvalidToken := devfileCtx.NewPrivateURLDevfileCtx(OutputDevfileYamlPath, invalidToken) - curDevfileContextWithoutToken := devfileCtx.NewURLDevfileCtx(OutputDevfileYamlPath) - invalidTokenError := "failed to clone repo with token, ensure that the url and token is correct" invalidGitSwitchError := "failed to switch repo to revision*" + invalidDevfilePathError := "error getting devfile from url: failed to retrieve*" tests := []struct { - name string - curDevfileCtx *devfileCtx.DevfileCtx - gitUrl *git.Url - importReference v1.ImportReference - wantDevFile DevfileObj - wantError *string + name string + gitUrl *git.GitUrl + token string + destDir string + importReference v1.ImportReference + wantDevFile DevfileObj + wantError *string + wantResources []string + wantResourceContent []byte }{ { - name: "private main devfile URL", - curDevfileCtx: &curDevfileContextWithValidToken, - gitUrl: privateGitUrl, + name: "private parent devfile", + gitUrl: validGitUrl, + token: validToken, importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ Uri: server.URL, }, }, - wantDevFile: minimalDevfile, + wantDevFile: minimalDevfile, + wantResources: []string{"resource.file"}, + wantResourceContent: []byte("private repo\ngit switched"), }, { - name: "private main devfile Bitbucket URL", - curDevfileCtx: &curDevfileContextWithValidToken, - gitUrl: privateBitbucketGitUrl, + name: "public parent devfile", + gitUrl: validGitUrl, + token: "", importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ Uri: server.URL, }, }, - wantDevFile: minimalDevfile, + wantDevFile: minimalDevfile, + wantResources: []string{"resource.file"}, + wantResourceContent: []byte("public repo\ngit switched"), }, { - name: "private main devfile with a private parent reference", - curDevfileCtx: &curDevfileContextWithValidToken, - gitUrl: privateGitUrl, + // a valid parent url must contain a revision + name: "private parent devfile without a revision", + gitUrl: &git.GitUrl{ + Protocol: "https", + Host: "raw.githubusercontent.com", + Owner: "devfile", + Repo: "library", + Revision: "", + Path: "devfile.yaml", + IsFile: true, + }, + token: validToken, importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ - Uri: parent.URL, + Uri: server.URL, }, }, - wantDevFile: parentDevfile, + wantError: &invalidDevfilePathError, + wantResources: []string{}, }, { - name: "private main devfile with a private parent with a nested private parent reference", - curDevfileCtx: &curDevfileContextWithValidToken, - gitUrl: privateGitUrl, + name: "public parent devfile with no devfile path", + gitUrl: &git.GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "devfile", + Repo: "library", + IsFile: false, + }, + token: "", importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ - Uri: nestedParent.URL, + Uri: server.URL, }, }, - wantDevFile: nestParentDevfile, + wantError: &invalidDevfilePathError, + wantResources: []string{}, }, { - name: "private main devfile without a valid token", - curDevfileCtx: &curDevfileContextWithInvalidToken, - gitUrl: privateGitUrl, + name: "public parent devfile with invalid devfile path", + gitUrl: &git.GitUrl{ + Protocol: "https", + Host: "raw.githubusercontent.com", + Owner: "devfile", + Repo: "library", + Revision: "main", + Path: "text.txt", + IsFile: true, + }, + token: "", importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ Uri: server.URL, }, }, - wantError: &invalidTokenError, + wantError: &invalidDevfilePathError, + wantResources: []string{}, }, { - name: "public main devfile without a token", - curDevfileCtx: &curDevfileContextWithoutToken, - gitUrl: publicGitUrl, + name: "private parent devfile with invalid token", + gitUrl: validGitUrl, + token: invalidToken, importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ Uri: server.URL, }, }, - wantDevFile: minimalDevfile, + wantError: &invalidTokenError, + wantResources: []string{}, }, { - name: "private parent devfile with invalid revision", - curDevfileCtx: &curDevfileContextWithoutToken, - gitUrl: privateGitUrlInvalidRevision, + name: "private parent devfile with invalid revision", + gitUrl: &git.GitUrl{ + Protocol: "https", + Host: "raw.githubusercontent.com", + Owner: "devfile", + Repo: "library", + Revision: invalidRevision, + Path: "devfile.yaml", + IsFile: true, + }, + token: validToken, importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ - Uri: parent.URL, + Uri: server.URL, }, }, - wantError: &invalidGitSwitchError, + wantError: &invalidGitSwitchError, + wantResources: []string{}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - downloadGitRepoResources = mockDownloadGitRepoResources(tt.gitUrl) - got, err := parseFromURI(tt.importReference, *tt.curDevfileCtx, &resolutionContextTree{}, resolverTools{}) + destDir := t.TempDir() + curDevfileContext := devfileCtx.NewDevfileCtx(path.Join(destDir, OutputDevfileYamlPath)) + err := curDevfileContext.SetAbsPath() + if err != nil { + t.Errorf("Unexpected err: %+v", err) + } + + // tt.gitUrl is the parent devfile URL + downloadGitRepoResources = mockDownloadGitRepoResources(tt.gitUrl, tt.token) + got, err := parseFromURI(tt.importReference, curDevfileContext, &resolutionContextTree{}, resolverTools{}) + + // validate even if we want an error; check that no files are copied to destDir + validateGitResourceFunctions(t, tt.wantResources, tt.wantResourceContent, destDir) + if (err != nil) != (tt.wantError != nil) { t.Errorf("Unexpected error: %v, wantErr %v", err, tt.wantError) } else if err == nil && !reflect.DeepEqual(got.Data, tt.wantDevFile.Data) { @@ -4391,7 +4366,41 @@ func Test_parseFromURI_GitProviders(t *testing.T) { } } -func mockDownloadGitRepoResources(gURL *git.Url) func(url string, destDir string, httpTimeout *int, token string) error { +// copied from: https://github.com/devfile/registry-support/blob/main/registry-library/library/library_test.go#L1118 +func validateGitResourceFunctions(t *testing.T, wantFiles []string, wantResourceContent []byte, path string) { + wantNumFiles := len(wantFiles) + files, err := os.ReadDir(path) + if err != nil { + if wantNumFiles != 0 { + t.Errorf("error reading directory %s", path) + } + } else { + // verify only the expected number of files are downloaded + gotNumFiles := len(files) + if gotNumFiles != wantNumFiles { + t.Errorf("The number of downloaded files do not match, want %d got %d", wantNumFiles, gotNumFiles) + } + // verify the expected resources are copied to the dest directory + for _, wantFile := range wantFiles { + if _, err = os.Stat(path + "/" + wantFile); err != nil && os.IsNotExist(err) { + t.Errorf("file %s should exist ", wantFile) + } + } + + // verify contents of resource file; don't need to check if wantResourceContent is nil + if wantResourceContent != nil { + resourceContent, err := os.ReadFile(filepath.Clean(path) + "/resource.file") + if err != nil { + t.Errorf("failed to open test resource: %v", err) + } + if !bytes.Equal(resourceContent, wantResourceContent) { + t.Errorf("Wanted resource content:\n%v\ngot:\n%v\ndifference at\n%v", wantResourceContent, resourceContent, pretty.Compare(string(wantResourceContent), string(resourceContent))) + } + } + } +} + +func mockDownloadGitRepoResources(gURL *git.GitUrl, mockToken string) func(url string, destDir string, httpTimeout *int, token string) error { return func(url string, destDir string, httpTimeout *int, token string) error { // this converts the real git URL to a mock URL mockGitUrl := git.MockGitUrl{ @@ -4404,14 +4413,24 @@ func mockDownloadGitRepoResources(gURL *git.Url) func(url string, destDir string IsFile: gURL.IsFile, } - if mockGitUrl.IsGitProviderRepo() && mockGitUrl.IsFile { - stackDir, err := ioutil.TempDir(os.TempDir(), fmt.Sprintf("git-resources")) + if mockGitUrl.IsGitProviderRepo() { + if !mockGitUrl.IsFile || mockGitUrl.Revision == "" || !strings.Contains(mockGitUrl.Path, OutputDevfileYamlPath) { + return fmt.Errorf("error getting devfile from url: failed to retrieve %s", url+"/"+mockGitUrl.Path) + } + + stackDir, err := os.MkdirTemp("", fmt.Sprintf("git-resources")) if err != nil { return fmt.Errorf("failed to create dir: %s, error: %v", stackDir, err) } - defer os.RemoveAll(stackDir) - err = mockGitUrl.SetToken(token) + defer func(path string) { + err := os.RemoveAll(path) + if err != nil { + err = fmt.Errorf("failed to create dir: %s, error: %v", stackDir, err) + } + }(stackDir) + + err = mockGitUrl.SetToken(mockToken) if err != nil { return err } @@ -4421,12 +4440,7 @@ func mockDownloadGitRepoResources(gURL *git.Url) func(url string, destDir string return err } - if mockGitUrl.GetToken() != "" { - _, err = os.Stat(stackDir + "/private-repo-resource.txt") - } else { - _, err = os.Stat(stackDir + "/public-repo-resource.txt") - } - + err = git.CopyAllDirFiles(stackDir, destDir) if err != nil { return err } @@ -4773,7 +4787,7 @@ func Test_parseFromKubeCRD(t *testing.T) { func Test_DownloadGitRepoResources(t *testing.T) { httpTimeout := 0 - validGitUrl := git.Url{ + validGitUrl := git.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4782,55 +4796,58 @@ func Test_DownloadGitRepoResources(t *testing.T) { Path: "stacks/python/3.0.0/devfile.yaml", IsFile: true, } - validGitUrl.SetToken("valid-token", &httpTimeout) invalidTokenErr := "failed to clone repo with token, ensure that the url and token is correct" tests := []struct { - name string - url string - gitUrl git.Url - destDir string - token string - wantErr bool + name string + url string + gitUrl git.GitUrl + destDir string + token string + wantErr bool + wantResources []string + wantResourceContent []byte }{ { - name: "should be able to get resources with valid token", - url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", - gitUrl: validGitUrl, - token: "valid-token", - wantErr: false, + name: "should be able to get resources with valid token", + url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", + gitUrl: validGitUrl, + token: "valid-token", + wantErr: false, + wantResources: []string{"resource.file"}, + wantResourceContent: []byte("private repo\ngit switched"), }, { - name: "should be able to get resources from public repo (empty token)", - url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", - gitUrl: validGitUrl, - token: "", - wantErr: false, + name: "should be able to get resources from public repo (empty token)", + url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", + gitUrl: validGitUrl, + token: "", + wantErr: false, + wantResources: []string{"resource.file"}, + wantResourceContent: []byte("public repo\ngit switched"), }, { - name: "should fail to get resources with invalid token", - url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", - gitUrl: validGitUrl, - token: "invalid-token", - wantErr: true, + name: "should fail to get resources with invalid token", + url: "https://raw.githubusercontent.com/devfile/registry/main/stacks/python/3.0.0/devfile.yaml", + gitUrl: validGitUrl, + token: "invalid-token", + wantErr: true, + wantResources: []string{}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - destDir, err := ioutil.TempDir("", "") - if err != nil { - t.Errorf("Failed to create dest dir: %s, error: %v", destDir, err) - } - defer os.RemoveAll(destDir) - - downloadGitRepoResources = mockDownloadGitRepoResources(&tt.gitUrl) - err = downloadGitRepoResources(tt.url, destDir, &httpTimeout, tt.token) + destDir := t.TempDir() + downloadGitRepoResources = mockDownloadGitRepoResources(&tt.gitUrl, tt.token) + err := downloadGitRepoResources(tt.url, destDir, &httpTimeout, tt.token) if (err != nil) && (tt.wantErr != true) { t.Errorf("Unexpected error = %v", err) } else if tt.wantErr == true { assert.Containsf(t, err.Error(), invalidTokenErr, "expected error containing %q, got %s", invalidTokenErr, err) + } else { + validateGitResourceFunctions(t, tt.wantResources, tt.wantResourceContent, destDir) } }) } diff --git a/pkg/git/git.go b/pkg/git/git.go index 44a7dc47..94c7680e 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -18,6 +18,7 @@ package git import ( "fmt" "net/url" + "os" "os/exec" "path/filepath" "strings" @@ -30,19 +31,19 @@ const ( BitbucketHost string = "bitbucket.org" ) -type Url struct { +type GitUrl struct { Protocol string // URL scheme Host string // URL domain name Owner string // name of the repo owner Repo string // name of the repo Revision string // branch name, tag name, or commit id Path string // path to a directory or file in the repo - token string // used for authenticating a private repo + token string // authenticates private repo actions for parent devfiles IsFile bool // defines if the URL points to a file in the repo } // NewGitUrlWithURL NewGitUrl creates a GitUrl from a string url -func NewGitUrlWithURL(url string) (Url, error) { +func NewGitUrlWithURL(url string) (GitUrl, error) { gitUrl, err := ParseGitUrl(url) if err != nil { return gitUrl, err @@ -52,8 +53,8 @@ func NewGitUrlWithURL(url string) (Url, error) { // ParseGitUrl extracts information from a support git url // Only supports git repositories hosted on GitHub, GitLab, and Bitbucket -func ParseGitUrl(fullUrl string) (Url, error) { - var g Url +func ParseGitUrl(fullUrl string) (GitUrl, error) { + var g GitUrl err := ValidateURL(fullUrl) if err != nil { return g, err @@ -81,7 +82,7 @@ func ParseGitUrl(fullUrl string) (Url, error) { return g, err } -func (g *Url) GetToken() string { +func (g *GitUrl) GetToken() string { return g.token } @@ -106,7 +107,7 @@ var execute = func(baseDir string, cmd CommandType, args ...string) ([]byte, err return []byte(""), fmt.Errorf(unsupportedCmdMsg, string(cmd)) } -func (g *Url) CloneGitRepo(destDir string) error { +func (g *GitUrl) CloneGitRepo(destDir string) error { exist := CheckPathExists(destDir) if !exist { return fmt.Errorf("failed to clone repo, destination directory: '%s' does not exists", destDir) @@ -140,6 +141,10 @@ func (g *Url) CloneGitRepo(destDir string) error { if g.Revision != "" { _, err := execute(destDir, "git", "switch", "--detach", "origin/"+g.Revision) if err != nil { + err = os.RemoveAll(destDir) + if err != nil { + return err + } return fmt.Errorf("failed to switch repo to revision. repo dir: %v, revision: %v", destDir, g.Revision) } } @@ -147,7 +152,7 @@ func (g *Url) CloneGitRepo(destDir string) error { return nil } -func (g *Url) parseGitHubUrl(url *url.URL) error { +func (g *GitUrl) parseGitHubUrl(url *url.URL) error { var splitUrl []string var err error @@ -209,7 +214,7 @@ func (g *Url) parseGitHubUrl(url *url.URL) error { return err } -func (g *Url) parseGitLabUrl(url *url.URL) error { +func (g *GitUrl) parseGitLabUrl(url *url.URL) error { var splitFile, splitOrg []string var err error @@ -257,7 +262,7 @@ func (g *Url) parseGitLabUrl(url *url.URL) error { return err } -func (g *Url) parseBitbucketUrl(url *url.URL) error { +func (g *GitUrl) parseBitbucketUrl(url *url.URL) error { var splitUrl []string var err error @@ -295,7 +300,7 @@ func (g *Url) parseBitbucketUrl(url *url.URL) error { // SetToken validates the token with a get request to the repo before setting the token // Defaults token to empty on failure. -func (g *Url) SetToken(token string, httpTimeout *int) error { +func (g *GitUrl) SetToken(token string, httpTimeout *int) error { err := g.validateToken(HTTPRequestParams{Token: token, Timeout: httpTimeout}) if err != nil { g.token = "" @@ -307,7 +312,7 @@ func (g *Url) SetToken(token string, httpTimeout *int) error { // IsPublic checks if the GitUrl is public with a get request to the repo using an empty token // Returns true if the request succeeds -func (g *Url) IsPublic(httpTimeout *int) bool { +func (g *GitUrl) IsPublic(httpTimeout *int) bool { err := g.validateToken(HTTPRequestParams{Token: "", Timeout: httpTimeout}) if err != nil { return false @@ -317,7 +322,7 @@ func (g *Url) IsPublic(httpTimeout *int) bool { // validateToken makes a http get request to the repo with the GitUrl token // Returns an error if the get request fails -func (g *Url) validateToken(params HTTPRequestParams) error { +func (g *GitUrl) validateToken(params HTTPRequestParams) error { var apiUrl string switch g.Host { @@ -341,14 +346,14 @@ func (g *Url) validateToken(params HTTPRequestParams) error { } // GitRawFileAPI returns the endpoint for the git providers raw file -func (g *Url) GitRawFileAPI() string { +func (g *GitUrl) GitRawFileAPI() 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.Revision, 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) + apiRawFile = fmt.Sprintf("https://gitlab.com/api/v4/projects/%s%%2F%s/repository/files/%s/raw?ref=%s", g.Owner, g.Repo, g.Path, g.Revision) case BitbucketHost: apiRawFile = fmt.Sprintf("https://api.bitbucket.org/2.0/repositories/%s/%s/src/%s/%s", g.Owner, g.Repo, g.Revision, g.Path) } @@ -357,7 +362,7 @@ func (g *Url) GitRawFileAPI() string { } // IsGitProviderRepo checks if the url matches a repo from a supported git provider -func (g *Url) IsGitProviderRepo() bool { +func (g *GitUrl) IsGitProviderRepo() bool { switch g.Host { case GitHubHost, RawGitHubHost, GitLabHost, BitbucketHost: return true diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index a958185d..be6be7a2 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -28,7 +28,7 @@ func Test_ParseGitUrl(t *testing.T) { tests := []struct { name string url string - wantUrl Url + wantUrl GitUrl wantErr string }{ { @@ -45,7 +45,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitHub repo with root path", url: "https://github.com/devfile/library", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -58,7 +58,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitHub repo with root path and tag", url: "https://github.com/devfile/library/tree/v2.2.0", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -71,7 +71,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitHub repo with root path and revision", url: "https://github.com/devfile/library/tree/0ce592a416fb185564516353891a45016ac7f671", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -89,7 +89,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitHub repo with file path", url: "https://github.com/devfile/library/blob/main/devfile.yaml", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -102,7 +102,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitHub repo with raw file path", url: "https://raw.githubusercontent.com/devfile/library/main/devfile.yaml", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -146,7 +146,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitLab repo with root path", url: "https://gitlab.com/gitlab-org/gitlab-foss", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "gitlab-org", @@ -164,7 +164,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse GitLab repo with file path", url: "https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/README.md", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "gitlab-org", @@ -193,7 +193,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse Bitbucket repo with root path", url: "https://bitbucket.org/fake-owner/fake-public-repo", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -211,7 +211,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse Bitbucket repo with file path", url: "https://bitbucket.org/fake-owner/fake-public-repo/src/main/README.md", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -224,7 +224,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse Bitbucket file path with nested path", url: "https://bitbucket.org/fake-owner/fake-public-repo/src/main/directory/test.txt", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -237,7 +237,7 @@ func Test_ParseGitUrl(t *testing.T) { { name: "should parse Bitbucket repo with raw file path", url: "https://bitbucket.org/fake-owner/fake-public-repo/raw/main/README.md", - wantUrl: Url{ + wantUrl: GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "fake-owner", @@ -281,12 +281,12 @@ func Test_ParseGitUrl(t *testing.T) { func Test_GetGitRawFileAPI(t *testing.T) { tests := []struct { name string - g Url + g GitUrl want string }{ { name: "Github url", - g: Url{ + g: GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -298,19 +298,19 @@ func Test_GetGitRawFileAPI(t *testing.T) { }, { name: "GitLab url", - g: Url{ + g: GitUrl{ Protocol: "https", Host: "gitlab.com", Owner: "gitlab-org", Repo: "gitlab", - Revision: "master", + Revision: "v15.11.0-ee", Path: "README.md", }, - want: "https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab/repository/files/README.md/raw", + want: "https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab/repository/files/README.md/raw?ref=v15.11.0-ee", }, { name: "Bitbucket url", - g: Url{ + g: GitUrl{ Protocol: "https", Host: "bitbucket.org", Owner: "owner", @@ -322,7 +322,7 @@ func Test_GetGitRawFileAPI(t *testing.T) { }, { name: "Empty GitUrl", - g: Url{}, + g: GitUrl{}, want: "", }, } @@ -338,7 +338,7 @@ func Test_GetGitRawFileAPI(t *testing.T) { } func Test_IsPublic(t *testing.T) { - publicGitUrl := Url{ + publicGitUrl := GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -347,7 +347,7 @@ func Test_IsPublic(t *testing.T) { token: "fake-token", } - privateGitUrl := Url{ + privateGitUrl := GitUrl{ Protocol: "https", Host: "github.com", Owner: "not", @@ -360,7 +360,7 @@ func Test_IsPublic(t *testing.T) { tests := []struct { name string - g Url + g GitUrl want bool }{ { @@ -387,11 +387,8 @@ func Test_IsPublic(t *testing.T) { func Test_CloneGitRepo(t *testing.T) { tempInvalidDir := t.TempDir() - tempDirGitHub := t.TempDir() - tempDirGitLab := t.TempDir() - tempDirBitbucket := t.TempDir() - invalidGitUrl := Url{ + invalidGitUrl := GitUrl{ Protocol: "", Host: "", Owner: "nonexistent", @@ -399,31 +396,7 @@ func Test_CloneGitRepo(t *testing.T) { Revision: "nonexistent", } - validPublicGitHubUrl := Url{ - Protocol: "https", - Host: "github.com", - Owner: "devfile", - Repo: "library", - Revision: "main", - } - - validPublicGitLabUrl := Url{ - Protocol: "https", - Host: "gitlab.com", - Owner: "mike-hoang", - Repo: "public-testing-repo", - Revision: "main", - } - - validPublicBitbucketUrl := Url{ - Protocol: "https", - Host: "bitbucket.org", - Owner: "mike-hoang", - Repo: "public-testing-repo", - Revision: "master", - } - - invalidPrivateGitHubRepo := Url{ + invalidPrivateGitHubRepo := GitUrl{ Protocol: "https", Host: "github.com", Owner: "fake-owner", @@ -438,7 +411,7 @@ func Test_CloneGitRepo(t *testing.T) { tests := []struct { name string - gitUrl Url + gitUrl GitUrl destDir string wantErr string }{ @@ -460,21 +433,6 @@ func Test_CloneGitRepo(t *testing.T) { destDir: tempInvalidDir, wantErr: privateRepoBadTokenErr, }, - { - name: "should be able to clone valid public github url", - gitUrl: validPublicGitHubUrl, - destDir: tempDirGitHub, - }, - { - name: "should be able to clone valid public gitlab url", - gitUrl: validPublicGitLabUrl, - destDir: tempDirGitLab, - }, - { - name: "should be able to clone valid public bitbucket url", - gitUrl: validPublicBitbucketUrl, - destDir: tempDirBitbucket, - }, } for _, tt := range tests { diff --git a/pkg/git/mock.go b/pkg/git/mock.go index a6b6c77d..50075762 100644 --- a/pkg/git/mock.go +++ b/pkg/git/mock.go @@ -44,30 +44,43 @@ var mockExecute = func(baseDir string, cmd CommandType, args ...string) ([]byte, u, _ := url.Parse(args[1]) password, hasPassword := u.User.Password() + resourceFile, err := os.Create(filepath.Clean(baseDir) + "/resource.file") + if err != nil { + return nil, fmt.Errorf("failed to create test resource: %v", err) + } + // private repository if hasPassword { switch password { case "valid-token": - _, err := os.Create(filepath.Clean(baseDir) + "/private-repo-resource.txt") + _, err := resourceFile.WriteString("private repo\n") if err != nil { - return nil, fmt.Errorf("failed to create test resource: %v", err) + return nil, fmt.Errorf("failed to write to test resource: %v", err) } - return []byte("test"), nil + return []byte(""), nil default: return []byte(""), fmt.Errorf("not a valid token") } } - // public repository - _, err := os.Create(filepath.Clean(baseDir) + "/public-repo-resource.txt") + + _, err = resourceFile.WriteString("public repo\n") if err != nil { - return nil, fmt.Errorf("failed to create test resource: %v", err) + return nil, fmt.Errorf("failed to write to test resource: %v", err) } - return []byte("test"), nil + return []byte(""), nil } if len(args) > 0 && args[0] == "switch" { revision := strings.TrimPrefix(args[2], "origin/") if revision != "invalid-revision" { + resourceFile, err := os.OpenFile(filepath.Clean(baseDir)+"/resource.file", os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0644) + if err != nil { + return nil, fmt.Errorf("failed to open test resource: %v", err) + } + _, err = resourceFile.WriteString("git switched") + if err != nil { + return nil, fmt.Errorf("failed to write to test resource: %v", err) + } return []byte("git switched to revision"), nil } return []byte(""), fmt.Errorf("failed to switch revision") diff --git a/pkg/util/util.go b/pkg/util/util.go index aa490a30..b4a4fb3c 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1100,7 +1100,7 @@ func DownloadInMemory(params HTTPRequestParams) ([]byte, error) { ResponseHeaderTimeout: HTTPRequestResponseTimeout, }, Timeout: HTTPRequestResponseTimeout} - var g git.Url + var g git.GitUrl var err error if IsGitProviderRepo(params.URL) { @@ -1113,7 +1113,7 @@ func DownloadInMemory(params HTTPRequestParams) ([]byte, error) { return downloadInMemoryWithClient(params, httpClient, g) } -func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, g git.Url) ([]byte, error) { +func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, g git.GitUrl) ([]byte, error) { var url string url = params.URL req, err := http.NewRequest("GET", url, nil) @@ -1128,6 +1128,7 @@ func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, return nil, err } if !g.IsPublic(params.Timeout) { + // check that the token is valid before adding to the header err = g.SetToken(params.Token, params.Timeout) if err != nil { return nil, err