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

Respect proxy settings and set http timeout in login and ssh commands #406

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

pmatseykanets
Copy link
Contributor

@pmatseykanets pmatseykanets commented Dec 5, 2024

Ref: rancher/rancher#48321

Dedicated http clients in login and ssh commands that downloads certs and an ssh key respectively currently don't respect proxy settings and don't set http timeout.

This PR fixes that and makes use of Norman's API Client Timeout and ProxyURL options and makes it possible to specify those in the server config.

@pmatseykanets pmatseykanets self-assigned this Dec 5, 2024
KubeCredentials map[string]*ExecCredential `json:"kubeCredentials"`
KubeConfigs map[string]*api.Config `json:"kubeConfigs"`
ProxyURL string `json:"proxyUrl"`
HTTPTimeoutSeconds int `json:"httpTimeoutSeconds"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We probably don't need a finer precision for this. Thus *Seconds.

cmd/login.go Outdated Show resolved Hide resolved
cmd/login.go Outdated Show resolved Hide resolved
@alegrey91 alegrey91 self-requested a review December 6, 2024 09:46
alegrey91
alegrey91 previously approved these changes Dec 6, 2024
@pmatseykanets pmatseykanets marked this pull request as ready for review December 6, 2024 16:10
@pmatseykanets pmatseykanets requested a review from a team as a code owner December 6, 2024 16:10
andreas-kupries
andreas-kupries previously approved these changes Dec 6, 2024
alegrey91
alegrey91 previously approved these changes Dec 6, 2024
Copy link

@alegrey91 alegrey91 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -200,3 +204,91 @@ func TestGetMemberNameFromPrincipal(t *testing.T) {
})
}
}

func TestNewHTTPClient(t *testing.T) {
Copy link
Contributor Author

@pmatseykanets pmatseykanets Dec 6, 2024

Choose a reason for hiding this comment

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

FYI Can't use t.Parallel() because of t.Setenv().

Copy link
Contributor

@crobby crobby left a comment

Choose a reason for hiding this comment

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

lgtm. Tests seem reasonable.
Should link the issue

@pmatseykanets
Copy link
Contributor Author

pmatseykanets commented Dec 6, 2024

Should link the issue

Apparently we never had one, so I opened a new one.

@crobby crobby mentioned this pull request Dec 6, 2024
@pmatseykanets
Copy link
Contributor Author

Start local proxy

mitmproxy -p 8088

Set proxy and run rancher cli

export http_proxy='localhost:8088'
export https_proxy='localhost:8088'

rancher cluster ls
CURRENT   ID             STATE     NAME         PROVIDER   NODES     CPU       RAM            PODS
*         c-m-tbgzfbgf   active    downstream   K3S        1         0.55/12   0.32/7.65 GB   12/110
          local          active    local        K3S        1         0.20/12   0.14/7.65 GB   11/110

image

@pmatseykanets pmatseykanets changed the title Make http client proxy and timeout configurable Respect proxy settings and set http timeout in login and ssh commands Dec 9, 2024
@pmatseykanets pmatseykanets merged commit 80099f6 into rancher:main Dec 9, 2024
1 check passed
@pmatseykanets pmatseykanets deleted the http-proxy-timeout branch December 9, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants