Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxy and custom timeout support #143

Merged
merged 1 commit into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
30 changes: 18 additions & 12 deletions pkg/devfile/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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)
}

Expand Down
35 changes: 25 additions & 10 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down
25 changes: 21 additions & 4 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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)

Expand Down