From 82a6d4aff1bd25d62b2ed3f84872fff2dcfee10a Mon Sep 17 00:00:00 2001 From: Kim Tsao <84398375+kim-tsao@users.noreply.github.com> Date: Tue, 13 Sep 2022 17:41:19 -0400 Subject: [PATCH] proxy and custom timeout support (#143) Signed-off-by: Kim Tsao Signed-off-by: Kim Tsao --- README.md | 22 +++++++++++++++++----- pkg/devfile/parser/parse.go | 30 ++++++++++++++++++------------ pkg/util/util.go | 35 +++++++++++++++++++++++++---------- pkg/util/util_test.go | 25 +++++++++++++++++++++---- 4 files changed, 81 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 4033e16d..184cd0ae 100644 --- a/README.md +++ b/README.md @@ -34,8 +34,19 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g // if top-level variables are not substituted successfully, the warnings can be logged by parsing variableWarning devfile, variableWarning, err := devfilePkg.ParseDevfileAndValidate(parserArgs) ``` + + +2. To override the HTTP request and response timeouts for a devfile with a parent reference from a registry URL, specify the HTTPTimeout value in the parser arguments + ```go + // specify the timeout in seconds + httpTimeout := 20 + parserArgs := parser.ParserArgs{ + HTTPTimeout: &httpTimeout + } + ``` + -2. To get specific content from devfile +3. To get specific content from devfile ```go // To get all the components from the devfile components, err := devfile.Data.GetComponents(DevfileOptions{}) @@ -67,7 +78,7 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g }) ``` -3. To get the Kubernetes objects from the devfile, visit [generators.go source file](pkg/devfile/generator/generators.go) +4. To get the Kubernetes objects from the devfile, visit [generators.go source file](pkg/devfile/generator/generators.go) ```go // To get a slice of Kubernetes containers of type corev1.Container from the devfile component containers containers, err := generator.GetContainers(devfile) @@ -84,7 +95,7 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g deployment := generator.GetDeployment(deployParams) ``` -4. To update devfile content +5. To update devfile content ```go // To update an existing component in devfile object err := devfile.Data.UpdateComponent(v1.Component{ @@ -116,7 +127,7 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g err := devfile.Data.DeleteComponent(componentName) ``` -5. To write to a devfile, visit [writer.go source file](pkg/devfile/parser/writer.go) +6. To write to a devfile, visit [writer.go source file](pkg/devfile/parser/writer.go) ```go // If the devfile object has been created with devfile path already set, can simply call WriteYamlDevfile to writes the devfile err := devfile.WriteYamlDevfile() @@ -147,7 +158,7 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g // write to the devfile on disk err = devfile.WriteYamlDevfile() ``` -6. To parse the outerloop Kubernetes/OpenShift component's uri or inline content, call the read and parse functions +7. To parse the outerloop Kubernetes/OpenShift component's uri or inline content, call the read and parse functions ```go // Read the YAML content values, err := ReadKubernetesYaml(src, fs) @@ -156,6 +167,7 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g resources, err := ParseKubernetesYaml(values) ``` + ## Projects using devfile/library The following projects are consuming this library as a Golang dependency diff --git a/pkg/devfile/parser/parse.go b/pkg/devfile/parser/parse.go index 3e4863f0..08844e2a 100644 --- a/pkg/devfile/parser/parse.go +++ b/pkg/devfile/parser/parse.go @@ -5,19 +5,17 @@ import ( "encoding/json" "fmt" "github.com/devfile/api/v2/pkg/attributes" - "net/url" - "path" - "strings" - - "github.com/devfile/library/pkg/util" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/clientcmd" - "sigs.k8s.io/controller-runtime/pkg/client" - devfileCtx "github.com/devfile/library/pkg/devfile/parser/context" "github.com/devfile/library/pkg/devfile/parser/data" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" + "github.com/devfile/library/pkg/util" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/clientcmd" "k8s.io/klog" + "net/url" + "path" + "sigs.k8s.io/controller-runtime/pkg/client" + "strings" "reflect" @@ -88,6 +86,8 @@ type ParserArgs struct { K8sClient client.Client // ExternalVariables override variables defined in the Devfile ExternalVariables map[string]string + // HTTPTimeout overrides the request and response timeout values for reading a parent devfile reference from the registry. If a negative value is specified, the default timeout will be used. + HTTPTimeout *int } // ParseDevfile func populates the devfile data, parses and validates the devfile integrity. @@ -111,6 +111,7 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) { registryURLs: args.RegistryURLs, context: args.Context, k8sClient: args.K8sClient, + httpTimeout: args.HTTPTimeout, } flattenedDevfile := true @@ -158,6 +159,8 @@ type resolverTools struct { context context.Context // K8sClient is the Kubernetes client instance used for interacting with a cluster k8sClient client.Client + // httpTimeout is the timeout value in seconds passed in from the client. + httpTimeout *int } func populateAndParseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolverTools, flattenedDevfile bool) (DevfileObj, error) { @@ -398,7 +401,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio id := importReference.Id registryURL := importReference.RegistryUrl if registryURL != "" { - devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version) + devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version, tool.httpTimeout) if err != nil { return DevfileObj{}, err } @@ -412,7 +415,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio } else if tool.registryURLs != nil { for _, registryURL := range tool.registryURLs { - devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version) + devfileContent, err := getDevfileFromRegistry(id, registryURL, importReference.Version, tool.httpTimeout) if devfileContent != nil && err == nil { d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent) if err != nil { @@ -431,13 +434,16 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio return DevfileObj{}, fmt.Errorf("failed to get id: %s from registry URLs provided", id) } -func getDevfileFromRegistry(id, registryURL, version string) ([]byte, error) { +func getDevfileFromRegistry(id, registryURL, version string, httpTimeout *int) ([]byte, error) { if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") { return nil, fmt.Errorf("the provided registryURL: %s is not a valid URL", registryURL) } param := util.HTTPRequestParams{ URL: fmt.Sprintf("%s/devfiles/%s/%s", registryURL, id, version), } + + param.Timeout = httpTimeout + return util.HTTPGetRequest(param, 0) } diff --git a/pkg/util/util.go b/pkg/util/util.go index 7d10a9b7..fc03c8f4 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -40,10 +40,9 @@ import ( ) const ( - HTTPRequestTimeout = 30 * time.Second // HTTPRequestTimeout configures timeout of all HTTP requests - ResponseHeaderTimeout = 30 * time.Second // ResponseHeaderTimeout is the timeout to retrieve the server's response headers - ModeReadWriteFile = 0600 // default Permission for a file - CredentialPrefix = "odo-" // CredentialPrefix is the prefix of the credential that uses to access secure registry + HTTPRequestResponseTimeout = 30 * time.Second // HTTPRequestTimeout configures timeout of all HTTP requests + ModeReadWriteFile = 0600 // default Permission for a file + CredentialPrefix = "odo-" // CredentialPrefix is the prefix of the credential that uses to access secure registry ) // httpCacheDir determines directory where odo will cache HTTP respones @@ -70,8 +69,9 @@ type ResourceRequirementInfo struct { // HTTPRequestParams holds parameters of forming http request type HTTPRequestParams struct { - URL string - Token string + URL string + Token string + Timeout *int } // DownloadParams holds parameters of forming file download request @@ -723,11 +723,26 @@ func HTTPGetRequest(request HTTPRequestParams, cacheFor int) ([]byte, error) { req.Header.Add("Authorization", bearer) } + 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{ - ResponseHeaderTimeout: ResponseHeaderTimeout, + Proxy: http.ProxyFromEnvironment, + ResponseHeaderTimeout: overriddenTimeout, }, - Timeout: HTTPRequestTimeout, + Timeout: overriddenTimeout, } klog.V(4).Infof("HTTPGetRequest: %s", req.URL.String()) @@ -1022,8 +1037,8 @@ func DownloadFile(params DownloadParams) error { // DownloadFileInMemory uses the url to download the file and return bytes func DownloadFileInMemory(url string) ([]byte, error) { var httpClient = &http.Client{Transport: &http.Transport{ - ResponseHeaderTimeout: ResponseHeaderTimeout, - }, Timeout: HTTPRequestTimeout} + ResponseHeaderTimeout: HTTPRequestResponseTimeout, + }, Timeout: HTTPRequestResponseTimeout} resp, err := httpClient.Get(url) if err != nil { return nil, err diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index dbe8dcca..12b1304d 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -695,6 +695,9 @@ func TestGetRemoteFilesMarkedForDeletion(t *testing.T) { } 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 @@ -707,9 +710,10 @@ func TestHTTPGetRequest(t *testing.T) { defer server.Close() tests := []struct { - name string - url string - want []byte + name string + url string + want []byte + timeout *int }{ { name: "Case 1: Input url is valid", @@ -723,12 +727,25 @@ func TestHTTPGetRequest(t *testing.T) { 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, + URL: tt.url, + Timeout: tt.timeout, } got, err := HTTPGetRequest(request, 0)