-
Notifications
You must be signed in to change notification settings - Fork 386
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
Align how http proxies are handled when fetching gpg keys for download verification #1125
Conversation
…d verification * Copy receive_gpg_keys in python/install.sh to other features which use gpg verification * Ensure http proxy url is passed to gpg when receiving keys from servers * Could connect to key servers but not receive keys * Remove httpProxy from features as it would have been broken from the issue above
* Fix proxy arg mismatch between downloading collateral and verifying collateral * Should be both or none * Curl should automatically pick up proxy config
@@ -15,6 +15,7 @@ GIT_LFS_ARCHIVE_GPG_KEY_URI="https://packagecloud.io/github/git-lfs/gpgkey" | |||
GIT_LFS_ARCHIVE_ARCHITECTURES="amd64 arm64" | |||
GIT_LFS_ARCHIVE_VERSION_CODENAMES="stretch buster bullseye bionic focal jammy" | |||
GIT_LFS_CHECKSUM_GPG_KEYS="0x88ace9b29196305ba9947552f1ba225c0223b187 0x86cd3297749375bcf8206715f54fe648088335a9 0xaa3b3450295830d2de6db90caba67be5a5795889" | |||
KEYSERVER_PROXY="${HTTP_PROXY:-""}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rbambrough-intel Thank you so much for taking the time to contribute this PR.
Instead of these changes, how about we making similar changes #1119 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the link is not pointing to the right PR and I don't understand your statement as it seems to be the common way to define a default variable value in this project (e.g.:
Line 18 in 0be7941
ADDITIONAL_VERSIONS="${ADDITIONALVERSIONS:-""}" |
Line 29 in 302feca
ADDITIONAL_VERSIONS="${ADDITIONALVERSIONS:-""}" |
features/src/powershell/install.sh
Line 16 in 302feca
POWERSHELL_MODULES="${MODULES:-""}" |
KEYSERVER_PROXY="${HTTP_PROXY:-""}" | |
KEYSERVER_PROXY="${HTTP_PROXY}" |
But maybe I missed something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I pasted the incorrect link.
I was recommending #1124 which tests the servers before using them to download the keys.
KEYSERVER_PROXY="${HTTP_PROXY:-""}"
This simply assigns the KEYSERVER_PROXY
to "" if HTTP_PROXY
is not defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jarsop If we are fine with removing the httpProxy setting from some of the features then it can be simplified into:
KEYSERVER_PROXY="$HTTP_PROXY"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samruddhikhandale It does do curl checking of the key servers, just not the same as the one you linked (map instead of multiple function calls). I wasn't sure touching the function one so I adjusted all the others that looked closee
Thanks for your PR, I came to the same conclusion and it avoids duplicate work. |
I noticed while trying to run a dev-container setup behind a corporate proxy that git-lfs was getting stuck waiting for gpg keys.
It downloads the collateral file from, mostly, github and then checks to make sure the hashes match up with what is expected. Due to how the proxy was being handled through the install scripts it would either failure to download the collateral (http proxy not correctly setup in container environment) or would fail to verify the collateral (http proxy setup correctly, but gpg not being passed proxy values).
To solve this I went through and aligned all of the features which use gpg checking with how the python feature does it.
I also removed the httpProxy parameter from the features which had it as anyone using it would have already had to have had the proxy correctly setup in order to download the collateral initially. I was one the fence about this change and would remove it if it is deemed no backwards compatible. I didn't see any documentation on how this type of removal should be handled.
Affected features:
git-lfs
git
github-cli
kubectl-helm-minikube
python
ruby
terraform