launchpad-reviewers team mailing list archive

[Ines Almeida <mp+464303@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master.

Commit message:
Refactor buildd-manager builder proxy logic
    
This separates the logic for making calls to the builder proxy and the fetch service out of the BuilderProxyMixin and into their own handlers.
This will make it easier to later add the logic to end sessions or, retreieve metadata.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464303

All tests in `lp.snappy.tests` have be run and passed successfully.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:fetch-service-buildd-manager-refactor into launchpad:master.

diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 3375042..e4da105 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -49,79 +49,159 @@ class BuilderProxyMixin:
         self,
         args: BuildArgs,
         allow_internet: bool = True,
-        fetch_service: bool = False,
+        use_fetch_service: bool = False,
     ) -> Generator[None, Dict[str, str], None]:
         if not allow_internet:
             return
-        if not fetch_service and _get_proxy_config("builder_proxy_host"):
-            token = yield self._requestProxyToken()
-            args["proxy_url"] = (
-                "http://{username}:{password}@{host}:{port}".format(
-                    username=token["username"],
-                    password=token["secret"],
-                    host=_get_proxy_config("builder_proxy_host"),
-                    port=_get_proxy_config("builder_proxy_port"),
-                )
-            )
-            args["revocation_endpoint"] = "{endpoint}/{token}".format(
-                endpoint=_get_proxy_config("builder_proxy_auth_api_endpoint"),
-                token=token["username"],
-            )
-        elif fetch_service and _get_proxy_config("fetch_service_host"):
-            session = yield self._requestFetchServiceSession()
-            args["proxy_url"] = (
-                "http://{session_id}:{token}@{host}:{port}".format(
-                    session_id=session["id"],
-                    token=session["token"],
-                    host=_get_proxy_config("fetch_service_host"),
-                    port=_get_proxy_config("fetch_service_port"),
-                )
-            )
-            args["revocation_endpoint"] = "{endpoint}/{session_id}".format(
-                endpoint=_get_proxy_config("fetch_service_control_endpoint"),
-                session_id=session["id"],
-            )
+        if not use_fetch_service and _get_proxy_config("builder_proxy_host"):
+            proxy_service = BuilderProxy
+        elif use_fetch_service and _get_proxy_config("fetch_service_host"):
+            proxy_service = FetchService
+        else:
+            return
+
+        self.proxy_service = proxy_service(
+            build_id=self.build.build_cookie, worker=self._worker
+        )
+        new_session = yield self.proxy_service.startSession()
+        args["proxy_url"] = new_session["proxy_url"]
+        args["revocation_endpoint"] = new_session["revocation_endpoint"]
+
+
+class BuilderProxy:
+
+    def __init__(self, build_id, worker):
+        self.control_endpoint = _get_value_from_config(
+            "builder_proxy_auth_api_endpoint"
+        )
+        self.proxy_endpoint = "{host}:{port}".format(
+            host=_get_proxy_config("builder_proxy_host"),
+            port=_get_proxy_config("builder_proxy_port"),
+        )
+        self.auth_header = self._getAuthHeader()
+
+        self.build_id = build_id
+        self.worker = worker
+
+    @staticmethod
+    def _getAuthHeader():
+        """Helper method to generate authentication needed to call the
+        builder proxy authentication endpoint."""
 
-    @defer.inlineCallbacks
-    def _requestProxyToken(self):
         admin_username = _get_value_from_config(
             "builder_proxy_auth_api_admin_username"
         )
-        secret = _get_value_from_config("builder_proxy_auth_api_admin_secret")
-        url = _get_value_from_config("builder_proxy_auth_api_endpoint")
+        admin_secret = _get_value_from_config(
+            "builder_proxy_auth_api_admin_secret"
+        )
+        auth_string = f"{admin_username}:{admin_secret}".strip()
+        return b"Basic " + base64.b64encode(auth_string.encode("ASCII"))
+
+    @defer.inlineCallbacks
+    def startSession(self):
+        """Request a token from the builder proxy to be used by the builders.
+
+        :returns: dictionary with an authenticated `proxy_url` for the builder
+        to use, and a `revocation_endpoint` to revoke the token when it's no
+        longer required.
+        """
         timestamp = int(time.time())
         proxy_username = "{build_id}-{timestamp}".format(
-            build_id=self.build.build_cookie, timestamp=timestamp
+            build_id=self.build_id, timestamp=timestamp
         )
-        auth_string = f"{admin_username}:{secret}".strip()
-        auth_header = b"Basic " + base64.b64encode(auth_string.encode("ASCII"))
 
-        token = yield self._worker.process_pool.doWork(
+        token = yield self.worker.process_pool.doWork(
             RequestProxyTokenCommand,
-            url=url,
-            auth_header=auth_header,
+            url=self.control_endpoint,
+            auth_header=self.auth_header,
             proxy_username=proxy_username,
         )
-        return token
 
-    @defer.inlineCallbacks
-    def _requestFetchServiceSession(self):
+        proxy_url = "http://{username}:{password}@{proxy_endpoint}".format(
+            username=token["username"],
+            password=token["secret"],
+            proxy_endpoint=self.proxy_endpoint,
+        )
+        revocation_endpoint = "{endpoint}/{token}".format(
+            endpoint=self.control_endpoint,
+            token=token["username"],
+        )
+
+        return {
+            "proxy_url": proxy_url,
+            "revocation_endpoint": revocation_endpoint,
+        }
+
+
+class FetchService:
+
+    PROXY_URL = "http://{session_id}:{token}@{proxy_endpoint}";
+    START_SESSION_ENDPOINT = "{control_endpoint}"
+    TOKEN_REVOCATION = "{control_endpoint}/session/{session_id}/token"
+    RETRIEVE_METADATA_ENDPOINT = "{control_endpoint}/session/{session_id}"
+    END_SESSION_ENDPOINT = "{control_endpoint}/session/{session_id}"
+
+    def __init__(self, build_id, worker):
+        self.control_endpoint = _get_value_from_config(
+            "fetch_service_control_endpoint"
+        )
+        self.proxy_endpoint = "{host}:{port}".format(
+            host=_get_proxy_config("fetch_service_host"),
+            port=_get_proxy_config("fetch_service_port"),
+        )
+        self.auth_header = self._getAuthHeader()
+
+        self.build_id = build_id
+        self.worker = worker
+        self.session_id = None
+
+    @staticmethod
+    def _getAuthHeader():
+        """Helper method to generate authentication needed to call the
+        fetch service control endpoint."""
         admin_username = _get_value_from_config(
             "fetch_service_control_admin_username"
         )
         secret = _get_value_from_config("fetch_service_control_admin_secret")
-        url = _get_value_from_config("fetch_service_control_endpoint")
+        auth_string = f"{admin_username}:{secret}".strip()
+        return b"Basic " + base64.b64encode(auth_string.encode("ASCII"))
+
+    @defer.inlineCallbacks
+    def startSession(self):
+        """Requests a fetch service session and returns session information.
+
+        :returns: dictionary with an authenticated `proxy_url` for the builder
+        to use, and a `revocation_endpoint` to revoke the token when it's no
+        longer required.
+        """
         timestamp = int(time.time())
         proxy_username = "{build_id}-{timestamp}".format(
-            build_id=self.build.build_cookie, timestamp=timestamp
+            build_id=self.build_id, timestamp=timestamp
         )
-        auth_string = f"{admin_username}:{secret}".strip()
-        auth_header = b"Basic " + base64.b64encode(auth_string.encode("ASCII"))
 
-        session = yield self._worker.process_pool.doWork(
+        session_data = yield self.worker.process_pool.doWork(
             RequestFetchServiceSessionCommand,
-            url=url,
-            auth_header=auth_header,
+            url=self.START_SESSION_ENDPOINT.format(
+                control_endpoint=self.control_endpoint
+            ),
+            auth_header=self.auth_header,
             proxy_username=proxy_username,
         )
-        return session
+
+        self.session_id = session_data["id"]
+        token = session_data["token"]
+
+        proxy_url = self.PROXY_URL.format(
+            session_id=self.session_id,
+            token=token,
+            proxy_endpoint=self.proxy_endpoint,
+        )
+        revocation_endpoint = self.TOKEN_REVOCATION.format(
+            control_endpoint=self.control_endpoint,
+            session_id=self.session_id,
+        )
+
+        return {
+            "proxy_url": proxy_url,
+            "revocation_endpoint": revocation_endpoint,
+        }