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

Handle new juju charm proxy settings and https keyserver URLs #248

Merged
merged 9 commits into from Feb 26, 2019

Conversation

Projects
None yet
4 participants
@dshcherb
Copy link
Contributor

commented Nov 24, 2018

This change reworks proxy server handling for add-apt-repository and
key retrieval cases and introduces support for new juju-prefixed proxy
setting environment variables that do not modify the http(s) connections
made from hook execution environments by charm code or forked and
execve-ed applications that were given environment variables of a parent
process. The new proxy settings are available as of Juju 2.4.0 but are
properly applied as of Juju 2.4.2 (see lp:1782236).

add-apt-repository comes from the software-properties package and only
reacts to HTTP_PROXY and HTTPS_PROXY environment variables as of bionic
when it was switched to using curl (lp:1433761) and HTTPS-based Ubuntu
keyserver URLs. For Xenial and other releases older than Bionic it is
necessary to use charm options specifying GPG keys in the radix format
AND source URLs in the following formats to avoid triggering the
add-apt-repository behavior related to GPG keys for ppa shortcuts:

deb [arch=<arches-csv] uri distribution [component1] [component2] [...]

See lp:1433761 and https://git.launchpad.net/ubuntu/+source/software-properties/commit/?id=f57935235ca0f52b32da7efe2a24cb26c7fc4573

A manpage for apt-key mentions the following in a section about the
"add" command:
"Note: Instead of using this command a keyring should be placed directly
in the /etc/apt/trusted.gpg.d/ directory with a descriptive name and
either "gpg" or "asc" as file extension."

The support for /etc/apt/trusted.gpg.d/ goes back to 2010:
https://salsa.debian.org/nathanruiz-guest/apt/commit/c24f6ce22cd6720004addad2e3382b3caa6b1b7c

Using "asc" in this directory is only supported as of apt 1.4.

https://salsa.debian.org/nathanruiz-guest/apt/commit/f77ea8235cafb258d1cb0b2b90e95aa36e5c4650
https://salsa.debian.org/nathanruiz-guest/apt/commit/2906182db398419a9c59a928b7ae73cf7c7aa307

Binary GPG format is used in this change given that trusty uses 1.0.1,
xenial uses 1.2.x and only bionic has 1.6.x. This requires de-armoring
of ASCII armor-formatted GPG keys downloaded from the Ubuntu keyserver.

apt-key usage is completely removed by this change.

HTTPS is used for key retrieval with this change which is a functional
change to a more secure way of retrieving GPG keys. A subset of charms
using PPAs will be affected by that.

Since HTTPS is used, if SSLBump-like HTTPS proxies are in place,
they will impersonate keyserver.ubuntu.com and generate a certificate
with keyserver.ubuntu.com in the CN field or in SubjAltName fields of a
certificate. If such proxy behavior is expected it is necessary to add
the CA certificate chain containing the intermediate CA of the SSLBump
proxy to every machine that this code runs on via ca-certs cloud-init
directive (via cloudinit-userdata model-config) or via other means
(such as through a custom charm option). curl relies on openssl provided
by the distribution which means it is affected by the system trusted
certificate store. Also note that DNS resolution for the hostname in a
URL is done at a proxy server - not at the client side.

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 12451f1 to 936d83a Nov 24, 2018

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

Pull-request updated, HEAD is now 936d83a

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

Manual functional test environment setup:
https://gist.github.com/dshcherb/adecba7f2f9b7a0aea9f2e6c8c9df3bd

Cases tested manually (landscape server charm, new-style proxy settings, default gateway disabled before charm execution):

  • trusty, xenial with the deb-style source (16.06 and 17.03) and key id specified;
  • bionic with a ppa shortcut-style 18.03 source;
  • xenial and bionic with the deb-style source and ASCII armor key specified via a charm option;

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 936d83a to f7cf302 Dec 3, 2018

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

Pull-request updated, HEAD is now f7cf302

@ajkavanagh
Copy link
Contributor

left a comment

This is my first run through.

I was wondering if you could use the --with-colons option with the gpg command to make it predictably machine readable? i.e. avoid the re.search(...) functions, or at least use them on predictable text output?

Otherwise, a nice set of changes, but I think just needs a few tweaks to bring it into the library. Thanks.

log(error)
raise GPGKeyError(error)
log("Writing provided PGP key in the binary format", level=DEBUG)
key_bytes = key.encode('utf-8')

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

This the encode/encode of a str to bytes is usually only done in PY3 -- is there a reason this isn't fenced with an if six.PY3?

Also, it would be good to add a :type keyid: (type in PY2, type in PY3) to the functions docstring

or binary GPG key material. Can be used, for example, to generate file
names for keys passed via charm options.
@param key_material: ASCII armor-encoded or binary GPG key material (bytes)

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

Let's use :param key_material: and :type key_material: as this seems to be the default that is starting to propagate through the code.

@param key_material: ASCII armor-encoded or binary GPG key material (bytes)
"""
# trusty, xenial and bionic handling differs due to gpg 1.x to 2.x change
release = lsb_release()['DISTRIB_CODENAME'].lower()

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

I can't believe we don't have a utils function to do this! And it turns out we don't! I think I'll raise a bug; it's ludicrous that this exact line of code is repeated all over charm-helpers and appears in numerous charms; even charms.openstack uses the exact same line.

ps = subprocess.Popen(['gpg', '--with-fingerprint'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
stdin=subprocess.PIPE)

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

I think the above would be better as:

if is_gpgv2_distro:
    cmd = "gpg --import-options show-only"
else:
    cmd = "gpg --with-fingerprint"
ps = subprocess.Popen(cmd.split(), ...)

It helps to identify what is different and what is the same in the Popen command by just using the if statement to show the differences.

Show resolved Hide resolved charmhelpers/fetch/ubuntu.py


def _write_apt_gpg_keyfile(key_name, key_material):
"""

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

Start title on same line as """ please.



def _env_proxy_settings(selected_settings=None):
"""

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

Please change docstring comment to title + description. Also title on same line as """.

This comment has been minimized.

Copy link
@petevg

petevg Feb 12, 2019

Contributor

@ajkavanagh better now?

Show resolved Hide resolved charmhelpers/fetch/ubuntu.py Outdated
'no_proxy': 'NO_PROXY',
'ftp': 'FTP_PROXY'
}
if not selected_settings:

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

Please explicitly check for None if using it as a sentinel for "parameter not set by caller". e.g.

if selected_settings is None:
    selected_settings = SUPPORTED_SETTINGS

This makes it clear that this piece of code is actually doing the default, rather than if the user passed {} for example.

charm_var_val = os.getenv('JUJU_CHARM_{}'.format(var))
if charm_var_val:
proxy_settings[var] = charm_var_val
# add a lowercase version as well

This comment has been minimized.

Copy link
@ajkavanagh

ajkavanagh Dec 5, 2018

Contributor

I don't think this comment is needed. i.e. there's no comment in the above equivalent.

@ryan-beisner
Copy link
Contributor

left a comment

@dshcherb Thank you for your work on this. When you post your next commits to address the review, can you please also rebase with charmhelpers master?

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Dec 6, 2018

@ajkavanagh @ryan-beisner

Thanks for the review and comments, I'll address them and update the change.

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from f7cf302 to 1e464f0 Jan 28, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 28, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

Pull-request updated, HEAD is now 1e464f0

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

@ajkavanagh Thanks again for the thorough review, I think the latest revision is a bit cleaner because of that.

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 1e464f0 to 56b3cc1 Jan 28, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 28, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

Pull-request updated, HEAD is now 56b3cc1

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 56b3cc1 to 08e4fc5 Jan 28, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 28, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

Pull-request updated, HEAD is now 08e4fc5

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 08e4fc5 to a1345c1 Jan 28, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 28, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

Pull-request updated, HEAD is now a1345c1

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from a1345c1 to e43d24a Jan 28, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 28, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2019

Pull-request updated, HEAD is now e43d24a

SUPPORTED_SETTINGS = {
'http': 'HTTP_PROXY',
'https': 'HTTPS_PROXY',
'no_proxy': 'NO_PROXY',

This comment has been minimized.

Copy link
@petevg

petevg Jan 30, 2019

Contributor

juju-no-proxy can contain subnets, which the NO_PROXY environment variable does not support. Are we introducing a bug by passing the value of juju-no-proxy to NO_PROXY without attempting to expand those subnets?

This comment has been minimized.

Copy link
@dshcherb

dshcherb Jan 30, 2019

Author Contributor

@petevg

I would rather leave it as-is because the underlying software may or may not support this notation.

add-apt-repository specifically doesn't need no_proxy because it (at worst) connects to two endpoints:

  1. launchpad.net to resolve a ppa shortcut to a full deb line;
  2. keyserver.ubuntu.com to download a GPG key for a repo to add.

Both of these will be accessed through a proxy in real-life scenarios. If they really need to be accessed without a proxy when proxy settings are configured they can be added to no_proxy as hostnames.

So there is no harm in the fact that curl invoked by add-apt-repository will ignore no_proxy.

The strong argument against expanding no_proxy in every situation is that expanded no_proxy can reach the limits imposed on argument size in terms of a child process stack memory - the decision on whether to expand no_proxy or not should be done by the caller of _env_proxy_settings:

https://elixir.bootlin.com/linux/v4.14.96/source/fs/exec.c#L244

static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
		int write)
// ...
		/*
		 * We've historically supported up to 32 pages (ARG_MAX)
		 * of argument strings even with small stacks
		 */
		if (size <= ARG_MAX)
			return page;
// ...
		/*
		 * Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
		 * (whichever is smaller) for the argv+env strings.
		 * This ensures that:
		 *  - the remaining binfmt code will not run out of stack space,
		 *  - the program will have a reasonable amount of stack left
		 *    to work from.
		 */
		limit = _STK_LIM / 4 * 3;
		limit = min(limit, rlimit(RLIMIT_STACK) / 4);
		if (size > limit)
			goto fail;

https://elixir.bootlin.com/linux/v4.14.96/source/include/uapi/linux/resource.h#L66
#define _STK_LIM (8*1024*1024)

This comment has been minimized.

Copy link
@petevg

petevg Jan 31, 2019

Contributor

@dshcherb Hmmm ... I'm tempted to say that we shouldn't include NO_PROXY in the default list of supported settings.

I guess the question is this: if I naively pass the result of this routine to _env_proxy_settings, without specifying any arguments, what is the best way for this to fail?

  1. It ignores NO_PROXY by default, which means that I struggle to figure out why it isn't respecting my juju-no-proxy setting.
  2. I have something in juju-no-proxy that will cause mischief, and my charm falls over as a result.

This comment has been minimized.

Copy link
@petevg

petevg Jan 31, 2019

Contributor

@dshcherb After talking this over with the team, I think that it would be better to handle both the cases where no_proxy has a cidr, and where it is too big inline here.

If juju-no-proxy contains a cidr, attempt to expand it.
If the cidr is now too big, either log a warning, or throw an Exception.

The goal is to make it easy to troubleshoot if something does go wrong. And also to attempt to do the "right" thing in the first place.

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from fd94e2a to b9f658e Jan 30, 2019

dshcherb added a commit to dshcherb/charm-helpers that referenced this pull request Jan 30, 2019

repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in juju#248
@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2019

Pull-request updated, HEAD is now b9f658e

Rebased since #268 got merged.

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2019

Pull-request updated, HEAD is now 96e4cf6

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

Pull-request updated, HEAD is now f48e318

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

Rebased to master again. The previous test failure was some kind of timeout during test execution.

ReadTimeoutError: HTTPSConnectionPool(host='files.pythonhosted.org', port=443): Read timed out.
The command "pip install -r test_requirements.txt" failed and exited with 2 during .
@ajkavanagh

This comment has been minimized.

Copy link
Contributor

commented Feb 18, 2019

This is a really big piece of work, and nicely done. However, it would be useful if a functional test of these changes in a charm could be presented as evidence that it does work -- I see the unit tests work, but it's too large a patch for me to determine if it actually works in a charm. Thanks!

dshcherb added some commits Nov 24, 2018

Handle new juju charm proxy settings and https keyserver URLs
This change reworks proxy server handling for add-apt-repository and
key retrieval cases and introduces support for new juju-prefixed proxy
setting environment variables that do not modify the http(s) connections
made from hook execution environments by charm code or forked and
execve-ed applications that were given environment variables of a parent
process. The new proxy settings are available as of Juju 2.4.0 but are
properly applied as of Juju 2.4.2 (see lp:1782236).

add-apt-repository comes from the software-properties package and only
reacts to HTTP_PROXY and HTTPS_PROXY environment variables as of bionic
when it was switched to using curl (lp:1433761) and HTTPS-based Ubuntu
keyserver URLs. For Xenial and other releases older than Bionic it is
necessary to use charm options specifying GPG keys in the radix format
AND source URLs in the following formats to avoid triggering the
add-apt-repository behavior related to GPG keys for ppa shortcuts:

deb [arch=<arches-csv] uri distribution [component1] [component2] [...]

See lp:1433761 and https://git.launchpad.net/ubuntu/+source/software-properties/commit/?id=f57935235ca0f52b32da7efe2a24cb26c7fc4573

A manpage for apt-key mentions the following in a section about the
"add" command:
"Note: Instead of using this command a keyring should be placed directly
in the /etc/apt/trusted.gpg.d/ directory with a descriptive name and
either "gpg" or "asc" as file extension."

The support for /etc/apt/trusted.gpg.d/ goes back to 2010:
https://salsa.debian.org/nathanruiz-guest/apt/commit/c24f6ce22cd6720004addad2e3382b3caa6b1b7c

Using "asc" in this directory is only supported as of apt 1.4.

https://salsa.debian.org/nathanruiz-guest/apt/commit/f77ea8235cafb258d1cb0b2b90e95aa36e5c4650
https://salsa.debian.org/nathanruiz-guest/apt/commit/2906182db398419a9c59a928b7ae73cf7c7aa307

Binary GPG format is used in this change given that trusty uses 1.0.1,
xenial uses 1.2.x and only bionic has 1.6.x. This requires de-armoring
of ASCII armor-formatted GPG keys downloaded from the Ubuntu keyserver.

apt-key usage is completely removed by this change.

HTTPS is used for key retrieval with this change which is a functional
change to a more secure way of retrieving GPG keys. A subset of charms
using PPAs will be affected by that.

Since HTTPS is used, if SSLBump-like HTTPS proxies are in place,
they will impersonate keyserver.ubuntu.com and generate a certificate
with keyserver.ubuntu.com in the CN field or in SubjAltName fields of a
certificate. If such proxy behavior is expected it is necessary to add
the CA certificate chain containing the intermediate CA of the SSLBump
proxy to every machine that this code runs on via ca-certs cloud-init
directive (via cloudinit-userdata model-config) or via other means
(such as through a custom charm option). curl relies on openssl provided
by the distribution which means it is affected by the system trusted
certificate store. Also note that DNS resolution for the hostname in a
URL is done at a proxy server - not at the client side.
repo and keyserver rework: post-review changes
* merged get_distrib_codename branch;
* converted fetch/ubuntu.py and tests to use get_distrib_codename;
* simplified gpg command line output by using --with-columns;
* code-style and other changes as requested in #248

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from f48e318 to d67fe9c Feb 25, 2019

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2019

Pull-request updated, HEAD is now d67fe9c

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from d67fe9c to 77b36dd Feb 25, 2019

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2019

Pull-request updated, HEAD is now 77b36dd

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

@ajkavanagh
@petevg
@ryan-beisner

I moved env_proxy_settings to "hookenv" instead of "fetch" as this function is generally useful for charms to use and works with a hook context. I actually have a use for it already in a vendor charm that does a curl not minding new proxy settings.

Test results with the latest code when only proxy server connectivity is available:

xenial (full source line and a full-text ASCII armor key)
https://gist.github.com/dshcherb/6e1a4fff161e2091e92ed19aeca6edb0#file-proxy-test-landscape-xenial-full-txt

xenial (full source line and a long key id)
https://gist.github.com/dshcherb/9f59874dca12abbf24597647013c1b42

bionic (ppa shortcut and medium key id specified => add-apt-repository to launchpad.net connectivity via a proxy + keyserver via a proxy):
https://gist.github.com/dshcherb/67a9d7ea671c093124aa6e8a37a1e89d#file-proxy-landscape-bionic-ppa-keyid-txt

trusty (full source line and short key id)
https://gist.github.com/dshcherb/0b6fc364b34785b428f0ec4d92b2e2c9/98f7608d6f037c99a3537ceb94563b9468f27126#file-proxy-test-landscape-trusty-full-keyid-txt

@ajkavanagh
Copy link
Contributor

left a comment

Great stuff!

Just one possible bug, and a single improvement.

The tests look good, and neither of the changes I'm suggesting will require retesting. Thanks.

Show resolved Hide resolved charmhelpers/contrib/openstack/amulet/utils.py Outdated
Show resolved Hide resolved charmhelpers/core/hookenv.py Outdated

dshcherb and others added some commits Jan 30, 2019

Added warning to _env_proxy_settings if passing through cidr to no_proxy
_env_proxy_settings will pass the value of juju-no-proxy transparently
to NO_PROXY in a command's environment. juju-no-proxy may contain a
cidr, which is not supported by most applications using NO_PROXY. We
therefore log a warning message that things may break, so that charm
authors can troubleshoot if their charm fails as a result.
Added tests for cidr re.
Added a series of simple tests to verify that our regular expression
catches cidrs and not ips.
Added wildcard domains to check for cidrs in fetch/ubuntu
juju-no-proxy can also take domains as an argument. For example
.foo.com or *.foo.com. Tweaked a check for cidrs so that it catches
these as well.
Fixed up docstring, per comments on PR.
Now conforms better to standards.
Move _env_proxy_settings to hookenv
This function seems general enough for charms to use other than for
add-apt-repository (e.g. curl or wget invocations, application-specific
proxy config and so on). It also belongs to hook tools and helpers and,
therefore, is moved to hookenv by this change.

@dshcherb dshcherb force-pushed the dshcherb:repo-and-keyserver-rework branch from 77b36dd to 842e5e1 Feb 26, 2019

@dshcherb

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2019

Pull-request updated, HEAD is now 842e5e1

My review was a request to rebase before proceeding, which is done (thank you).

@petevg petevg merged commit 0f198e4 into juju:master Feb 26, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

dshcherb added a commit to dshcherb/contrail-charms that referenced this pull request Feb 27, 2019

Sync charm-helpers to include env_proxy_settings
env_proxy_settings helper was added in

juju/charm-helpers#248

to address the use of new-style proxy settings:

juju-http-proxy
juju-https-proxy
juju-no-proxy

They do not add {HTTP,HTTPS,NO}_PROXY environment variables to the
hook environment and instead use JUJU_CHARM_{HTTP,HTTPS,NO}_PROXY
variables that have to be used by the charm explicitly so that hook
invocations are not adversely affected by them.

This charmhelper sync is solely for getting the new helper code.

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>

dshcherb added a commit to dshcherb/contrail-charms that referenced this pull request Feb 27, 2019

Run curl in a proxy-aware mode
In order to use new-style proxy settings which do not affect hook
environment by default we need to use a helper introduced in
charm-helpers:

juju/charm-helpers#248

This will make sure that the curl invocation used to fetch a GPG key
over https explicitly uses the appropriate https-proxy variable set via
model-config.

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>

Andrey-mp added a commit to Juniper/contrail-charms that referenced this pull request Feb 28, 2019

Run curl in a proxy-aware mode (#47)
* Switch charm-helpers to github instead of lp

charm-helpers have been moved to GH so to perform a sync a different
approach is needed.

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>

* Sync charm-helpers to include env_proxy_settings

env_proxy_settings helper was added in

juju/charm-helpers#248

to address the use of new-style proxy settings:

juju-http-proxy
juju-https-proxy
juju-no-proxy

They do not add {HTTP,HTTPS,NO}_PROXY environment variables to the
hook environment and instead use JUJU_CHARM_{HTTP,HTTPS,NO}_PROXY
variables that have to be used by the charm explicitly so that hook
invocations are not adversely affected by them.

This charmhelper sync is solely for getting the new helper code.

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>

* Run curl in a proxy-aware mode

In order to use new-style proxy settings which do not affect hook
environment by default we need to use a helper introduced in
charm-helpers:

juju/charm-helpers#248

This will make sure that the curl invocation used to fetch a GPG key
over https explicitly uses the appropriate https-proxy variable set via
model-config.

Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.