Merge ~adam-collard/maas:bootsources-proxy into maas:master

[Adam Collard]

Remove http{,s}_proxy from environ when fetching bootsources

If MAAS has been configured not to use a HTTP{,/S} proxy, then it will now remove any proxies from the environment it executes simplestreams in to fetch images.

This test was failing before since although not overriding the proxy, we weren't ensuring it was unset.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootsources-proxy lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 7abc3218a7c474d3ed1ef85d73aedab409bdfbf1

review: Approve

[Alberto Donato (ack)]

+1

it's a pity that we have to set it globally but I don't see a better way for now

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bootsources-proxy lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/11048/console
COMMIT: f5b70f44c659c1e38eac198d67f1e7a67843fdc1

review: Needs Fixing

[Adam Collard (adam-collard)]

jenkins: !test

diff --git a/src/maasserver/bootsources.py b/src/maasserver/bootsources.py
index 8ca822d..0f3e1f1 100644
--- a/src/maasserver/bootsources.py
+++ b/src/maasserver/bootsources.py
@@ -113,8 +113,8 @@ def get_simplestreams_env():
             no_proxy_hosts = "127.0.0.1,localhost"
             # When using a proxy and using an image mirror, we may not want
             # to use the proxy to download the images, as they could be
-            # localted in the local network, hence it makes no sense to use
-            # it. With this, we add the image mirror localtion(s) to the
+            # located in the local network, hence it makes no sense to use
+            # it. With this, we add the image mirror location(s) to the
             # no proxy variable, which ensures MAAS contacts the mirror
             # directly instead of through the proxy.
             no_proxy = Config.objects.get_config("boot_images_no_proxy")
@@ -124,6 +124,11 @@ def get_simplestreams_env():
                     host = urlparse(source["url"]).netloc.split(":")[0]
                     no_proxy_hosts = ",".join((no_proxy_hosts, host))
             env["no_proxy"] = no_proxy_hosts
+    else:
+        # The proxy is disabled, let's not accidentally use proxy from
+        # encompassing environment.
+        env["http_proxy"] = ""
+        env["https_proxy"] = ""
     return env
 
 
diff --git a/src/maasserver/tests/test_bootsources.py b/src/maasserver/tests/test_bootsources.py
index 5b70e8e..deb3412 100644
--- a/src/maasserver/tests/test_bootsources.py
+++ b/src/maasserver/tests/test_bootsources.py
@@ -7,8 +7,7 @@
 import html
 from os import environ
 import random
-from unittest import skip
-from unittest.mock import ANY, MagicMock
+from unittest.mock import ANY, MagicMock, patch
 
 from fixtures import EnvironmentVariableFixture
 from requests.exceptions import ConnectionError
@@ -410,7 +409,6 @@ class TestPrivateCacheBootSources(MAASTransactionServerTestCase):
             MockCalledOnceWith(ANY, user_agent=get_maas_user_agent()),
         )
 
-    @skip("XXX: GavinPanella 2015-12-04 bug=1546235: Fails spuriously.")
     def test_doesnt_have_env_http_and_https_proxy_set_if_disabled(self):
         proxy_address = factory.make_name("proxy")
         Config.objects.set_config("http_proxy", proxy_address)
@@ -419,7 +417,16 @@ class TestPrivateCacheBootSources(MAASTransactionServerTestCase):
             self
         )
         factory.make_BootSource(keyring_data=b"1234")
-        cache_boot_sources()
+        env_http_proxy_address = factory.make_name("http-proxy")
+        env_https_proxy_address = factory.make_name("https-proxy")
+        with patch.dict(
+            "os.environ",
+            {
+                "http_proxy": env_http_proxy_address,
+                "https_proxy": env_https_proxy_address,
+            },
+        ):
+            cache_boot_sources()
         self.assertEqual(
             ("", ""),
             (