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

feat: Add proxy support to CLI API client #12527

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shimako55
Copy link

Fixes #10794

Motivation

Issue:
In the current version of Argo Workflows, the API client does not function properly in environments that use a proxy server.

Purpose:
This PR aims to modify the Argo Workflows API client to ensure it functions correctly when routing through a proxy server.

Modifications

Summary of Changes:
Made modifications in cmd/argo/commands/client/conn.go, pkg/apiclient/apiclient.go, pkg/apiclient/http1-client.go, and pkg/apiclient/http1/facade.go.

Specific Changes:
Added proxy functionality using http.ProxyURL.
Integrated new proxy settings into the API client construction process.
Added proxy settings to the configuration of http.Client and http.Transport.

Reason for Changes:
To support the operation of the API client in proxy environments.

Verification

Testing Method:
Conducted manual tests in an environment with a configured proxy server.

Test Results:
Confirmed that the API client operates correctly when routing through a proxy.

Reproduction Steps:
Replace http://example.com:8080 with your actual proxy server URL in the following commands:

$ ./argo template list --proxy-url=http://example.com:8080

NAME
awesome-tiger

or

$ export HTTPS_PROXY=http://example.com:8080
$ ./argo template list

NAME
awesome-tiger

Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

Can you add some tests in apiclient_test.go and facade_test.go? Thanks

@shimako55
Copy link
Author

Okay, thanks. I'll give it a try. However, it might take me some time.

@shimako55 shimako55 force-pushed the feat-proxy-support-cli-tool branch 3 times, most recently from 42130cf to 5103e5a Compare February 20, 2024 11:32
@shimako55
Copy link
Author

@isubasinghe
I have added facade_test.go.
I didn't know what tests were expected for apiclient_test.go. Could you please tell me specifically what kind of tests are expected?

@Joibel
Copy link
Member

Joibel commented Sep 9, 2024

As suggested by @monaka, closing as implemented by #12867.

@Joibel Joibel closed this Sep 9, 2024
@Joibel
Copy link
Member

Joibel commented Sep 9, 2024

Oops, this isn't implemented there.

@Joibel Joibel reopened this Sep 9, 2024
@agilgur5 agilgur5 changed the title feat: Add proxy support to Argo CLI API client feat: Add proxy support to CLI API client Oct 19, 2024
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.

CLI: HTTP client ignores proxy
3 participants