launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+493139@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-svt-handler-feature-flags into launchpad:master.

Commit message:
Add handler import/export specific feature flags
    
To enable a handler to be used by `lp.vulnerabilities.importData()` and
`lp.vulnerabilities.exportData()` we will need to add the `handler.name`
to `bugs.vulnerability_import_handler.enabled` and
`bugs.vulnerability_export_handler.enabled` respectively.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/493139
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-svt-handler-feature-flags into launchpad:master.

diff --git a/lib/lp/bugs/model/tests/test_vulnerability.py b/lib/lp/bugs/model/tests/test_vulnerability.py
index 5d7492e..631d104 100644
--- a/lib/lp/bugs/model/tests/test_vulnerability.py
+++ b/lib/lp/bugs/model/tests/test_vulnerability.py
@@ -37,7 +37,8 @@ from lp.bugs.interfaces.vulnerabilityjob import (
 from lp.bugs.model.exportvulnerabilityjob import ExportVulnerabilityJob
 from lp.bugs.model.importvulnerabilityjob import ImportVulnerabilityJob
 from lp.bugs.model.vulnerability import (
-    VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG,
+    VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG,
+    VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG,
     UnauthorizedVulnerabilityHandler,
 )
 from lp.bugs.model.vulnerabilitysubscription import VulnerabilitySubscription
@@ -680,7 +681,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         method.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -720,7 +723,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         method and the UCT handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "UCT Handler"
+        )
 
         repo = self.factory.makeGitRepository(
             owner=self.team, information_type=InformationType.USERDATA
@@ -764,9 +769,12 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         # All parameters None, feature flag is the first check
         self.assertRaisesWithContent(
             NotImplementedError,
-            "Vulnerability import API is currently disabled",
+            "Vulnerability import API is currently disabled for "
+            f"{self.handler.name}",
             getUtility(IVulnerabilitySet).importData,
-            *(None,) * 7,
+            None,
+            self.handler,
+            *(None,) * 5,
         )
 
     def test_importData_git_repo_unauthorized(self):
@@ -774,7 +782,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         repository that is not visible for that user.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -804,7 +814,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         not authorized to use the handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss")
 
@@ -835,7 +847,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         information_type is public but the git repository is private.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -869,7 +883,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         not in the git repository.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -903,7 +919,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         later.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -946,7 +964,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         if there is already a peding one for the same handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -988,7 +1008,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         if the existing job is suspended for the same handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1035,7 +1057,9 @@ class TestVulnerabilitySetImportData(TestCaseWithFactory):
         task's soft_time_limit
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1089,7 +1113,9 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         method.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1112,10 +1138,11 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         # All parameters None, feature flag is the first check
         self.assertRaisesWithContent(
             NotImplementedError,
-            "Vulnerability export API is currently disabled",
+            "Vulnerability export API is currently disabled for "
+            f"{self.handler.name}",
             getUtility(IVulnerabilitySet).exportData,
             self.requester,
-            None,
+            self.handler,
             None,
         )
 
@@ -1124,7 +1151,9 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         not authorized to use the handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss")
 
@@ -1142,7 +1171,9 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         if there is already a peding one for the same handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1167,7 +1198,9 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         a suspended one for the same handler.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1197,7 +1230,9 @@ class TestVulnerabilitySetExportData(TestCaseWithFactory):
         task's soft_time_limit
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         self.factory.makeDistribution(name="soss", owner=self.requester)
 
@@ -1263,7 +1298,9 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         webservice.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         repo = self.factory.makeGitRepository(owner=self.team)
         repo_url = api_url(repo)
@@ -1317,7 +1354,8 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         )
         self.assertEqual(500, response.status)
         self.assertEqual(
-            "Vulnerability import API is currently disabled",
+            "Vulnerability import API is currently disabled for "
+            f"{self.handler.name}",
             response.body.decode().split("\n")[0],
         )
 
@@ -1326,7 +1364,9 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         webservice when the git repository is not visible by the user.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         owner = self.factory.makePerson()
         repo = self.factory.makeGitRepository(
@@ -1382,7 +1422,9 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         webservice.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         webservice = webservice_for_person(
             self.requester,
@@ -1410,7 +1452,8 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         )
         self.assertEqual(500, response.status)
         self.assertEqual(
-            "Vulnerability export API is currently disabled",
+            "Vulnerability export API is currently disabled for "
+            f"{self.handler.name}",
             response.body.decode().split("\n")[0],
         )
 
@@ -1419,7 +1462,9 @@ class TestVulnerabilitySetWebService(TestCaseWithFactory):
         webservice.
         """
         self.useContext(feature_flags())
-        set_feature_flag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG, "true")
+        set_feature_flag(
+            VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG, "SOSS"
+        )
 
         user_unauthorized = self.factory.makePerson()
 
diff --git a/lib/lp/bugs/model/vulnerability.py b/lib/lp/bugs/model/vulnerability.py
index 11f5dde..6503b8b 100644
--- a/lib/lp/bugs/model/vulnerability.py
+++ b/lib/lp/bugs/model/vulnerability.py
@@ -67,8 +67,11 @@ from lp.services.database.stormexpr import Array, ArrayAgg, ArrayIntersects
 from lp.services.features import getFeatureFlag
 from lp.services.xref.interfaces import IXRefSet
 
-VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG = (
-    "bugs.vulnerability_import_export.enabled"
+VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG = (
+    "bugs.vulnerability_import_handler.enabled"
+)
+VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG = (
+    "bugs.vulnerability_export_handler.enabled"
 )
 
 HANDLER_DISTRIBUTION_MAP = {
@@ -416,9 +419,16 @@ class VulnerabilitySet:
     ):
         """See `IVulnerabilitySet`."""
 
-        if not getFeatureFlag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG):
+        feature_flag = (
+            getFeatureFlag(VULNERABILITY_IMPORT_HANDLER_ENABLED_FEATURE_FLAG)
+            or ""
+        )
+
+        enabled_handlers = [i.strip() for i in feature_flag.split()]
+        if handler.name not in enabled_handlers:
             raise NotImplementedError(
-                "Vulnerability import API is currently disabled"
+                "Vulnerability import API is currently disabled for "
+                f"{handler.name}"
             )
 
         # Launchpad's object permissions automatically check requester's
@@ -485,9 +495,16 @@ class VulnerabilitySet:
     ):
         """See `IVulnerabilitySet`."""
 
-        if not getFeatureFlag(VULNERABILITY_IMPORT_ENABLED_FEATURE_FLAG):
+        feature_flag = (
+            getFeatureFlag(VULNERABILITY_EXPORT_HANDLER_ENABLED_FEATURE_FLAG)
+            or ""
+        )
+
+        enabled_handlers = [i.strip() for i in feature_flag.split()]
+        if handler.name not in enabled_handlers:
             raise NotImplementedError(
-                "Vulnerability export API is currently disabled"
+                f"Vulnerability export API is currently disabled for "
+                f"{handler.name}"
             )
 
         # Check requester's permissions to handler
diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
index c4472d8..da9d12b 100644
--- a/lib/lp/services/features/flags.py
+++ b/lib/lp/services/features/flags.py
@@ -331,10 +331,19 @@ flag_info = sorted(
             "",
         ),
         (
-            "bugs.vulnerability_import_export.enabled",
+            "bugs.vulnerability_import_handler.enabled",
             "boolean",
             "If true, users with the right permissions can trigger "
-            "vulnerability data imports and exports via API",
+            "vulnerability data imports for this handler via API",
+            "",
+            "",
+            "",
+        ),
+        (
+            "bugs.vulnerability_export_handler.enabled",
+            "boolean",
+            "If true, users with the right permissions can trigger "
+            "vulnerability data exports for this handler via API",
             "",
             "",
             "",