launchpad-reviewers team mailing list archive

[Colin Watson <mp+425835@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:remove-development-builder-proxy-config into launchpad:master.

Commit message:
Remove development builder proxy configuration

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425835

The development configuration for the builder proxy is somewhat convenient if you're actually working on the builder proxy.  In other situations, it's an obstacle: it's one more obscure piece of configuration that you typically need to remove if you're trying to set up a local builder.  Local builders generally don't have the sort of strict DNS/networking configuration that we run with in production and that requires this proxy, so the proxy is normally of little benefit there.

Let's just remove this configuration to simplify the setup procedure, and enable it only for the tests that need it; people who need to set up a local builder proxy as well should have little difficulty configuring Launchpad to use it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-development-builder-proxy-config into launchpad:master.

diff --git a/configs/development/launchpad-lazr.conf b/configs/development/launchpad-lazr.conf
index b8ba73c..9cd9897 100644
--- a/configs/development/launchpad-lazr.conf
+++ b/configs/development/launchpad-lazr.conf
@@ -13,10 +13,6 @@ root: /var/tmp/builddmaster/
 uploader: scripts/process-upload.py -Mvv
 bzr_builder_sources_list: None
 authentication_endpoint: http://xmlrpc-private.launchpad.test:8087/authserver
-builder_proxy_auth_api_admin_username: admin-launchpad.test
-builder_proxy_auth_api_endpoint: http://builder-proxy.launchpad.test:8080/tokens
-builder_proxy_host: builder-proxy.launchpad.test
-builder_proxy_port: 3128
 
 [canonical]
 show_tracebacks: True
diff --git a/lib/lp/buildmaster/tests/builderproxy.py b/lib/lp/buildmaster/tests/builderproxy.py
index 356bc0f..1fa6225 100644
--- a/lib/lp/buildmaster/tests/builderproxy.py
+++ b/lib/lp/buildmaster/tests/builderproxy.py
@@ -86,9 +86,11 @@ class InProcessProxyAuthAPIFixture(fixtures.Fixture):
         config.push("in-process-proxy-auth-api-fixture", dedent("""
             [builddmaster]
             builder_proxy_auth_api_admin_secret: admin-secret
-            builder_proxy_auth_api_endpoint: http://%s:%s/tokens
-            """) %
-            (port.getHost().host, port.getHost().port))
+            builder_proxy_auth_api_admin_username: admin-launchpad.test
+            builder_proxy_auth_api_endpoint: http://{host}:{port}/tokens
+            builder_proxy_host: {host}
+            builder_proxy_port: {port}
+            """).format(host=port.getHost().host, port=port.getHost().port))
         self.addCleanup(config.pop, "in-process-proxy-auth-api-fixture")
 
 
diff --git a/lib/lp/charms/tests/test_charmrecipebuildbehaviour.py b/lib/lp/charms/tests/test_charmrecipebuildbehaviour.py
index a473487..308ee08 100644
--- a/lib/lp/charms/tests/test_charmrecipebuildbehaviour.py
+++ b/lib/lp/charms/tests/test_charmrecipebuildbehaviour.py
@@ -216,6 +216,16 @@ class TestAsyncCharmRecipeBuildBehaviour(
 
     @defer.inlineCallbacks
     def test_requestProxyToken_unconfigured(self):
+        self.pushConfig("builddmaster", builder_proxy_host=None)
+        job = self.makeJob()
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual([], self.proxy_api.tokens.requests)
+        self.assertNotIn("proxy_url", args)
+        self.assertNotIn("revocation_endpoint", args)
+
+    @defer.inlineCallbacks
+    def test_requestProxyToken_no_secret(self):
         self.pushConfig(
             "builddmaster", builder_proxy_auth_api_admin_secret=None)
         job = self.makeJob()
diff --git a/lib/lp/code/model/tests/test_cibuildbehaviour.py b/lib/lp/code/model/tests/test_cibuildbehaviour.py
index 1d25aa3..efbbd0b 100644
--- a/lib/lp/code/model/tests/test_cibuildbehaviour.py
+++ b/lib/lp/code/model/tests/test_cibuildbehaviour.py
@@ -213,6 +213,16 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
 
     @defer.inlineCallbacks
     def test_requestProxyToken_unconfigured(self):
+        self.pushConfig("builddmaster", builder_proxy_host=None)
+        job = self.makeJob()
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual([], self.proxy_api.tokens.requests)
+        self.assertNotIn("proxy_url", args)
+        self.assertNotIn("revocation_endpoint", args)
+
+    @defer.inlineCallbacks
+    def test_requestProxyToken_no_secret(self):
         self.pushConfig(
             "builddmaster", builder_proxy_auth_api_admin_secret=None)
         job = self.makeJob()
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index 51f38de..53adac5 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -220,6 +220,17 @@ class TestAsyncOCIRecipeBuildBehaviour(
 
     @defer.inlineCallbacks
     def test_requestProxyToken_unconfigured(self):
+        self.pushConfig("builddmaster", builder_proxy_host=None)
+        [ref] = self.factory.makeGitRefs()
+        job = self.makeJob(git_ref=ref)
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual([], self.proxy_api.tokens.requests)
+        self.assertNotIn("proxy_url", args)
+        self.assertNotIn("revocation_endpoint", args)
+
+    @defer.inlineCallbacks
+    def test_requestProxyToken_no_secret(self):
         self.pushConfig(
             "builddmaster", builder_proxy_auth_api_admin_secret=None)
         [ref] = self.factory.makeGitRefs()
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 9f8c753..f767b0f 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -291,6 +291,17 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
 
     @defer.inlineCallbacks
     def test_requestProxyToken_unconfigured(self):
+        self.pushConfig("builddmaster", builder_proxy_host=None)
+        branch = self.factory.makeBranch()
+        job = self.makeJob(branch=branch)
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual([], self.proxy_api.tokens.requests)
+        self.assertNotIn("proxy_url", args)
+        self.assertNotIn("revocation_endpoint", args)
+
+    @defer.inlineCallbacks
+    def test_requestProxyToken_no_secret(self):
         self.pushConfig(
             "builddmaster", builder_proxy_auth_api_admin_secret=None)
         branch = self.factory.makeBranch()