launchpad-reviewers team mailing list archive

[Ines Almeida <mp+463119@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master with ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service as a prerequisite.

Commit message:
End proxy session and gather data when using the fetch service

We send an API request to the fetch service to end the proxy session, save the data from the response locally, and upload it as a build file

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/463119
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad-buildd:close-session-for-fetch-service into launchpad-buildd:master.

diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index fdaf5ca..a5d9232 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -12,7 +12,11 @@ from twisted.python.compat import intToBytes
 from twisted.web import http, proxy
 from zope.interface import implementer
 
-from lpbuildd.util import RevokeProxyTokenError, revoke_proxy_token
+from lpbuildd.util import (
+    RevokeProxyTokenError,
+    end_fetch_service_session,
+    revoke_proxy_token,
+)
 
 
 class BuilderProxyClient(proxy.ProxyClient):
@@ -262,3 +266,20 @@ class BuildManagerProxyMixin:
             )
         except RevokeProxyTokenError as e:
             self._builder.log(f"{e}\n")
+
+    def endProxySession(self, output_file):
+        """ When using the fetch service, we want to end the session at the end
+        of a build and save the metadata from the session.
+
+        This file can be uploaded to the librarian as any other build file.
+        """
+        if not self._use_fetch_service:
+            return
+
+        response = end_fetch_service_session(
+            self.proxy_url,
+            self.end_session_endpoint,
+        )
+        
+        with self.backend.open(output_file, "wb") as f:
+            f.write(response.content)
diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py
index c5b3205..3033ced 100644
--- a/lpbuildd/snap.py
+++ b/lpbuildd/snap.py
@@ -41,6 +41,7 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
         self.use_fetch_service = extra_args.get("use_fetch_service")
         self.proxy_url = extra_args.get("proxy_url")
         self.revocation_endpoint = extra_args.get("revocation_endpoint")
+        self.end_session_endpoint = extra_args.get("end_session_endpoint")
         self.build_source_tarball = extra_args.get(
             "build_source_tarball", False
         )
@@ -153,3 +154,9 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
             )
             if self.backend.path_exists(source_tarball_path):
                 self.addWaitingFileFromBackend(source_tarball_path)
+
+        if self.use_fetch_service:
+            session_file = os.path.join(output_path, "session-data.json")
+            self.endProxySession(output_file=session_file)
+            if self.backend.path_exists(session_file):
+                self.addWaitingFileFromBackend(session_file)
diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py
index a31934a..dfd4b1c 100644
--- a/lpbuildd/tests/test_snap.py
+++ b/lpbuildd/tests/test_snap.py
@@ -3,6 +3,7 @@
 
 import base64
 import os
+from unittest import mock
 
 import responses
 from fixtures import EnvironmentVariable, TempDir
@@ -18,6 +19,7 @@ from lpbuildd.tests.fakebuilder import FakeBuilder
 from lpbuildd.tests.matchers import HasWaitingFiles
 
 
+
 class MockBuildManager(SnapBuildManager):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -906,3 +908,36 @@ class TestSnapBuildManagerIteration(TestCase):
             f"http://fetch-service.example/{session_id}/token";, request.url
         )
     
+    @responses.activate
+    def test_endProxySession(self):
+        session_id = "123"
+        token = "test_token"
+        metadata = {"test": "data"}
+        output_file = "output_file.json"
+
+        responses.add(
+            "DELETE",
+            f"http://fetch-service.example/{session_id}";,
+            json=metadata,
+        )
+
+        self.buildmanager.backend = mock.MagicMock()
+        self.buildmanager.use_fetch_service = True
+        self.buildmanager.end_session_endpoint = (
+            f"http://fetch-service.example/{session_id}";
+        )
+        self.buildmanager.proxy_url = f"http://session_id:{token}@proxy.example";
+
+        self.buildmanager.endProxySession(output_file)
+
+        self.assertEqual(1, len(responses.calls))
+        request = responses.calls[0].request
+        self.assertEqual(f"Basic {token}", request.headers["Authorization"])
+        self.assertEqual(
+            f"http://fetch-service.example/{session_id}";, request.url
+        )
+        self.assertEqual(1, self.buildmanager.backend.open.call_count)
+        self.assertEqual(
+            mock.call(output_file, 'wb'),
+            self.buildmanager.backend.open.call_args
+        )
diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py
index 26e631a..2e9d8ac 100644
--- a/lpbuildd/tests/test_util.py
+++ b/lpbuildd/tests/test_util.py
@@ -7,6 +7,7 @@ import responses
 from testtools import TestCase
 
 from lpbuildd.util import (
+    end_fetch_service_session,
     get_arch_bits,
     revoke_proxy_token,
     set_personality,
@@ -105,3 +106,43 @@ class TestRevokeToken(TestCase):
         self.assertEqual(
             f"Basic {token}", response.request.headers["Authorization"]
         )
+
+    @responses.activate
+    def test_revoke_fetch_service_token(self):
+        """Proxy token revocation for the fetch service, uses the right
+        authentication and returns metadata """
+
+        token = "test-token"
+        proxy_url = f"https://session_id:{token}@host:port";
+        end_session_endpoint = "https://builder.api.endpoint/session_id";
+
+        responses.add(responses.DELETE, end_session_endpoint)
+
+        response = end_fetch_service_session(
+            proxy_url,
+            end_session_endpoint,
+        )
+        self.assertEqual(
+            f"Basic {token}", response.request.headers["Authorization"]
+        )
+
+    @responses.activate
+    def test_end_fetch_service_session(self):
+        """Proxy token revocation for the fetch service, uses the right
+        authentication and returns metadata """
+
+        token = "test-token"
+        proxy_url = f"https://session_id:{token}@host:port";
+        end_session_endpoint = "https://builder.api.endpoint/session_id";
+        metadata = {"test": "data"}
+
+        responses.add(responses.DELETE, end_session_endpoint, json=metadata)
+
+        response = end_fetch_service_session(
+            proxy_url,
+            end_session_endpoint,
+        )
+        self.assertEqual(
+            f"Basic {token}", response.request.headers["Authorization"]
+        )
+        self.assertEqual(metadata, response.json())
diff --git a/lpbuildd/util.py b/lpbuildd/util.py
index e3188b1..4f2ff2a 100644
--- a/lpbuildd/util.py
+++ b/lpbuildd/util.py
@@ -69,6 +69,16 @@ class RevokeProxyTokenError(Exception):
         return f"Unable to revoke token for {self.username}: {self.exception}"
 
 
+class EndProxySessionError(Exception):
+    def __init__(self, session, exception):
+        super().__init__(self)
+        self.session = session
+        self.exception = exception
+
+    def __str__(self):
+        return f"Unable to end session for {self.session}: {self.exception}"
+
+
 def revoke_proxy_token(proxy_url, revocation_endpoint, use_fetch_service=False):
     """Revoke builder proxy token.
 
@@ -103,3 +113,28 @@ def revoke_proxy_token(proxy_url, revocation_endpoint, use_fetch_service=False):
         )
     except requests.RequestException as e:
         raise RevokeProxyTokenError(url.username, e)
+
+
+def end_fetch_service_session(proxy_url, revocation_endpoint):
+    """End fetch service session.
+
+    The proxy_url for the fetch service:
+    http://{session_id}:{token}@{host}:{port}
+
+    We use the token for authentication.
+
+    :raises EndProxySessionError: if attempting to end the session failed.
+    :returns metadata from the fetch service session
+    """
+    url = urlparse(proxy_url)
+    token = url.password
+
+    try:
+        return requests.delete(
+            revocation_endpoint,
+            headers={'Authorization': f'Basic {token}'},
+            timeout=15,
+        )
+    except requests.RequestException as e:
+        session = url.username
+        raise EndProxySessionError(session, e)