diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 23ee8be7..f6b681f9 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -26,9 +26,6 @@ import ( "reflect" "strings" - "github.com/devfile/library/v2/pkg/git" - "github.com/hashicorp/go-multierror" - "github.com/devfile/api/v2/pkg/attributes" devfileCtx "github.com/devfile/library/v2/pkg/devfile/parser/context" "github.com/devfile/library/v2/pkg/devfile/parser/data" @@ -47,57 +44,6 @@ import ( "github.com/pkg/errors" ) -// 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 - - if util.IsGitProviderRepo(url) { - gitUrl, err := git.NewGitUrlWithURL(url) - if err != nil { - return err - } - - 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 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 { - returnedErr = multierror.Append(returnedErr, err) - return returnedErr - } - } - - err = gitUrl.CloneGitRepo(stackDir) - if err != nil { - returnedErr = multierror.Append(returnedErr, err) - return returnedErr - } - - dir := path.Dir(path.Join(stackDir, gitUrl.Path)) - err = git.CopyAllDirFiles(dir, destDir) - if err != nil { - returnedErr = multierror.Append(returnedErr, err) - return returnedErr - } - } - - return nil -} - // ParseDevfile func validates the devfile integrity. // Creates devfile context and runtime objects func parseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolverTools, flattenedDevfile bool) (DevfileObj, error) { @@ -134,7 +80,7 @@ func parseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolver // ParserArgs is the struct to pass into parser functions which contains required info for parsing devfile. // It accepts devfile path, devfile URL or devfile content in []byte format. type ParserArgs struct { - // Path is a relative or absolute devfile path. + // Path is a relative or absolute devfile path on disk Path string // URL is the URL address of the specific devfile. URL string @@ -170,6 +116,8 @@ type ParserArgs struct { ImageNamesAsSelector *ImageSelectorArgs // DownloadGitResources downloads the resources from Git repository if true DownloadGitResources *bool + // DevfileUtilsClient exposes the interface for mock implementation. + DevfileUtilsClient DevfileUtils } // ImageSelectorArgs defines the structure to leverage for using image names as selectors after parsing the Devfile. @@ -217,6 +165,10 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) { d.Ctx.SetToken(args.Token) } + if args.DevfileUtilsClient == nil { + args.DevfileUtilsClient = NewDevfileUtilsClient() + } + downloadGitResources := true if args.DownloadGitResources != nil { downloadGitResources = *args.DownloadGitResources @@ -229,6 +181,7 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) { k8sClient: args.K8sClient, httpTimeout: args.HTTPTimeout, downloadGitResources: downloadGitResources, + devfileUtilsClient: args.DevfileUtilsClient, } flattenedDevfile := true @@ -284,6 +237,8 @@ type resolverTools struct { httpTimeout *int // downloadGitResources downloads the resources from Git repository if true downloadGitResources bool + // devfileUtilsClient exposes the Git Interface to be able to use mock implementation. + devfileUtilsClient DevfileUtils } func populateAndParseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolverTools, flattenedDevfile bool) (DevfileObj, error) { @@ -535,7 +490,7 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D if tool.downloadGitResources { destDir := path.Dir(curDevfileCtx.GetAbsPath()) - err = downloadGitRepoResources(newUri, destDir, tool.httpTimeout, token) + err = tool.devfileUtilsClient.DownloadGitRepoResources(newUri, destDir, token) if err != nil { return DevfileObj{}, err } diff --git a/pkg/devfile/parser/parse_test.go b/pkg/devfile/parser/parse_test.go index 60b8bd95..f4e214af 100644 --- a/pkg/devfile/parser/parse_test.go +++ b/pkg/devfile/parser/parse_test.go @@ -33,8 +33,6 @@ import ( "github.com/devfile/library/v2/pkg/util" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/library/v2/pkg/git" - "github.com/devfile/api/v2/pkg/attributes" devfilepkg "github.com/devfile/api/v2/pkg/devfile" devfileCtx "github.com/devfile/library/v2/pkg/devfile/parser/context" @@ -2864,8 +2862,7 @@ func Test_parseParentAndPluginFromURI(t *testing.T) { tt.args.devFileObj.Data.AddComponents(plugincomp) } - downloadGitRepoResources = mockDownloadGitRepoResources("", &git.GitUrl{}, "") - err := parseParentAndPlugin(tt.args.devFileObj, &resolutionContextTree{}, resolverTools{}) + err := parseParentAndPlugin(tt.args.devFileObj, &resolutionContextTree{}, resolverTools{devfileUtilsClient: NewDevfileUtilsClient()}) // Unexpected error if (err != nil) != (tt.wantErr != nil) { @@ -3079,12 +3076,12 @@ func Test_parseParentAndPlugin_RecursivelyReference(t *testing.T) { httpTimeout := 0 tool := resolverTools{ - k8sClient: testK8sClient, - context: context.Background(), - httpTimeout: &httpTimeout, + k8sClient: testK8sClient, + context: context.Background(), + httpTimeout: &httpTimeout, + devfileUtilsClient: NewMockDevfileUtilsClient(), } - 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, @@ -4176,8 +4173,7 @@ func Test_parseFromURI(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - downloadGitRepoResources = mockDownloadGitRepoResources("", &git.GitUrl{}, "") - got, err := parseFromURI(tt.importReference, tt.curDevfileCtx, &resolutionContextTree{}, resolverTools{}) + got, err := parseFromURI(tt.importReference, tt.curDevfileCtx, &resolutionContextTree{}, resolverTools{devfileUtilsClient: NewDevfileUtilsClient()}) if (err != nil) != (tt.wantErr != nil) { t.Errorf("Test_parseFromURI() unexpected error: %v, wantErr %v", err, tt.wantErr) } else if err == nil && !reflect.DeepEqual(got.Data, tt.wantDevFile.Data) { @@ -4219,7 +4215,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { }, } - validGitUrl := &git.GitUrl{ + validGitUrl := &util.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4234,16 +4230,17 @@ func Test_parseFromURI_GitProviders(t *testing.T) { 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*" + invalidGitProviderError := "Failed to download resources from parent devfile. Unsupported Git Provider for %s " tests := []struct { name string - url string - gitUrl *git.GitUrl + url string // alias for parent devfile URL + gitUrl *util.GitUrl token string destDir string importReference v1.ImportReference wantDevFile DevfileObj - wantError *string + wantError string wantResources []string wantResourceContent []byte downloadGitResources bool @@ -4293,8 +4290,8 @@ 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{ + url: validUrl, + gitUrl: &util.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4309,28 +4306,28 @@ func Test_parseFromURI_GitProviders(t *testing.T) { Uri: server.URL, }, }, - wantError: &invalidDevfilePathError, + wantError: invalidDevfilePathError, wantResources: []string{}, downloadGitResources: true, }, { name: "public parent devfile that is not from a git provider", url: "http://localhost:5000/devfile.yaml", - gitUrl: &git.GitUrl{}, + gitUrl: &util.GitUrl{}, token: "", importReference: v1.ImportReference{ ImportReferenceUnion: v1.ImportReferenceUnion{ Uri: server.URL, }, }, - wantDevFile: minimalDevfile, + wantError: fmt.Sprintf(invalidGitProviderError, "http://localhost:5000/devfile.yaml"), wantResources: []string{}, downloadGitResources: true, }, { name: "public parent devfile with no devfile path", url: "https://github.com/devfile/library", - gitUrl: &git.GitUrl{ + gitUrl: &util.GitUrl{ Protocol: "https", Host: "github.com", Owner: "devfile", @@ -4343,14 +4340,14 @@ func Test_parseFromURI_GitProviders(t *testing.T) { Uri: server.URL, }, }, - wantError: &invalidDevfilePathError, + wantError: invalidDevfilePathError, wantResources: []string{}, downloadGitResources: true, }, { name: "public parent devfile with invalid devfile path", url: "https://raw.githubusercontent.com/devfile/library/main/text.txt", - gitUrl: &git.GitUrl{ + gitUrl: &util.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4365,7 +4362,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { Uri: server.URL, }, }, - wantError: &invalidDevfilePathError, + wantError: invalidDevfilePathError, wantResources: []string{}, downloadGitResources: true, }, @@ -4379,14 +4376,14 @@ func Test_parseFromURI_GitProviders(t *testing.T) { Uri: server.URL, }, }, - wantError: &invalidTokenError, + wantError: invalidTokenError, wantResources: []string{}, downloadGitResources: true, }, { name: "private parent devfile with invalid revision", url: "https://raw.githubusercontent.com/devfile/library/invalid-revision/devfile.yaml", - gitUrl: &git.GitUrl{ + gitUrl: &util.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4401,7 +4398,7 @@ func Test_parseFromURI_GitProviders(t *testing.T) { Uri: server.URL, }, }, - wantError: &invalidGitSwitchError, + wantError: invalidGitSwitchError, wantResources: []string{}, downloadGitResources: true, }, @@ -4416,19 +4413,22 @@ func Test_parseFromURI_GitProviders(t *testing.T) { t.Errorf("Unexpected err: %+v", err) } - // tt.gitUrl is the parent devfile URL - downloadGitRepoResources = mockDownloadGitRepoResources(tt.url, tt.gitUrl, tt.token) - got, err := parseFromURI(tt.importReference, curDevfileContext, &resolutionContextTree{}, resolverTools{downloadGitResources: tt.downloadGitResources}) + mockDC := NewMockDevfileUtilsClient() + mockDC.ParentURLAlias = tt.url + mockDC.GitTestToken = tt.token + mockDC.MockGitURL = util.MockGitUrl(*tt.gitUrl) + + got, err := parseFromURI(tt.importReference, curDevfileContext, &resolutionContextTree{}, resolverTools{downloadGitResources: tt.downloadGitResources, devfileUtilsClient: mockDC}) // 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) + if (err != nil) != (tt.wantError != "") { + t.Errorf("Unexpected error: %v, wantErr %v", err, tt.wantError) } else if err == nil && !reflect.DeepEqual(got.Data, tt.wantDevFile.Data) { t.Errorf("Wanted: %v, got: %v, difference at %v", tt.wantDevFile, got, pretty.Compare(tt.wantDevFile, got)) } else if err != nil { - assert.Regexp(t, *tt.wantError, err.Error(), "Error message should match") + assert.Regexp(t, tt.wantError, err.Error(), "Error message should match") } }) } @@ -4468,58 +4468,6 @@ func validateGitResourceFunctions(t *testing.T, wantFiles []string, wantResource } } -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 { - - 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) - } - - stackDir, err := os.MkdirTemp("", fmt.Sprintf("git-resources")) - if err != nil { - return fmt.Errorf("failed to create dir: %s, error: %v", stackDir, err) - } - - 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 - } - - err = mockGitUrl.CloneGitRepo(stackDir) - if err != nil { - return err - } - - err = git.CopyAllDirFiles(stackDir, destDir) - if err != nil { - return err - } - - } - - return nil - } -} - func Test_parseFromRegistry(t *testing.T) { const ( registry = "127.0.0.1:8080" @@ -4855,9 +4803,8 @@ func Test_parseFromKubeCRD(t *testing.T) { } func Test_DownloadGitRepoResources(t *testing.T) { - httpTimeout := 0 - validGitUrl := git.GitUrl{ + validGitUrl := util.GitUrl{ Protocol: "https", Host: "raw.githubusercontent.com", Owner: "devfile", @@ -4872,7 +4819,7 @@ func Test_DownloadGitRepoResources(t *testing.T) { tests := []struct { name string url string - gitUrl git.GitUrl + gitUrl util.GitUrl destDir string token string wantErr bool @@ -4907,11 +4854,14 @@ func Test_DownloadGitRepoResources(t *testing.T) { }, } + mockDC := NewMockDevfileUtilsClient() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { destDir := t.TempDir() - downloadGitRepoResources = mockDownloadGitRepoResources(tt.url, &tt.gitUrl, tt.token) - err := downloadGitRepoResources(tt.url, destDir, &httpTimeout, tt.token) + mockDC.MockGitURL = util.MockGitUrl(tt.gitUrl) + mockDC.GitTestToken = tt.token + mockDC.ParentURLAlias = tt.url + err := mockDC.DownloadGitRepoResources(tt.url, destDir, tt.token) if (err != nil) && (tt.wantErr != true) { t.Errorf("Unexpected error = %v", err) } else if tt.wantErr == true { diff --git a/pkg/devfile/parser/parser_mock.go b/pkg/devfile/parser/parser_mock.go new file mode 100644 index 00000000..aa67db92 --- /dev/null +++ b/pkg/devfile/parser/parser_mock.go @@ -0,0 +1,75 @@ +// +// Copyright 2023 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package parser + +import ( + "fmt" + "github.com/devfile/library/v2/pkg/util" + "os" + "strings" +) + +type MockDevfileUtilsClient struct { + ParentURLAlias string // Specify a valid git URL as an alias if using a localhost HTTP server in order to pass validation. + MockGitURL util.MockGitUrl + GitTestToken string // Mock Git token. Specify the string "valid-token" for the mock CloneGitRepo to pass +} + +func NewMockDevfileUtilsClient() MockDevfileUtilsClient { + return MockDevfileUtilsClient{} +} + +func (gc MockDevfileUtilsClient) DownloadGitRepoResources(url string, destDir string, token string) error { + + //the url parameter that gets passed in will be the localhost IP of the test server, so it will fail all the validation checks. We will use the global testURL variable instead + //skip the Git Provider check since it'll fail + if util.IsGitProviderRepo(gc.ParentURLAlias) { + // this converts the test git URL to a mock URL + mockGitUrl := gc.MockGitURL + mockGitUrl.Token = gc.GitTestToken + + 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 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.CloneGitRepo(stackDir) + if err != nil { + return err + } + + err = util.CopyAllDirFiles(stackDir, destDir) + if err != nil { + return err + } + + } else { + return fmt.Errorf("Failed to download resources from parent devfile. Unsupported Git Provider for %s ", gc.ParentURLAlias) + } + + return nil +} diff --git a/pkg/devfile/parser/utils.go b/pkg/devfile/parser/utils.go index d65ce6ff..477e3903 100644 --- a/pkg/devfile/parser/utils.go +++ b/pkg/devfile/parser/utils.go @@ -1,5 +1,5 @@ // -// Copyright 2022 Red Hat, Inc. +// Copyright 2022-2023 Red Hat, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,13 +17,75 @@ package parser import ( "fmt" + "github.com/devfile/library/v2/pkg/util" + "github.com/hashicorp/go-multierror" + "os" + "path" "reflect" + "strings" devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/v2/pkg/devfile/parser/data" "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common" ) +type DevfileUtilsClient struct { +} + +func NewDevfileUtilsClient() DevfileUtilsClient { + return DevfileUtilsClient{} +} + +type DevfileUtils interface { + DownloadGitRepoResources(url string, destDir string, token string) error +} + +// DownloadGitRepoResources mock implementation of the real method. +func (gc DevfileUtilsClient) DownloadGitRepoResources(url string, destDir string, token string) error { + var returnedErr error + if util.IsGitProviderRepo(url) { + gitUrl, err := util.NewGitURL(url, token) + if err != nil { + return err + } + + 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 func(path string) { + err := os.RemoveAll(path) + if err != nil { + returnedErr = multierror.Append(returnedErr, err) + } + }(stackDir) + + gitUrl.Token = token + + err = gitUrl.CloneGitRepo(stackDir) + if err != nil { + returnedErr = multierror.Append(returnedErr, err) + return returnedErr + } + + dir := path.Dir(path.Join(stackDir, gitUrl.Path)) + err = util.CopyAllDirFiles(dir, destDir) + if err != nil { + returnedErr = multierror.Append(returnedErr, err) + return returnedErr + } + } else { + return fmt.Errorf("Failed to download resources from parent devfile. Unsupported Git Provider for %s ", url) + } + + return nil +} + // GetDeployComponents gets the default deploy command associated components func GetDeployComponents(devfileData data.DevfileData) (map[string]string, error) { deployCommandFilter := common.DevfileOptions{ diff --git a/pkg/git/util.go b/pkg/git/util.go deleted file mode 100644 index 79ed650a..00000000 --- a/pkg/git/util.go +++ /dev/null @@ -1,260 +0,0 @@ -// -// Copyright 2023 Red Hat, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package git - -import ( - "fmt" - "github.com/devfile/library/v2/pkg/testingutil/filesystem" - "github.com/gregjones/httpcache" - "github.com/gregjones/httpcache/diskcache" - "github.com/pkg/errors" - "io" - "io/ioutil" - "k8s.io/klog" - "net/http" - "net/url" - "os" - "path" - "path/filepath" - "time" -) - -const ( - HTTPRequestResponseTimeout = 30 * time.Second // HTTPRequestTimeout configures timeout of all HTTP requests -) - -// httpCacheDir determines directory where odo will cache HTTP responses -var httpCacheDir = filepath.Join(os.TempDir(), "odohttpcache") - -// HTTPRequestParams holds parameters of forming http request -type HTTPRequestParams struct { - URL string - Token string - Timeout *int - TelemetryClientName string //optional client name for telemetry -} - -// HTTPGetRequest gets resource contents given URL and token (if applicable) -// cacheFor determines how long the response should be cached (in minutes), 0 for no caching -func HTTPGetRequest(request HTTPRequestParams, cacheFor int) ([]byte, error) { - // Build http request - req, err := http.NewRequest("GET", request.URL, nil) - if err != nil { - return nil, err - } - if request.Token != "" { - bearer := "Bearer " + request.Token - req.Header.Add("Authorization", bearer) - } - - //add the telemetry client name - req.Header.Add("Client", request.TelemetryClientName) - - overriddenTimeout := HTTPRequestResponseTimeout - timeout := request.Timeout - if timeout != nil { - //if value is invalid, the default will be used - if *timeout > 0 { - //convert timeout to seconds - overriddenTimeout = time.Duration(*timeout) * time.Second - klog.V(4).Infof("HTTP request and response timeout overridden value is %v ", overriddenTimeout) - } else { - klog.V(4).Infof("Invalid httpTimeout is passed in, using default value") - } - - } - - httpClient := &http.Client{ - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - ResponseHeaderTimeout: overriddenTimeout, - }, - Timeout: overriddenTimeout, - } - - klog.V(4).Infof("HTTPGetRequest: %s", req.URL.String()) - - if cacheFor > 0 { - // if there is an error during cache setup we show warning and continue without using cache - cacheError := false - httpCacheTime := time.Duration(cacheFor) * time.Minute - - // make sure that cache directory exists - err = os.MkdirAll(httpCacheDir, 0750) - if err != nil { - cacheError = true - klog.WarningDepth(4, "Unable to setup cache: ", err) - } - err = cleanHttpCache(httpCacheDir, httpCacheTime) - if err != nil { - cacheError = true - klog.WarningDepth(4, "Unable to clean up cache directory: ", err) - } - - if !cacheError { - httpClient.Transport = httpcache.NewTransport(diskcache.New(httpCacheDir)) - klog.V(4).Infof("Response will be cached in %s for %s", httpCacheDir, httpCacheTime) - } else { - klog.V(4).Info("Response won't be cached.") - } - } - - resp, err := httpClient.Do(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.Header.Get(httpcache.XFromCache) != "" { - klog.V(4).Infof("Cached response used.") - } - - // We have a non 1xx / 2xx status, return an error - if (resp.StatusCode - 300) > 0 { - return nil, errors.Errorf("failed to retrieve %s, %v: %s", request.URL, resp.StatusCode, http.StatusText(resp.StatusCode)) - } - - // Process http response - bytes, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, err - } - - return bytes, err -} - -// ValidateURL validates the URL -func ValidateURL(sourceURL string) error { - u, err := url.Parse(sourceURL) - if err != nil { - return err - } - - if len(u.Host) == 0 || len(u.Scheme) == 0 { - return errors.New("URL is invalid") - } - - return nil -} - -// cleanHttpCache checks cacheDir and deletes all files that were modified more than cacheTime back -func cleanHttpCache(cacheDir string, cacheTime time.Duration) error { - cacheFiles, err := ioutil.ReadDir(cacheDir) - if err != nil { - return err - } - - for _, f := range cacheFiles { - if f.ModTime().Add(cacheTime).Before(time.Now()) { - klog.V(4).Infof("Removing cache file %s, because it is older than %s", f.Name(), cacheTime.String()) - err := os.Remove(filepath.Join(cacheDir, f.Name())) - if err != nil { - return err - } - } - } - return nil -} - -// CheckPathExists checks if a path exists or not -func CheckPathExists(path string) bool { - return checkPathExistsOnFS(path, filesystem.DefaultFs{}) -} - -func checkPathExistsOnFS(path string, fs filesystem.Filesystem) bool { - if _, err := fs.Stat(path); !os.IsNotExist(err) { - // path to file does exist - return true - } - klog.V(4).Infof("path %s doesn't exist, skipping it", path) - return false -} - -// CopyAllDirFiles recursively copies a source directory to a destination directory -func CopyAllDirFiles(srcDir, destDir string) error { - return copyAllDirFilesOnFS(srcDir, destDir, filesystem.DefaultFs{}) -} - -func copyAllDirFilesOnFS(srcDir, destDir string, fs filesystem.Filesystem) error { - var info os.FileInfo - - files, err := fs.ReadDir(srcDir) - if err != nil { - return errors.Wrapf(err, "failed reading dir %v", srcDir) - } - - for _, file := range files { - srcPath := path.Join(srcDir, file.Name()) - destPath := path.Join(destDir, file.Name()) - - if file.IsDir() { - if info, err = fs.Stat(srcPath); err != nil { - return err - } - if err = fs.MkdirAll(destPath, info.Mode()); err != nil { - return err - } - if err = copyAllDirFilesOnFS(srcPath, destPath, fs); err != nil { - return err - } - } else { - if file.Name() == "devfile.yaml" { - continue - } - // Only copy files that do not exist in the destination directory - if !checkPathExistsOnFS(destPath, fs) { - if err := copyFileOnFs(srcPath, destPath, fs); err != nil { - return errors.Wrapf(err, "failed to copy %s to %s", srcPath, destPath) - } - } - } - } - return nil -} - -// copied from: https://github.com/devfile/registry-support/blob/main/index/generator/library/util.go -func copyFileOnFs(src, dst string, fs filesystem.Filesystem) error { - var err error - var srcinfo os.FileInfo - - srcfd, err := fs.Open(src) - if err != nil { - return err - } - defer func() { - if e := srcfd.Close(); e != nil { - fmt.Printf("err occurred while closing file: %v", e) - } - }() - - dstfd, err := fs.Create(dst) - if err != nil { - return err - } - defer func() { - if e := dstfd.Close(); e != nil { - fmt.Printf("err occurred while closing file: %v", e) - } - }() - - if _, err = io.Copy(dstfd, srcfd); err != nil { - return err - } - if srcinfo, err = fs.Stat(src); err != nil { - return err - } - return fs.Chmod(dst, srcinfo.Mode()) -} diff --git a/pkg/git/util_test.go b/pkg/git/util_test.go deleted file mode 100644 index 3c0f54c3..00000000 --- a/pkg/git/util_test.go +++ /dev/null @@ -1,122 +0,0 @@ -// -// Copyright 2023 Red Hat, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package git - -import ( - "github.com/devfile/library/v2/pkg/testingutil/filesystem" - "net/http" - "net/http/httptest" - "reflect" - "testing" -) - -func TestHTTPGetRequest(t *testing.T) { - invalidHTTPTimeout := -1 - validHTTPTimeout := 20 - - // 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 - want []byte - timeout *int - }{ - { - name: "Case 1: Input url is valid", - url: server.URL, - // Want(Expected) result is "OK" - // According to Unicode table: O == 79, K == 75 - want: []byte{79, 75}, - }, - { - name: "Case 2: Input url is invalid", - url: "invalid", - want: nil, - }, - { - name: "Case 3: Test invalid httpTimeout, default timeout will be used", - url: server.URL, - timeout: &invalidHTTPTimeout, - want: []byte{79, 75}, - }, - { - name: "Case 4: Test valid httpTimeout", - url: server.URL, - timeout: &validHTTPTimeout, - want: []byte{79, 75}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - request := HTTPRequestParams{ - URL: tt.url, - Timeout: tt.timeout, - } - got, err := HTTPGetRequest(request, 0) - - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("Got: %v, want: %v", got, tt.want) - t.Logf("Error message is: %v", err) - } - }) - } -} - -func TestCheckPathExists(t *testing.T) { - fs := filesystem.NewFakeFs() - fs.MkdirAll("/path/to/devfile", 0755) - fs.WriteFile("/path/to/devfile/devfile.yaml", []byte(""), 0755) - - file := "/path/to/devfile/devfile.yaml" - missingFile := "/path/to/not/devfile" - - tests := []struct { - name string - filePath string - want bool - }{ - { - name: "should be able to get file that exists", - filePath: file, - want: true, - }, - { - name: "should fail if file does not exist", - filePath: missingFile, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := checkPathExistsOnFS(tt.filePath, fs) - if !reflect.DeepEqual(result, tt.want) { - t.Errorf("Got error: %t, want error: %t", result, tt.want) - } - }) - } -} diff --git a/pkg/git/git.go b/pkg/util/git.go similarity index 95% rename from pkg/git/git.go rename to pkg/util/git.go index bbd69547..f9ca6f1f 100644 --- a/pkg/git/git.go +++ b/pkg/util/git.go @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package git +package util import ( "fmt" @@ -38,10 +38,20 @@ type GitUrl struct { 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 // authenticates private repo actions for parent devfiles + Token string // authenticates private repo actions for parent devfiles IsFile bool // defines if the URL points to a file in the repo } +// NewGitURL NewGitUrl creates a GitUrl from a string url and token. Will eventually replace NewGitUrlWithURL +func NewGitURL(url string, token string) (*GitUrl, error) { + gitUrl, err := ParseGitUrl(url) + if err != nil { + return &gitUrl, err + } + gitUrl.Token = token + return &gitUrl, nil +} + // NewGitUrlWithURL NewGitUrl creates a GitUrl from a string url func NewGitUrlWithURL(url string) (GitUrl, error) { gitUrl, err := ParseGitUrl(url) @@ -83,7 +93,7 @@ func ParseGitUrl(fullUrl string) (GitUrl, error) { } func (g *GitUrl) GetToken() string { - return g.token + return g.Token } type CommandType string @@ -300,18 +310,20 @@ func (g *GitUrl) 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. +// Deprecated. Avoid using since this will cause rate limiting issues func (g *GitUrl) SetToken(token string, httpTimeout *int) error { err := g.validateToken(HTTPRequestParams{Token: token, Timeout: httpTimeout}) if err != nil { - g.token = "" + g.Token = "" return fmt.Errorf("failed to set token. error: %v", err) } - g.token = token + g.Token = token return nil } // IsPublic checks if the GitUrl is public with a get request to the repo using an empty token // Returns true if the request succeeds +// Deprecated. Avoid using since this will cause rate limiting issues func (g *GitUrl) IsPublic(httpTimeout *int) bool { err := g.validateToken(HTTPRequestParams{Token: "", Timeout: httpTimeout}) if err != nil { diff --git a/pkg/git/git_test.go b/pkg/util/git_test.go similarity index 93% rename from pkg/git/git_test.go rename to pkg/util/git_test.go index 83dc3902..634c62aa 100644 --- a/pkg/git/git_test.go +++ b/pkg/util/git_test.go @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package git +package util import ( "github.com/kylelemons/godebug/pretty" @@ -351,54 +351,6 @@ func Test_GetGitRawFileAPI(t *testing.T) { } } -func Test_IsPublic(t *testing.T) { - publicGitUrl := GitUrl{ - Protocol: "https", - Host: "github.com", - Owner: "devfile", - Repo: "library", - Revision: "main", - token: "fake-token", - } - - privateGitUrl := GitUrl{ - Protocol: "https", - Host: "github.com", - Owner: "not", - Repo: "a-valid", - Revision: "none", - token: "fake-token", - } - - httpTimeout := 0 - - tests := []struct { - name string - g GitUrl - want bool - }{ - { - name: "should be public", - g: publicGitUrl, - want: true, - }, - { - name: "should be private", - g: privateGitUrl, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := tt.g.IsPublic(&httpTimeout) - if !reflect.DeepEqual(result, tt.want) { - t.Errorf("Got: %t, want: %t", result, tt.want) - } - }) - } -} - func Test_CloneGitRepo(t *testing.T) { invalidGitUrl := GitUrl{ Protocol: "", @@ -414,7 +366,7 @@ func Test_CloneGitRepo(t *testing.T) { Owner: "fake-owner", Repo: "fake-private-repo", Revision: "master", - token: "fake-github-token", + Token: "fake-github-token", } validGitHubRepoBranch := GitUrl{ diff --git a/pkg/git/mock.go b/pkg/util/mock.go similarity index 97% rename from pkg/git/mock.go rename to pkg/util/mock.go index b75cbbcf..bdfc4110 100644 --- a/pkg/git/mock.go +++ b/pkg/util/mock.go @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package git +package util import ( "fmt" @@ -29,12 +29,12 @@ type MockGitUrl struct { 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 // used for authenticating a private repo IsFile bool // defines if the URL points to a file in the repo } func (m *MockGitUrl) GetToken() string { - return m.token + return m.Token } var mockExecute = func(baseDir string, cmd CommandType, args ...string) ([]byte, error) { @@ -131,7 +131,7 @@ func (m *MockGitUrl) CloneGitRepo(destDir string) error { } func (m *MockGitUrl) SetToken(token string) error { - m.token = token + m.Token = token return nil } diff --git a/pkg/util/util.go b/pkg/util/util.go index b4a4fb3c..6ce8a6d6 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -21,9 +21,10 @@ import ( "bytes" "crypto/rand" "fmt" - "github.com/devfile/library/v2/pkg/git" gitpkg "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/gregjones/httpcache" + "github.com/gregjones/httpcache/diskcache" "io" "io/ioutil" "math/big" @@ -47,8 +48,6 @@ import ( "github.com/devfile/library/v2/pkg/testingutil/filesystem" "github.com/fatih/color" "github.com/gobwas/glob" - "github.com/gregjones/httpcache" - "github.com/gregjones/httpcache/diskcache" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -887,8 +886,8 @@ func ConvertGitSSHRemoteToHTTPS(remote string) string { // IsGitProviderRepo checks if the url matches a repo from a supported git provider func IsGitProviderRepo(url string) bool { - if strings.Contains(url, git.RawGitHubHost) || strings.Contains(url, git.GitHubHost) || - strings.Contains(url, git.GitLabHost) || strings.Contains(url, git.BitbucketHost) { + if strings.Contains(url, RawGitHubHost) || strings.Contains(url, GitHubHost) || + strings.Contains(url, GitLabHost) || strings.Contains(url, BitbucketHost) { return true } return false @@ -1100,11 +1099,11 @@ func DownloadInMemory(params HTTPRequestParams) ([]byte, error) { ResponseHeaderTimeout: HTTPRequestResponseTimeout, }, Timeout: HTTPRequestResponseTimeout} - var g git.GitUrl + var g GitUrl var err error if IsGitProviderRepo(params.URL) { - g, err = git.NewGitUrlWithURL(params.URL) + g, err = NewGitUrlWithURL(params.URL) if err != nil { return nil, errors.Errorf("failed to parse git repo. error: %v", err) } @@ -1113,7 +1112,7 @@ func DownloadInMemory(params HTTPRequestParams) ([]byte, error) { return downloadInMemoryWithClient(params, httpClient, g) } -func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, g git.GitUrl) ([]byte, error) { +func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, g GitUrl) ([]byte, error) { var url string url = params.URL req, err := http.NewRequest("GET", url, nil) @@ -1127,12 +1126,8 @@ func downloadInMemoryWithClient(params HTTPRequestParams, httpClient HTTPClient, if err != nil { 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 - } + + if params.Token != "" { 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 55a8bdae..19961b0c 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -939,6 +939,7 @@ func TestDownloadFile(t *testing.T) { } func TestDownloadInMemory(t *testing.T) { + const downloadErr = "failed to retrieve %s, 404: Not Found" // Start a local HTTP server server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { // Send response to be tested @@ -980,6 +981,12 @@ func TestDownloadInMemory(t *testing.T) { url: "https://github.com/devfile/library/main/README.md", wantErr: "failed to parse git repo. error: url path to directory or file should contain 'tree' or 'blob'*", }, + { + name: "Case 5: Public Github repo, with invalid token ", + url: "https://github.com/devfile/library/blob/main/devfile.yaml", + token: "fake-token", + wantErr: fmt.Sprintf(downloadErr, "https://"+RawGitHubHost+"/devfile/library/main/devfile.yaml"), + }, } for _, tt := range tests {