feat: allow config for multi-apps (#421)

These changes introduce `OwnerInstallationNameToUseForTask` model. This model allows an `Owner` to configure a `installation_name` to be used for a given `task_name`. This can only be configured manually for now. That is on purpose as we want to test these changes first. context: codecov/engineering-team#1146 > ⚠️ **These changes include a migration** > > Migration creates table for `OwnerInstallationNameToUseForTask`. > Should not be disruptive when deployed. Co-authored-by: Trent Schmidt <trent@codecov.io>
codecov · Mar 4, 2024 · bd7cf76 · bd7cf76
1 parent ab757ca
commit bd7cf76
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 0 deletions.
diff --git a/codecov_auth/migrations/0053_ownerinstallationnametousefortask_and_more.py b/codecov_auth/migrations/0053_ownerinstallationnametousefortask_and_more.py
@@ -0,0 +1,63 @@
+# Generated by Django 4.2.7 on 2024-02-21 16:03
+
+import uuid
+
+import django.db.models.deletion
+import django_prometheus.models
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("codecov_auth", "0052_githubappinstallation_app_id_and_more"),
+    ]
+
+    # BEGIN;
+    #     --
+    #     -- Create model OwnerInstallationNameToUseForTask
+    #     --
+    #     CREATE TABLE "codecov_auth_ownerinstallationnametousefortask" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "external_id" uuid NOT NULL, "created_at" timestamp with time zone NOT NULL, "updated_at" timestamp with time zone NOT NULL, "installation_name" text NOT NULL, "task_name" text NOT NULL, "owner_id" integer NOT NULL);
+    #     --
+    #     -- Create constraint single_task_name_per_owner on model ownerinstallationnametousefortask
+    #     --
+    #     CREATE UNIQUE INDEX "single_task_name_per_owner" ON "codecov_auth_ownerinstallationnametousefortask" ("owner_id", "task_name");
+    #     ALTER TABLE "codecov_auth_ownerinstallationnametousefortask" ADD CONSTRAINT "codecov_auth_ownerin_owner_id_8bf0ce9b_fk_owners_ow" FOREIGN KEY ("owner_id") REFERENCES "owners" ("ownerid") DEFERRABLE INITIALLY DEFERRED;
+    #     CREATE INDEX "codecov_auth_ownerinstalla_owner_id_8bf0ce9b" ON "codecov_auth_ownerinstallationnametousefortask" ("owner_id");
+    # COMMIT;
+
+    operations = [
+        migrations.CreateModel(
+            name="OwnerInstallationNameToUseForTask",
+            fields=[
+                ("id", models.BigAutoField(primary_key=True, serialize=False)),
+                ("external_id", models.UUIDField(default=uuid.uuid4, editable=False)),
+                ("created_at", models.DateTimeField(auto_now_add=True)),
+                ("updated_at", models.DateTimeField(auto_now=True)),
+                ("installation_name", models.TextField()),
+                ("task_name", models.TextField()),
+                (
+                    "owner",
+                    models.ForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="installation_name_to_use_for_tasks",
+                        to="codecov_auth.owner",
+                    ),
+                ),
+            ],
+            bases=(
+                django_prometheus.models.ExportModelOperationsMixin(
+                    "codecov_auth.github_app_installation"
+                ),
+                models.Model,
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="ownerinstallationnametousefortask",
+            constraint=models.UniqueConstraint(
+                models.F("owner_id"),
+                models.F("task_name"),
+                name="single_task_name_per_owner",
+            ),
+        ),
+    ]
diff --git a/codecov_auth/models.py b/codecov_auth/models.py
@@ -508,6 +508,13 @@ class GithubAppInstallation(
         related_name="github_app_installations",
     )
 
+    def is_configured(self) -> bool:
+        """Returns whether this installation is properly configured and can be used"""
+        if self.name == GITHUB_APP_INSTALLATION_DEFAULT_NAME:
+            # The default app is configured in the installation YAML
+            return True
+        return self.app_id is not None and self.pem_path is not None
+
     def repository_queryset(self) -> BaseManager[Repository]:
         """Returns a QuerySet of repositories covered by this installation"""
         if self.repository_service_ids is None:
@@ -527,6 +534,28 @@ def is_repo_covered_by_integration(self, repo: Repository) -> bool:
         return repo.service_id in self.repository_service_ids
 
 
+class OwnerInstallationNameToUseForTask(
+    ExportModelOperationsMixin("codecov_auth.github_app_installation"), BaseCodecovModel
+):
+    owner = models.ForeignKey(
+        Owner,
+        null=False,
+        on_delete=models.CASCADE,
+        blank=False,
+        related_name="installation_name_to_use_for_tasks",
+    )
+    installation_name = models.TextField(null=False, blank=False)
+    task_name = models.TextField(null=False, blank=False)
+
+    class Meta:
+        constraints = [
+            # Only 1 app name per task per owner_id
+            models.UniqueConstraint(
+                "owner_id", "task_name", name="single_task_name_per_owner"
+            )
+        ]
+
+
 class SentryUser(
     ExportModelOperationsMixin("codecov_auth.sentry_user"), BaseCodecovModel
 ):

diff --git a/codecov_auth/tests/unit/test_models.py b/codecov_auth/tests/unit/test_models.py
@@ -515,3 +515,30 @@ def test_covers_some_repos(self):
         assert list(owner.github_app_installations.all()) == [installation_obj]
         assert installation_obj.repository_queryset().exists()
         assert list(installation_obj.repository_queryset().all()) == [repo]
+
+    def test_is_configured(self):
+        owner = OwnerFactory()
+        installation_default = GithubAppInstallation(
+            owner=owner, repository_service_ids=None, installation_id=100
+        )
+        installation_configured = GithubAppInstallation(
+            owner=owner,
+            repository_service_ids=None,
+            name="my_installation",
+            installation_id=100,
+            app_id=123,
+            pem_path="some_path",
+        )
+        installation_not_configured = GithubAppInstallation(
+            owner=owner,
+            repository_service_ids=None,
+            installation_id=100,
+            name="my_other_installation",
+            app_id=1234,
+        )
+        installation_default.save()
+        installation_configured.save()
+        installation_not_configured.save()
+        assert installation_default.is_configured() == True
+        assert installation_configured.is_configured() == True
+        assert installation_not_configured.is_configured() == False
diff --git a/webhook_handlers/tests/test_github.py b/webhook_handlers/tests/test_github.py
@@ -638,6 +638,7 @@ def test_installation_creates_new_owner_if_dne(self, mock_refresh):
                     "id": 4,
                     "repository_selection": "selected",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -661,6 +662,7 @@ def test_installation_creates_new_owner_if_dne(self, mock_refresh):
         assert ghapp_installations_set.count() == 1
         installation = ghapp_installations_set.first()
         assert installation.installation_id == 4
+        assert installation.app_id == 15
         assert installation.repository_service_ids == ["12321", "12343"]
 
         assert mock_refresh.call_count == 1
@@ -691,6 +693,7 @@ def test_installation_creates_new_owner_if_dne_all_repos(self):
                     "id": 4,
                     "repository_selection": "all",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -730,6 +733,7 @@ def test_installation_repositories_creates_new_owner_if_dne(self):
                     "id": 4,
                     "repository_selection": "all",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repository_selection": "all",
                 "sender": {"type": "User"},
@@ -772,6 +776,7 @@ def test_installation_update_repos_existing_ghapp_installation(self):
                     "id": 4,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "repo1", "node_id": "R_node1"},
@@ -824,6 +829,7 @@ def test_installation_with_deleted_action_nulls_values(self):
                     "id": 25,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -883,6 +889,7 @@ def test_installation_repositories_update_existing_ghapp(self):
                     "id": installation.installation_id,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories_added": [
                     {"id": repo2.service_id, "node_id": "R_xDOGxCAT"}
@@ -935,6 +942,7 @@ def test_installation_repositories_update_existing_ghapp_all_repos(self):
                     "id": 12,
                     "repository_selection": "all",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories_added": [{"id": repo2.service_id}],
                 "repositories_removed": [],
@@ -968,6 +976,7 @@ def test_installation_with_other_actions_sets_owner_itegration_id_if_none(
                     "id": installation_id,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -1010,6 +1019,7 @@ def test_installation_repositories_with_other_actions_sets_owner_itegration_id_i
                     "id": installation_id,
                     "repository_selection": "all",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repository_selection": "all",
                 "action": "added",
@@ -1040,6 +1050,7 @@ def test_installation_trigger_refresh_with_other_actions(self, refresh_mock):
                     "id": 11,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "action": "added",
                 "sender": {"type": "User"},

diff --git a/webhook_handlers/tests/test_github_enterprise.py b/webhook_handlers/tests/test_github_enterprise.py
@@ -476,6 +476,7 @@ def test_installation_creates_new_owner_if_dne(self):
                     "id": 4,
                     "repository_selection": "selected",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -517,6 +518,7 @@ def test_installation_creates_new_owner_if_dne_all_repos(self):
                     "id": 4,
                     "repository_selection": "all",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -558,6 +560,7 @@ def test_installation_repositories_creates_new_owner_if_dne(self):
                     "id": 4,
                     "repository_selection": "all",
                     "account": {"id": service_id, "login": username},
+                    "app_id": 15,
                 },
                 "repository_selection": "all",
                 "sender": {"type": "User"},
@@ -614,6 +617,7 @@ def test_installation_with_deleted_action_nulls_values(self):
                     "id": 25,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_kgDOG2tZYQ"},
@@ -671,6 +675,7 @@ def test_installation_repositories_update_existing_ghapp(self):
                     "id": 12,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories_added": [{"id": repo2.service_id, "node_id": "R_repo2"}],
                 "repositories_removed": [
@@ -720,6 +725,7 @@ def test_installation_repositories_update_existing_ghapp_all_repos(self):
                     "id": 12,
                     "repository_selection": "all",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories_added": [{"id": repo2.service_id, "node_id": "R_repo2"}],
                 "repositories_removed": [],
@@ -753,6 +759,7 @@ def test_installation_with_other_actions_sets_owner_itegration_id_if_none(
                     "id": installation_id,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repositories": [
                     {"id": "12321", "node_id": "R_12321CAT"},
@@ -795,6 +802,7 @@ def test_installation_repositories_with_other_actions_sets_owner_itegration_id_i
                     "id": installation_id,
                     "repository_selection": "all",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "repository_selection": "all",
                 "action": "added",
@@ -825,6 +833,7 @@ def test_installation_trigger_refresh_with_other_actions(self, refresh_mock):
                     "id": 11,
                     "repository_selection": "selected",
                     "account": {"id": owner.service_id, "login": owner.username},
+                    "app_id": 15,
                 },
                 "action": "added",
                 "sender": {"type": "User"},

diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py
@@ -422,6 +422,10 @@ def _handle_installation_repository_events(self, request, *args, **kwargs):
         ghapp_installation, _ = GithubAppInstallation.objects.get_or_create(
             installation_id=installation_id, owner=owner
         )
+        app_id = request.data["installation"]["app_id"]
+        # Either update or set
+        # But this value shouldn't change for the installation, so doesn't matter
+        ghapp_installation.app_id = app_id
 
         all_repos_affected = request.data.get("repository_selection") == "all"
         if all_repos_affected:
@@ -481,6 +485,10 @@ def _handle_installation_events(
                 ghapp_installation, _ = GithubAppInstallation.objects.get_or_create(
                     installation_id=installation_id, owner=owner
                 )
+                app_id = request.data["installation"]["app_id"]
+                # Either update or set
+                # But this value shouldn't change for the installation, so doesn't matter
+                ghapp_installation.app_id = app_id
                 affects_all_repositories = (
                     request.data["installation"]["repository_selection"] == "all"
                 )