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

backport proxy and custom timeout support to console v4.11 #148

Merged
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
84 changes: 46 additions & 38 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,28 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g
```go
// ParserArgs is the struct to pass into parser functions which contains required info for parsing devfile.
parserArgs := parser.ParserArgs{
Path: path,
FlattenedDevfile: &flattenedDevfile,
RegistryURLs: registryURLs,
DefaultNamespace: defaultNamespace,
Context: context,
K8sClient: client,
}
Path: path,
FlattenedDevfile: &flattenedDevfile,
RegistryURLs: registryURLs,
DefaultNamespace: defaultNamespace,
Context: context,
K8sClient: client,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should include HTTPTimeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, that's just an example usage HTTPTimeout is optional so we can leave it out

}

// Parses the devfile and validates the devfile data
// 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 All @@ -42,55 +50,55 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g
// & import: {strategy: Dockerfile}
components, err := devfile.Data.GetComponents(DevfileOptions{
Filter: map[string]interface{}{
"tool": "console-import",
"import": map[string]interface{}{
"strategy": "Dockerfile",
},
},
"tool": "console-import",
"import": map[string]interface{}{
"strategy": "Dockerfile",
},
},
})

// To get all the volume components
components, err := devfile.Data.GetComponents(DevfileOptions{
ComponentOptions: ComponentOptions{
ComponentType: v1.VolumeComponentType,
},
ComponentOptions: ComponentOptions{
ComponentType: v1.VolumeComponentType,
},
})

// To get all the exec commands that belong to the build group
commands, err := devfile.Data.GetCommands(DevfileOptions{
CommandOptions: CommandOptions{
CommandType: v1.ExecCommandType,
CommandGroupKind: v1.BuildCommandGroupKind,
},
CommandOptions: CommandOptions{
CommandType: v1.ExecCommandType,
CommandGroupKind: v1.BuildCommandGroupKind,
},
})
```

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)

// To generate a Kubernetes deployment of type v1.Deployment
deployParams := generator.DeploymentParams{
TypeMeta: generator.GetTypeMeta(deploymentKind, deploymentAPIVersion),
ObjectMeta: generator.GetObjectMeta(name, namespace, labels, annotations),
InitContainers: initContainers,
Containers: containers,
Volumes: volumes,
PodSelectorLabels: labels,
}
deployment := generator.GetDeployment(deployParams)
TypeMeta: generator.GetTypeMeta(deploymentKind, deploymentAPIVersion),
ObjectMeta: generator.GetObjectMeta(name, namespace, labels, annotations),
InitContainers: initContainers,
Containers: containers,
Volumes: volumes,
PodSelectorLabels: labels,
}
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{
Name: "component1",
ComponentUnion: v1.ComponentUnion{
Container: &v1.ContainerComponent{
Container: v1.Container{
Image: "image1",
Name: "component1",
ComponentUnion: v1.ComponentUnion{
Container: &v1.ContainerComponent{
Container: v1.Container{
Image: "image1",
},
},
},
Expand All @@ -114,7 +122,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 All @@ -138,8 +146,8 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g

// create devfile object with the new DevfileCtx and DevfileData
devfile := parser.DevfileObj{
Ctx: ctx,
Data: devfileData,
Ctx: ctx,
Data: devfileData,
}

// write to the devfile on disk
Expand Down
14 changes: 11 additions & 3 deletions pkg/devfile/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ type ParserArgs struct {
Context context.Context
// K8sClient is the Kubernetes client instance used for interacting with a cluster
K8sClient client.Client
// 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason we make it a pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's to determine if this value is unset or not. If unset, we will use the default value of 30s

}

// ParseDevfile func populates the devfile data, parses and validates the devfile integrity.
Expand All @@ -104,6 +106,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 @@ -138,6 +141,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 @@ -378,7 +383,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio
id := importReference.Id
registryURL := importReference.RegistryUrl
if registryURL != "" {
devfileContent, err := getDevfileFromRegistry(id, registryURL)
devfileContent, err := getDevfileFromRegistry(id, registryURL, tool.httpTimeout)
if err != nil {
return DevfileObj{}, err
}
Expand All @@ -392,7 +397,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio

} else if tool.registryURLs != nil {
for _, registryURL := range tool.registryURLs {
devfileContent, err := getDevfileFromRegistry(id, registryURL)
devfileContent, err := getDevfileFromRegistry(id, registryURL, tool.httpTimeout)
if devfileContent != nil && err == nil {
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent)
if err != nil {
Expand All @@ -411,13 +416,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 string) ([]byte, error) {
func getDevfileFromRegistry(id, registryURL 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", registryURL, id),
}

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