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

Clone default transport to honor http proxy config #5488

Merged
merged 1 commit into from Apr 3, 2023

Conversation

g-gaston
Copy link
Member

Description of changes:
We need to preserve some of the defaults in the default transport. In particular Proxy, which is set to http.ProxyFromEnvironment. This will make the client honor the HTTP_PROXY, HTTPS_PROXY and NO_PROXY env variables.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@eks-distro-bot eks-distro-bot added do-not-merge/work-in-progress size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 31, 2023
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #5488 (b1786c6) into main (242c9e6) will increase coverage by 0.23%.
The diff coverage is 81.81%.

❗ Current head b1786c6 differs from pull request most recent head cd1e184. Consider uploading reports for the commit cd1e184 to get more accurate results

@@            Coverage Diff             @@
##             main    #5488      +/-   ##
==========================================
+ Coverage   72.71%   72.94%   +0.23%     
==========================================
  Files         442      442              
  Lines       36737    36773      +36     
==========================================
+ Hits        26713    26825     +112     
+ Misses       8424     8335      -89     
- Partials     1600     1613      +13     
Impacted Files Coverage Δ
pkg/files/reader.go 84.21% <81.81%> (+11.74%) ⬆️

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@g-gaston g-gaston force-pushed the fix-http-proxy-in-file-reader branch from cf35cc8 to fc299be Compare April 1, 2023 00:29
@eks-distro-bot eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 1, 2023
@g-gaston g-gaston changed the title [WIP] Clone default transport to honor http proxy config Clone default transport to honor http proxy config Apr 1, 2023
@g-gaston g-gaston force-pushed the fix-http-proxy-in-file-reader branch from fc299be to 4d2eb3f Compare April 1, 2023 00:36
Copy link
Member

@abhinavmpandey08 abhinavmpandey08 left a comment

Choose a reason for hiding this comment

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

/lgtm

pkg/files/reader.go Outdated Show resolved Hide resolved
We need to preserve some of the defaults in the default transport. In
particular Proxy, which is set to http.ProxyFromEnvironment. This will
make the client honor the HTTP_PROXY, HTTPS_PROXY and NO_PROXY env
variables.
@g-gaston g-gaston force-pushed the fix-http-proxy-in-file-reader branch from b1786c6 to cd1e184 Compare April 3, 2023 17:19
@g-gaston
Copy link
Member Author

g-gaston commented Apr 3, 2023

/approve

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: g-gaston

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@eks-distro-bot eks-distro-bot merged commit 85d1353 into aws:main Apr 3, 2023
10 checks passed
@g-gaston
Copy link
Member Author

g-gaston commented Apr 3, 2023

/cherry-pick release-0.15

@eks-distro-pr-bot
Copy link
Contributor

@g-gaston: new pull request created: #5510

In response to this:

/cherry-pick release-0.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rahulbabu95 pushed a commit to rahulbabu95/eks-anywhere that referenced this pull request Apr 3, 2023
We need to preserve some of the defaults in the default transport. In
particular Proxy, which is set to http.ProxyFromEnvironment. This will
make the client honor the HTTP_PROXY, HTTPS_PROXY and NO_PROXY env
variables.
ddjjia pushed a commit to ddjjia/eks-anywhere that referenced this pull request Jun 8, 2023
We need to preserve some of the defaults in the default transport. In
particular Proxy, which is set to http.ProxyFromEnvironment. This will
make the client honor the HTTP_PROXY, HTTPS_PROXY and NO_PROXY env
variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants