launchpad-reviewers team mailing list archive

[Ines Almeida <mp+474606@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:fix-get-blob-from-store-git into launchpad:master.

Commit message:
Allow snapcraft store git remote URLs
    
Currently we restrict which repositories we can fetch files from - this enables the *craft stores with turnip backend as a source of *craft.yaml files.
Currntly this adds a hardcoded list of URLs that can be used, to be refactored to become more extensible


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

All tests from `TestGitRefGetBlob` (including the newly added ones) passed.

We hardcode the store URLs similarly to how we handle the gitlab case, but we should definitely refactor it to make it more extensible. For now I just hardcoded the 2 staging URLs mentioned by the store folks.

There is also a chance for refactoring in the sense that I mostly copied the code from Launchpad.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:fix-get-blob-from-store-git into launchpad:master.

diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index 9790e04..8d5caf5 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -1010,6 +1010,44 @@ def _fetch_blob_from_launchpad(repository_url, ref_path, filename):
     return response.content
 
 
+# XXX ines-almeida 2024-10-07: Needs refactoring to allow extending more easily
+# especially since this list will for sure grow as we allow non-staging stores.
+# Potentially look into using a feature rule.
+_store_hostnames = {
+    "git.staging.snapcraftcontent.com",
+    "git.staging.pkg.store",
+}
+
+
+def _fetch_blob_from_store(repository_url, ref_path, filename):
+    """Fetch blob from a *craft store repo.
+
+    The *craft stores use a turnip backend, similar to Launchpad.
+    """
+    url = urlsplit(repository_url)
+    repo_path = url.path.strip("/")
+    try:
+        response = urlfetch(
+            "https://%s/%s/plain/%s";
+            % (url.hostname, repo_path, quote(filename)),
+            params={"h": ref_path},
+        )
+    except requests.RequestException as e:
+        if (
+            e.response is not None
+            and e.response.status_code == requests.codes.NOT_FOUND
+        ):
+            raise GitRepositoryBlobNotFound(
+                repository_url, filename, rev=ref_path
+            )
+        else:
+            raise GitRepositoryScanFault(
+                "Failed to get file from Git repository at %s: %s"
+                % (repository_url, str(e))
+            )
+    return response.content
+
+
 @implementer(IGitRef, IGitRefRemote)
 @provider(IGitRefRemoteSet)
 class GitRefRemote(GitRefMixin):
@@ -1133,6 +1171,10 @@ class GitRefRemote(GitRefMixin):
             return _fetch_blob_from_gitlab(
                 self.repository_url, self.path, filename
             )
+        if url.hostname in _store_hostnames:
+            return _fetch_blob_from_store(
+                self.repository_url, self.path, filename
+            )
         if (
             url.hostname == "git.launchpad.net"
             and config.vhost.mainsite.hostname != "launchpad.net"
@@ -1146,6 +1188,7 @@ class GitRefRemote(GitRefMixin):
             return _fetch_blob_from_launchpad(
                 self.repository_url, self.path, filename
             )
+
         codehosting_host = urlsplit(config.codehosting.git_anon_root).hostname
         if url.hostname == codehosting_host:
             repository = getUtility(IGitLookup).getByUrl(self.repository_url)
diff --git a/lib/lp/code/model/tests/test_gitref.py b/lib/lp/code/model/tests/test_gitref.py
index 4ba5916..8e33bbc 100644
--- a/lib/lp/code/model/tests/test_gitref.py
+++ b/lib/lp/code/model/tests/test_gitref.py
@@ -507,6 +507,62 @@ class TestGitRefGetBlob(TestCaseWithFactory):
         self.assertRaises(GitRepositoryScanFault, ref.getBlob, "dir/file")
 
     @responses.activate
+    def test_remote_store_branch(self):
+        url = "https://git.staging.snapcraftcontent.com/ubuntu/public/test";
+        ref = self.factory.makeGitRefRemote(
+            repository_url=url,
+            path="refs/heads/path",
+        )
+        responses.add(
+            "GET",
+            url + "/plain/dir/file?h=refs%2Fheads%2Fpath",
+            body=b"foo",
+        )
+        self.assertEqual(b"foo", ref.getBlob("dir/file"))
+
+    @responses.activate
+    def test_remote_store_HEAD(self):
+        url = "https://git.staging.snapcraftcontent.com/ubuntu/public/test";
+        ref = self.factory.makeGitRefRemote(
+            repository_url=url,
+            path="HEAD",
+        )
+        responses.add(
+            "GET",
+            url + "/plain/dir/file?h=HEAD",
+            body=b"foo",
+        )
+        self.assertEqual(b"foo", ref.getBlob("dir/file"))
+
+    @responses.activate
+    def test_remote_store_404(self):
+        url = "https://git.staging.snapcraftcontent.com/ubuntu/public/test";
+        ref = self.factory.makeGitRefRemote(
+            repository_url=url,
+            path="HEAD",
+        )
+        responses.add(
+            "GET",
+            url + "/plain/dir/file?h=HEAD",
+            status=404,
+        )
+        self.assertRaises(GitRepositoryBlobNotFound, ref.getBlob, "dir/file")
+
+    @responses.activate
+    def test_remote_store_error(self):
+        url = "https://git.staging.snapcraftcontent.com/ubuntu/public/test";
+        ref = self.factory.makeGitRefRemote(
+            repository_url=url,
+            path="HEAD",
+        )
+        responses.add(
+            "GET",
+            url + "/plain/dir/file?h=HEAD",
+            status=500,
+        )
+        self.assertRaises(GitRepositoryScanFault, ref.getBlob, "dir/file")
+
+    @responses.activate
     def test_remote_github_branch(self):
         ref = self.factory.makeGitRefRemote(
             repository_url="https://github.com/owner/name";,