launchpad-reviewers team mailing list archive

[Ines Almeida <mp+474874@xxxxxxxxxxxxxxxxxx>]

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

Commit message:
Make store git hostnames configurable via feature flag
    
Setting valid hostnames won't require a deployment from Launchpad with this change. Instead, the `git_repository_url.store_hostnames` feature flag can be set, with the various hostnames separated by spaces. Example 'git_repository_url.store_hostnames default 0 git.staging.snapcraftcontent.com git.staging.pkg.store'

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Continuation from https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/474606
Makes this configurable, particularly relevant if the amount of stores increases beyond snapcraft
-- 
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 8d5caf5..f7a426d 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -1010,13 +1010,17 @@ 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 _get_store_hostnames():
+    """Return the valid hostnames that we accept for the store git
+    repositories (which use a turnip backend).
+
+    Return value configured in the `git_repository_url.store_hostnames` feature
+    flag, or default values.
+    """
+    store_urls = getFeatureFlag("git_repository_url.store_hostnames")
+    if store_urls:
+        return store_urls.split(" ")
+    return ["git.staging.snapcraftcontent.com", "git.staging.pkg.store"]
 
 
 def _fetch_blob_from_store(repository_url, ref_path, filename):
@@ -1171,7 +1175,7 @@ class GitRefRemote(GitRefMixin):
             return _fetch_blob_from_gitlab(
                 self.repository_url, self.path, filename
             )
-        if url.hostname in _store_hostnames:
+        if url.hostname in _get_store_hostnames():
             return _fetch_blob_from_store(
                 self.repository_url, self.path, filename
             )
diff --git a/lib/lp/code/model/tests/test_gitref.py b/lib/lp/code/model/tests/test_gitref.py
index 8e33bbc..da717ce 100644
--- a/lib/lp/code/model/tests/test_gitref.py
+++ b/lib/lp/code/model/tests/test_gitref.py
@@ -521,6 +521,28 @@ class TestGitRefGetBlob(TestCaseWithFactory):
         self.assertEqual(b"foo", ref.getBlob("dir/file"))
 
     @responses.activate
+    def test_remote_store_branch_feature_flag(self):
+        self.useFixture(
+            FeatureFixture(
+                {
+                    "git_repository_url.store_hostnames": "git.test."
+                    "rockcraftcontent.com git.rockcraftcontent.com"
+                }
+            )
+        )
+        url = "https://git.test.rockcraftcontent.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(
diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index 9bd290d..f15c040 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -314,6 +314,14 @@ flag_info = sorted(
             "",
             "",
         ),
+        (
+            "git_repository_url.store_hostnames",
+            "space delimited",
+            "Valid store hostnames to use as remote repositories",
+            "git.staging.snapcraftcontent.com git.staging.pkg.store",
+            "",
+            "",
+        ),
     ]
 )