Add HTTP_PROXY support for gpg #1276
Conversation
|
|
|
ibcht
force-pushed
the
http-proxy-support
branch
3 times, most recently
from
9fd6b72 to
5dcf2ba
Compare
ibcht
force-pushed
the
http-proxy-support
branch
from
5dcf2ba to
04d471e
Compare
Signed-off-by: ibauchet <[email protected]>
ibcht
force-pushed
the
http-proxy-support
branch
from
04d471e to
610ad2e
Compare
|
Since this PR affects the install scripts it would need to bump the factory version and have a corresponding entry in the change log. cypress-docker-images/factory/.env Lines 19 to 21 in 15838fb https://github.com/cypress-io/cypress-docker-images/blob/master/factory/CHANGELOG.md I don't know if it would be possible to test this addition in CI (apart from testing that it doesn't break anything when no If not, and there has already been some response from the Cypress.io team, perhaps you could show some logs from a working external environment using a proxy? I'm not in an environment where I use a proxy, so I can't review the PR in a detailed way though. |
Hi @MikeMcC399, thanks for your comment. I will update the changelog as recommended. Regarding the testing of this contribution, I can definitely share some logs. Also, maybe the users who also reported the issue (#1003) could validate this PR on their side as well. But as this is an environment related option, adding a custom test this to the CircleCI tests looks difficult. Also, I must admit that since the default.sh file has been updated, this comment is not true anymore as it doesn't reflect the upstream docker-node Dockerfile (https://github.com/nodejs/docker-node/blob/main/20/bookworm-slim/Dockerfile). I don't know if it is acceptable, at least I can add a comment if needed. |
The code was no longer exactly the same even before your PR. I would suggest to add a comment about your change below the other comments. cypress-docker-images/factory/installScripts/node/default.sh Lines 6 to 7 in 15838fb One other small suggestion: In your original posting you write:
If you write instead:
then it will automatically link the PR and the issue, and it will pull in a dynamic link of the issue's description into the text as well, similar to the following:
|
|
Hi @MikeMcC399, Thanks for your feedback and advices. Latest commit :
Please feel free to adapt if necessary. I have proceed to test with and without the feature. Please note that all my containers are launched with HTTP_PROXY set by default, so I didn't have to mention the ARG at build time. Launching this command ... FACTORY_VERSION=5.1.2 BUILDKIT_PROGRESS=plain docker compose build test-factory-all-included | tee build-behind-proxy-without-feature.logI get this output : I killed the process here, as it take some times to timeout each time. In the end, the build fails. And here, with the FACTORY_VERSION=proxy-support BUILDKIT_PROGRESS=plain docker compose build test-factory-all-included | tee build-behind-proxy-with-feature.logI get this output : No more error on the gpg step. Full log is attached for the working scenario : |
|
Thanks for the updates! You mention
Could you explain in the documentation change whether it is always necessary to build a custom image or whether an existing image can be run by setting an environment variable for the proxy?
It's not possible for me to amend your PR as I don't have write privileges to the repo. That is reserved to the Cypress.io team. Also this isn't my area of expertise, so I can only make some general comments or raise some questions. |
|
Hi @MikeMcC399,
This PR doesn't change the current Cypress behavior regarding proxy, which honors the http_proxy variable for the http traffic. The only improvement is on the gpg key download during factory-based custom image build. So, yes, an existing image will run without any problem. I am not sure if I understood well your question, if so, I will update the documentation accordingly. |
|
Sorry that I was a little slow on understanding as I don't have the experience in this area. It sounds like your documentation is fine. Thank you for the additional explanation! |
No problem, I wish I didn't have to struggle with this proxy constraint on a daily basis as well ! Thanks for your kind follow-up. |
There was a problem hiding this comment.
The maintainers would need to allow the GitHub workflows to run in order to check that no errors have been introduced which affect the examples.
At this time I don't have any additional requests, so as far as I can tell this PR looks good. CircleCI workflows are successful.
|
Due the release of Cypress 14 and update of |
Currently, the cypress/factory base image can't be used behind a corporate proxy. The reason is gpg not taking into account HTTP_PROXY environment variable at build time, so hkps protocol is failing.
This PR aims to fix the issue #1003.
To set-up proxy, you now have have to provide HTTP_PROXY build-arg (which is a Dockerfile predefined arg).
NB: I think that HTTPS_PROXY would be more accurate to use for hpks (443), but as gpg only has keyserver option for http_proxy, it might be preferable to stick with standard HTTP_PROXY (in my use case they use the same proxy url).