diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 6f25a3ab..0d1810c1 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -50,12 +50,12 @@ import ( 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 util.IsGitProviderRepo(url) { + gitUrl, err := git.NewGitUrlWithURL(url) + if err != nil { + return err + } - 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) } diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index 4820a648..68b04cd5 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -19,6 +19,7 @@ import ( "bytes" "context" "fmt" + "github.com/devfile/library/v2/pkg/util" "io/ioutil" "net" "net/http" @@ -2862,7 +2863,7 @@ func Test_parseParentAndPluginFromURI(t *testing.T) { tt.args.devFileObj.Data.AddComponents(plugincomp) } - downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") + downloadGitRepoResources = mockDownloadGitRepoResources("", &git.GitUrl{}, "") err := parseParentAndPlugin(tt.args.devFileObj, &resolutionContextTree{}, resolverTools{}) // Unexpected error @@ -3082,7 +3083,7 @@ func Test_parseParentAndPlugin_RecursivelyReference(t *testing.T) { httpTimeout: &httpTimeout, } - downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") + 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, @@ -4174,7 +4175,7 @@ func Test_parseFromURI(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - downloadGitRepoResources = mockDownloadGitRepoResources(&git.GitUrl{}, "") + 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) @@ -4227,12 +4228,15 @@ func Test_parseFromURI_GitProviders(t *testing.T) { IsFile: true, } + validUrl := "https://raw.githubusercontent.com/devfile/library/main/devfile.yaml" + 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 + url string gitUrl *git.GitUrl token string destDir string @@ -4244,6 +4248,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }{ { name: "private parent devfile", + url: validUrl, gitUrl: validGitUrl, token: validToken, importReference: v1.ImportReference{ @@ -4257,6 +4262,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }, { name: "public parent devfile", + url: validUrl, gitUrl: validGitUrl, token: "", importReference: v1.ImportReference{ @@ -4271,6 +4277,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { { // a valid parent url must contain a revision name: "private parent devfile without a revision", + url: "https://raw.githubusercontent.com/devfile/library/devfile.yaml", gitUrl: &git.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", @@ -4289,8 +4296,22 @@ func Test_parseFromURI_GitProviders(t *testing.T) { wantError: &invalidDevfilePathError, wantResources: []string{}, }, + { + name: "public parent devfile that is not from a git provider", + url: "http://localhost:5000/devfile.yaml", + gitUrl: &git.GitUrl{}, + token: "", + importReference: v1.ImportReference{ + ImportReferenceUnion: v1.ImportReferenceUnion{ + Uri: server.URL, + }, + }, + wantDevFile: minimalDevfile, + wantResources: []string{}, + }, { name: "public parent devfile with no devfile path", + url: "https://github.com/devfile/library", gitUrl: &git.GitUrl{ Protocol: "https", Host: "github.com", @@ -4309,6 +4330,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }, { name: "public parent devfile with invalid devfile path", + url: "https://raw.githubusercontent.com/devfile/library/main/text.txt", gitUrl: &git.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", @@ -4329,6 +4351,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }, { name: "private parent devfile with invalid token", + url: validUrl, gitUrl: validGitUrl, token: invalidToken, importReference: v1.ImportReference{ @@ -4341,6 +4364,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }, { name: "private parent devfile with invalid revision", + url: "https://raw.githubusercontent.com/devfile/library/invalid-revision/devfile.yaml", gitUrl: &git.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", @@ -4371,7 +4395,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { } // tt.gitUrl is the parent devfile URL - downloadGitRepoResources = mockDownloadGitRepoResources(tt.gitUrl, tt.token) + downloadGitRepoResources = mockDownloadGitRepoResources(tt.url, 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 @@ -4422,20 +4446,21 @@ func validateGitResourceFunctions(t *testing.T, wantFiles []string, wantResource } } -func mockDownloadGitRepoResources(gURL *git.GitUrl, mockToken string) func(url string, destDir string, httpTimeout *int, token string) error { +func mockDownloadGitRepoResources(uri string, 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{ - Protocol: gURL.Protocol, - Host: gURL.Host, - Owner: gURL.Owner, - Repo: gURL.Repo, - Revision: gURL.Revision, - Path: gURL.Path, - IsFile: gURL.IsFile, - } - if mockGitUrl.IsGitProviderRepo() { + if util.IsGitProviderRepo(uri) { + // this converts the real git URL to a mock URL + mockGitUrl := git.MockGitUrl{ + Protocol: gURL.Protocol, + Host: gURL.Host, + Owner: gURL.Owner, + Repo: gURL.Repo, + Revision: gURL.Revision, + Path: gURL.Path, + IsFile: gURL.IsFile, + } + 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) } @@ -4466,6 +4491,7 @@ func mockDownloadGitRepoResources(gURL *git.GitUrl, mockToken string) func(url s if err != nil { return err } + } return nil @@ -4862,7 +4888,7 @@ func Test_DownloadGitRepoResources(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { destDir := t.TempDir() - downloadGitRepoResources = mockDownloadGitRepoResources(&tt.gitUrl, tt.token) + downloadGitRepoResources = mockDownloadGitRepoResources(tt.url, &tt.gitUrl, tt.token) err := downloadGitRepoResources(tt.url, destDir, &httpTimeout, tt.token) if (err != nil) && (tt.wantErr != true) { t.Errorf("Unexpected error = %v", err) diff --git a/pkg/git/git.go b/pkg/git/git.go index 94c7680e..bbd69547 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -139,13 +139,13 @@ func (g *GitUrl) CloneGitRepo(destDir string) error { } if g.Revision != "" { - _, err := execute(destDir, "git", "switch", "--detach", "origin/"+g.Revision) + _, err := execute(destDir, "git", "checkout", 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) + return fmt.Errorf("failed to switch repo to revision. repo dir: %v, revision: %v, error: %v", destDir, g.Revision, err) } } diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index 09116fef..83dc3902 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -400,8 +400,6 @@ func Test_IsPublic(t *testing.T) { } func Test_CloneGitRepo(t *testing.T) { - tempInvalidDir := t.TempDir() - invalidGitUrl := GitUrl{ Protocol: "", Host: "", @@ -419,9 +417,42 @@ func Test_CloneGitRepo(t *testing.T) { token: "fake-github-token", } + validGitHubRepoBranch := GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "devfile-resources", + Repo: "python-src-docker", + Revision: "testbranch", + } + + validGitHubRepoCommit := GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "devfile-resources", + Repo: "python-src-docker", + Revision: "bb00eeffc638f2657a0c752ef934a9b6dc87e2c1", + } + + validGitHubRepoInvalidCommit := GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "devfile-resources", + Repo: "python-src-docker", + Revision: "lkjatbasdlkfja0c752ef93faskj4bowdf1", + } + + validGitHubRepoTag := GitUrl{ + Protocol: "https", + Host: "github.com", + Owner: "OpenLiberty", + Repo: "devfile-stack", + Revision: "maven-0.7.0", + } + privateRepoBadTokenErr := "failed to clone repo with token*" publicRepoInvalidUrlErr := "failed to clone repo without a token" missingDestDirErr := "failed to clone repo, destination directory*" + switchRevisionErr := "failed to switch repo to revision*" tests := []struct { name string @@ -438,19 +469,43 @@ func Test_CloneGitRepo(t *testing.T) { { name: "should fail with invalid git url", gitUrl: invalidGitUrl, - destDir: tempInvalidDir, + destDir: "", wantErr: publicRepoInvalidUrlErr, }, { name: "should fail to clone invalid private git url with a bad token", gitUrl: invalidPrivateGitHubRepo, - destDir: tempInvalidDir, + destDir: "", wantErr: privateRepoBadTokenErr, }, + { + name: "should clone valid git url with branch revision", + gitUrl: validGitHubRepoBranch, + destDir: "", + }, + { + name: "should clone valid git url with commit revision", + gitUrl: validGitHubRepoCommit, + destDir: "", + }, + { + name: "should clone valid git url with tag revision", + gitUrl: validGitHubRepoTag, + destDir: "", + }, + { + name: "should fail to clone valid git url with invalid commit", + gitUrl: validGitHubRepoInvalidCommit, + destDir: "", + wantErr: switchRevisionErr, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.destDir == "" { + tt.destDir = t.TempDir() + } err := tt.gitUrl.CloneGitRepo(tt.destDir) if (err != nil) != (tt.wantErr != "") { t.Errorf("Unxpected error: %t, want: %v", err, tt.wantErr) diff --git a/pkg/git/mock.go b/pkg/git/mock.go index 458ab369..b75cbbcf 100644 --- a/pkg/git/mock.go +++ b/pkg/git/mock.go @@ -20,7 +20,6 @@ import ( "net/url" "os" "path/filepath" - "strings" ) type MockGitUrl struct { @@ -70,8 +69,8 @@ var mockExecute = func(baseDir string, cmd CommandType, args ...string) ([]byte, return []byte(""), nil } - if len(args) > 0 && args[0] == "switch" { - revision := strings.TrimPrefix(args[2], "origin/") + if len(args) > 0 && args[0] == "checkout" { + revision := args[1] if revision != "invalid-revision" { resourceFile, err := os.OpenFile(filepath.Clean(baseDir)+"/resource.file", os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) if err != nil { @@ -122,7 +121,7 @@ func (m *MockGitUrl) CloneGitRepo(destDir string) error { } if m.Revision != "" { - _, err := mockExecute(destDir, "git", "switch", "--detach", "origin/"+m.Revision) + _, err := mockExecute(destDir, "git", "checkout", m.Revision) if err != nil { return fmt.Errorf("failed to switch repo to revision. repo dir: %v, revision: %v", destDir, m.Revision) }