build,add: add support for corporate proxies #6274
Conversation
userid0x0
force-pushed
the
cmdAdd_proxyFix
branch
from
941f44a to
1d28e5b
Compare
|
Hi, |
|
@userid0x0 thanks for the contribution. It looks like your test passed, but for some reason the overall tests timed out. I'll restart them to see if we have better luck at them. On a quick look, the changes LGTM. |
|
Changes in |
userid0x0
force-pushed
the
cmdAdd_proxyFix
branch
from
1d28e5b to
52e9898
Compare
|
I want to apologize for the change in |
tests/bud.bats
Outdated
| _prefetch alpine | ||
| target=alpine-image | ||
| fake_proxy="FTP_PROXY=http://invalid.localhost:8080 ftp_proxy=http://invalid.localhost:8080 http_proxy=http://invalid.localhost:8080 HTTPS_PROXY=http://invalid.localhost:8080" | ||
| eval $fake_proxy run_buildah 125 build $WITH_POLICY_JSON -t alpine-image -f $BUDFILES/add-checksum/Containerfile $BUDFILES/add-checksum |
There was a problem hiding this comment.
This is not testing much, you just check the exit code which might as well happen if you have a typo in the command or for any other reason.
It must check for the proper error message to check that it fails connecting to the proxy.
assert "$output" =~ "<expected error message here>" "connection to fake proxy must fail"
Also the use of eval feels unnecessary just do
FTP_PROXY=http://invalid.localhost:8080 ftp_proxy=http://invalid.localhost:8080 http_proxy=http://invalid.localhost:8080 HTTPS_PROXY=http://invalid.localhost:8080 \
run_buildah 125 build ...
And I guess drop the ftp proxy ones since they are not in play here in any case.
There was a problem hiding this comment.
Thanks to ipv6 localhost may result in ::1 or 127.0.0.1. I use now a fixed ipv4 ip 127.0.0.1 with a reserved port 47. Let me know if there is anything else I can do.
Use the Proxy Environment http_proxy/HTTPS_PROXY/NO_PROXY to handle the http request. Signed-off-by: Alexander Merkle <[email protected]>
userid0x0
force-pushed
the
cmdAdd_proxyFix
branch
from
52e9898 to
2df30a8
Compare
|
This is a useful patch. I've applied and tested it in my environment and it's helped me in a similar situation. Thanks @userid0x0 for your work on this. |
|
@aeijdenberg no problem. @Luap99 is there anything else todo? |
|
A friendly reminder that this PR had no activity for 30 days. |
|
Hi, |
|
@nalind PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, Luap99, userid0x0 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 |
|
LGTM on my end but I like to see an ack/lgtm from @nalind |
Use the Proxy Environment http_proxy/HTTPS_PROXY/NO_PROXY to handle the http/https request.
What type of PR is this?
What this PR does / why we need it:
How to verify it
See instructions in initial error report. containers/podman#25220
Which issue(s) this PR fixes:
Fixes: https://github.com/containers/podman/issues/25220Special notes for your reviewer:
Does this PR introduce a user-facing change?