launchpad-reviewers team mailing list archive

[Cristian Gonzalez <cristian.gonzalez@xxxxxxxxxxxxx>]

Cristian Gonzalez has proposed merging ~cristiangsp/launchpad:fixing-git-ref-scan-job-with-oci into launchpad:master.

Commit message:
Fixing GitRefScanJob for OCI git repositories.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cristiangsp/launchpad/+git/launchpad/+merge/398807

GitRefScanJob jobs are executed using the branchscanner user which does not have the needed OCI related database permissions to process a repository that has an OCI project as target.

This MP includes:

- Adding the missing permissions to the branchscanner user in the securify.cfg file.
- A new test case that validates that the GitRefScanJob job can be executed when the repository has an OCI Project as target.


To be discussed:
- The new test is just a slightly different version of the existing `test_triggers_webhooks`, maybe there is a pattern in the codebase to avoid duplication?
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cristiangsp/launchpad:fixing-git-ref-scan-job-with-oci into launchpad:master.

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 4a3770c..f198381 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -715,6 +715,8 @@ public.karmaaction                        = SELECT
 public.message                            = SELECT, INSERT
 public.messagechunk                       = SELECT, INSERT
 public.milestonetag                       = SELECT
+public.ociproject                         = SELECT
+public.ociprojectname                     = SELECT
 public.person                             = SELECT
 public.personsettings                     = SELECT
 public.previewdiff                        = SELECT
diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py
index 18c91c9..1b41b35 100644
--- a/lib/lp/code/model/tests/test_gitjob.py
+++ b/lib/lp/code/model/tests/test_gitjob.py
@@ -222,6 +222,48 @@ class TestGitRefScanJob(TestCaseWithFactory):
                 logger.output, LogsScheduledWebhooks([
                     (hook, "git:push:0.1", payload_matcher)]))
 
+    def test_triggers_webhooks_with_oci_project_as_repository_target(self):
+        # Jobs trigger any relevant webhooks when they're enabled.
+        self.useFixture(FeatureFixture({'code.git.webhooks.enabled': 'on'}))
+        logger = self.useFixture(FakeLogger())
+        oci_project = self.factory.makeOCIProject()
+        repository = self.factory.makeGitRepository(target=oci_project)
+        self.factory.makeGitRefs(
+            repository, paths=['refs/heads/master', 'refs/tags/1.0'])
+        hook = self.factory.makeWebhook(
+            target=repository, event_types=['git:push:0.1'])
+        job = GitRefScanJob.create(repository)
+        paths = ('refs/heads/master', 'refs/tags/2.0')
+        self.useFixture(GitHostingFixture(refs=self.makeFakeRefs(paths)))
+        with dbuser('branchscanner'):
+            JobRunner([job]).runAll()
+        delivery = hook.deliveries.one()
+        sha1 = lambda s: hashlib.sha1(s).hexdigest()
+        payload_matcher = MatchesDict({
+            'git_repository': Equals('/' + repository.unique_name),
+            'git_repository_path': Equals(repository.unique_name),
+            'ref_changes': Equals({
+                'refs/tags/1.0': {
+                    'old': {'commit_sha1': sha1(b'refs/tags/1.0')},
+                    'new': None},
+                'refs/tags/2.0': {
+                    'old': None,
+                    'new': {'commit_sha1': sha1(b'refs/tags/2.0')}},
+            })})
+        self.assertThat(
+            delivery,
+            MatchesStructure(
+                event_type=Equals('git:push:0.1'),
+                payload=payload_matcher))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
+            self.assertThat(
+                logger.output, LogsScheduledWebhooks([
+                    (hook, "git:push:0.1", payload_matcher)]))
+
     def test_merge_detection_triggers_webhooks(self):
         self.useFixture(FeatureFixture(
             {BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG: 'on'}))