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

support add repo behind proxy on ubuntu(#42534) #42536

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

Conversation

hangsu-ma
Copy link

@hangsu-ma hangsu-ma commented Jul 9, 2018

SUMMARY

Fixes #42534
Similar reason behind #42443
apt_repository module calls apt-key to add new repo source on ubuntu.
apt-key does not respect http_proxy environment variable.
As a result, one would thought adding http_porxy env var using environment
will be sufficient, which is never the case and apt_repository will always fail
to add new repo behind proxy on ubuntu.

keyserver-options are used to pass in proxy settings for apt-key, example:
sudo apt-key adv --keyserver-options http-proxy=http://username:password@proxy.example.com:8080 --keyserver keyserver.ubuntu.com --recv-keys GPG_KEY

This fix read http_proxy environment variable and pass it on to apt-key using --keyserver-options.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

apt_repository

ANSIBLE VERSION
all
ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

@ansibot ansibot commented Jul 9, 2018

cc @sashka
click here for bot help

@ansibot ansibot added affects_2.7 bug core_review module needs_triage new_contributor support:core labels Jul 9, 2018
@hangsu-ma
Copy link
Author

@hangsu-ma hangsu-ma commented Jul 10, 2018

I actually have both https_proxy and no_proxy in the code, had to took them out pretty quickly, because they are not supported options, see error message below if you pass them in from --keyserver-options:

sudo apt-key adv --keyserver-options "http-proxy=http://username:password@proxy.example.com:8080 https-proxy=http://username:password@proxy.example.com:8080 https_proxy=http://username:password@proxy.example.com:8080 no-proxy=www.google.com no_proxy=www.google.com" --keyserver keyserver.ubuntu.com --recv-keys GPG_KEY
Executing: /tmp/apt-key-gpghome.zJ3uDH1oNy/gpg.1.sh --keyserver-options http-proxy=http://username:password@proxy.example.com:8080 https-proxy=http://username:password@proxy.example.com:8080 https_proxy=http://username:password@proxy.example.com:8080 no-proxy=www.google.com no_proxy=www.google.com --keyserver keyserver.ubuntu.com --recv-keys GPG_KEY
gpg: keyserver option 'https-proxy' is unknown
gpg: keyserver option 'https_proxy' is unknown
gpg: keyserver option 'no-proxy' is unknown
gpg: keyserver option 'no_proxy' is unknown
gpg: "GPG_KEY" not a key ID: skipping

below are my speculation due to the lack of documentation on keyserver-options:

  • no proxy, this apt-key is hitting single URL(the key server) which is either in the proxy or outside the proxy, so no_proxy or such is needed. I guess apt-key will fail when there is a URL forward or redirect on key server that cross proxy (not sure if it's allowed by OpenPGP)
  • https proxy, according to the answers on this: https://security.stackexchange.com/questions/4161/shouldnt-gpg-key-fetching-use-a-secure-connection
    it's always on http protocal so https_proxy is not releveant and not supported.

@sashka
Copy link
Contributor

@sashka sashka commented Jul 10, 2018

@ansibot ansibot removed the needs_triage label Jul 10, 2018
@hangsu-ma
Copy link
Author

@hangsu-ma hangsu-ma commented Jul 10, 2018

Hi @sashka

Agreed changing underlining system tools can fix this problem without Ansible code change. But this means an upgrade of these system package/s will be required, which might not be desired in all situations for obvious reasons.
Consider the common case where user need to install some common packages, but because their infra is behind proxy(which is very common in corp world, btw). They will have to upgrade to a newer version of their OS, this does not sound right. Also other alternative tools like Chef does not have this problem.

On the other hand, this change will fix the problem. I can see why you consider it a workaround for random edge case, but I would suggest it's a fix for an Ansible bug, which render apt_key and apt_repository modules broken for Ubuntu behind proxy. Unless there is a better approach available.
I did some digging and listed details below, long in short, I can't see better solution atm, please let me know otherwise.

Furthermore, the changes are in Ansible implementation details and not exposed to user. This is the precise reason one would use Ansible or an alternative for: Ansible handles the edge cases and workarounds, keep the playbook plain and simple.

some relevant information and documentations:

I am not openpgp expert, but what i get is

  • http_proxy is actually supported by low level tooling
  • it is been ignored by default, and can be turned on by pass in honor-http-proxy

does not look like one can pass in honor-http-proxy at apt-key level.

root@vagrant:~# apt-key adv --keyserver-options honor-http-proxy --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24
Executing: /tmp/apt-key-gpghome.R4SfNSiw6w/gpg.1.sh --keyserver-options honor-http-proxy --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24
gpg: keyserver option 'honor-http-proxy' is unknown
^C
gpg: signal Interrupt caught ... exiting
root@vagrant:~# apt-key adv --honor-http-proxy --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24
Executing: /tmp/apt-key-gpghome.0rxsxeRITg/gpg.1.sh --honor-http-proxy --keyserver keyserver.ubuntu.com --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24
gpg: WARNING: "--honor-http-proxy" is an obsolete option - it has no effect

Also tried to update config files in ~/.gnupg folder, didn't work.

So two other possible solution:

  • enable honor-http-proxy by default for apt-key, so it will respect the proxy if exists.
  • use temp home dir for gnupg for the ansible process and use a temp conf file in which honor-http-proxy is specified

Both of them requires changes in apt-key or replace/rewrite it all together in apt_key and apt_repository modules, which sounds like overkill for me, and will also introduce future maintenance issues.
Let me know if it make any sense.

@ansibot ansibot added the stale_ci label Jul 18, 2018
@ansibot ansibot added needs_revision and removed stale_ci core_review labels Jul 31, 2018
@ansibot
Copy link
Contributor

@ansibot ansibot commented Jul 31, 2018

The test ansible-test sanity --test pep8 [explain] failed with 7 errors:

lib/ansible/modules/packaging/os/apt_repository.py:430:9: E265 block comment should start with '# '
lib/ansible/modules/packaging/os/apt_repository.py:432:9: E265 block comment should start with '# '
lib/ansible/modules/packaging/os/apt_repository.py:433:12: E225 missing whitespace around operator
lib/ansible/modules/packaging/os/apt_repository.py:433:29: E231 missing whitespace after ','
lib/ansible/modules/packaging/os/apt_repository.py:433:32: E231 missing whitespace after ','
lib/ansible/modules/packaging/os/apt_repository.py:443:25: E225 missing whitespace around operator
lib/ansible/modules/packaging/os/apt_repository.py:463:32: E128 continuation line under-indented for visual indent

click here for bot help

@ansibot ansibot added the ci_verified label Jul 31, 2018
@ansibot ansibot added core_review and removed ci_verified needs_revision labels Jul 31, 2018
@ansibot ansibot added the stale_ci label Aug 16, 2018
@ansibot ansibot removed the stale_ci label Nov 25, 2018
@ansibot ansibot added the stale_ci label Dec 4, 2018
@maxamillion
Copy link
Contributor

@maxamillion maxamillion commented Dec 5, 2018

@sashka @hangsu-ma in respect to the "all apt modules have to carry a work around", would it make sense to create an apt focused module_util in order to provide a consistent solution to these kinds of situations?

@ansibot ansibot added needs_rebase and removed core_review labels Jan 16, 2019
@ansibot ansibot added test core_review and removed needs_revision labels Apr 10, 2020
@ansibot ansibot added the stale_ci label Apr 29, 2020
@bebosudo
Copy link

@bebosudo bebosudo commented May 13, 2020

I had the same issue on ubuntu 18.04; here's another version that uses less regex and more standard libraries: https://gist.github.com/bebosudo/91837eec70b3d06a9278b3c9ae5289f1
Feel free to use it if you find it useful for this PR.

Edit: Updated script to work on py2 (ipaddress is py3 only).
Use socket and struct to parse IP from string to binary. IPv4 only, but should be easy to create a version for IPv6 with socket.inet_pton()

@ansibot
Copy link
Contributor

@ansibot ansibot commented May 18, 2020

@hangsu-ma The following file(s) in this pull request are bundled copies of modules used to support incidental tests and should not be updated:

  • test/support/integration/plugins/module_utils/common/network.py
    • Possible match in the following collections: os_migrate.os_migrate

Because the original module(s) have been migrated to collections, please re-submit this pull request in relevant collection repositories, typically under https://github.com/ansible-collections.

If you need further assistence with identifying the correct repository, please stop by IRC or the mailing list:

click here for bot help

@ansibot ansibot added needs_revision core_review and removed core_review stale_ci needs_revision labels May 18, 2020
hangsu-ma added 2 commits May 20, 2020
So that the caller can determine if proxy settings should be applied.
Supports most common form of no_proxy listed below:
1. no_proxy is a comma separated list of one or more items below
2. full domain name or hostname
3. containing * wildcard. example: *.example.com
4. leading or trailing dot (.). example: 192.168.56.
5. subnet range. example: 192.168.23.0/24

NOTE: no DNS look up is carried out
Similar reason behind ansible#42443
apt_repository module calls apt-key to add new repo source on ubuntu.
apt-key does not respect Acquire::http::Proxy specified in apt conf files, nor http_proxy environment variable.
More discussion about these behaviours can be found here: https://bugs.launchpad.net/ubuntu/+source/software-properties/+bug/1433761

keyserver-options are used to pass in proxy settings for apt-key, example:
sudo apt-key adv --keyserver-options http-proxy=http://username:password@proxy.example.com:8080 --keyserver keyserver.ubuntu.com --recv-keys GPG_KEY

This fix parse http_proxy and no_proxy environment variables and pass on proxy to apt-key using --keyserver-options if ubuntu key server is not in no_proxy list.
@ansibot ansibot added needs_revision core_review and removed core_review needs_revision labels May 21, 2020
@ansibot ansibot added the stale_ci label May 30, 2020
@ansibot ansibot added needs_revision pre_azp and removed core_review stale_ci labels Dec 9, 2020
@opoplawski
Copy link
Contributor

@opoplawski opoplawski commented Jan 29, 2021

Ping? Just ran into this issue myself.

@ansibot ansibot added the needs_rebase label Feb 14, 2021
@nitzmahone nitzmahone added the P3 label Mar 23, 2022
@nitzmahone
Copy link
Member

@nitzmahone nitzmahone commented Mar 23, 2022

Since apt-key is deprecated, bigger picture, we probably need to fix the underlying module to directly fetch the keys, eg: https://www.linuxuprising.com/2021/01/apt-key-is-deprecated-how-to-add.html - if we used fetch_url to do this, the system proxy config would be respected "for free".

@nitzmahone nitzmahone added the waiting_on_contributor label Mar 23, 2022
@Abam
Copy link

@Abam Abam commented Jun 1, 2022

Any plan to fix this issue ?

The apt_repository module is still broken since 2018 (#42534).

The last comment from @nitzmahone in the issue #42534 is misleading.

apt-key is called without the proxy option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.7 bug has_issue module needs_rebase needs_revision new_contributor P3 packaging pre_azp support:core test waiting_on_contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants