Merge ~lutostag/layer-snap:use_juju_proxy into ~stub/layer-snap:master

[Greg Lutostanski]

Use proxying env vars from juju (with config option to disable).

So we don't have to set the proxy to the same as juju model proxy config for every charm we deploy.

[Stuart Bishop (stub)]

Do we actually want the configuration item?

I can't think of any situation where people set http_proxy but do not want it used to access the snap store (as opposed to apt_http_proxy, where that would be very wrong)

Unless we have actual use cases, I'd like the config option removed. We can add it later if needed, but I'd rather not have it polluting the config namespace of all charms using the layer if it isn't needed.

(and now I see this MP, perhaps the separate snap_proxy config item was a bad idea. Maybe we should deprecate that, if we can one day remove it without breaking actual deployments.)

review: Needs Fixing

[Stuart Bishop (stub)]

Otherwise all looks good, except that I'd like to avoid mutating the hookenv.config() dictionary and instead keep using the charms.reactive data_changed() helper. charmhelpers.hookenv needs to be reworked into charms.hookenv at some point, and the mutating config has been deprecated in favor of charmhelpers.unitdata (which data_changed uses under the hook).

[Greg Lutostanski (lutostag)]

Thanks for review. Didn't know about data_changed, good stuff. Made your requested changes. Running a test deployment now...

[Stuart Bishop (stub)]

Yup.

review: Approve

diff --git a/reactive/snap.py b/reactive/snap.py
index aa05314..b2ec73e 100644
--- a/reactive/snap.py
+++ b/reactive/snap.py
@@ -83,6 +83,18 @@ def ensure_snapd():
         subprocess.check_call(cmd, universal_newlines=True)
 
 
+def proxy_settings():
+    proxy_vars = ('http_proxy', 'https_proxy', 'no_proxy')
+    proxy_env = {key: value for key, value in os.environ.items()
+                 if key in proxy_vars}
+
+    snap_proxy = hookenv.config()['snap_proxy']
+    if snap_proxy:
+        proxy_env['http_proxy'] = snap_proxy
+        proxy_env['https_proxy'] = snap_proxy
+    return proxy_env
+
+
 def update_snap_proxy():
     # This is a hack based on
     # https://bugs.launchpad.net/layer-snap/+bug/1533899/comments/1
@@ -90,7 +102,7 @@ def update_snap_proxy():
     # Note we can't do this in a standard reactive handler as we need
     # to ensure proxies are configured before attempting installs or
     # updates.
-    proxy = hookenv.config()['snap_proxy']
+    proxy = proxy_settings()
 
     if get_series() == 'trusty':
         # The hack to configure a snapd proxy only works under
@@ -123,9 +135,9 @@ def create_snap_proxy_conf(path, proxy):
     content = dedent('''\
                         # Managed by Juju
                         [Service]
-                        Environment=http_proxy={}
-                        Environment=https_proxy={}
-                        ''').format(proxy, proxy)
+                        ''')
+    for proxy_key, proxy_value in proxy.items():
+        content += 'Environment={}={}\n'.format(proxy_key, proxy_value)
     host.write_file(path, content.encode())