From d34b6d707da62807c821d3587424556a42d99677 Mon Sep 17 00:00:00 2001 From: Chris Doehring Date: Mon, 11 May 2026 14:44:38 -0700 Subject: [PATCH 1/6] fix: skip auto-generated UniqueTogetherValidator on nested config serializer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UniqueConstraint(integration, action) added in #406 caused DRF to auto-attach a UniqueTogetherValidator to IntegrationConfigurationCreate UpdateSerializer. That validator forces `integration` and `action` to be required regardless of their declared `required=False`, which broke PATCH /v2/integrations/{id}/ from the portal — the portal omits `integration` on each configuration item because it is implied by the URL. The parent IntegrationCreateUpdateSerializer.update() already injects `integration` from the URL post-validation and uses update_or_create on the configuration id, and the DB-level UniqueConstraint enforces uniqueness on insert. The serializer-level UniqueTogetherValidator is redundant. Adds a regression test mirroring the portal's payload shape ({id, action, data} with no `integration`). Fixes GUNDI-5313 Co-Authored-By: Claude Opus 4.7 (1M context) --- cdip_admin/api/v2/serializers.py | 8 ++++++ .../api/v2/tests/test_integrations_api.py | 28 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/cdip_admin/api/v2/serializers.py b/cdip_admin/api/v2/serializers.py index 8224b696..55c0305f 100644 --- a/cdip_admin/api/v2/serializers.py +++ b/cdip_admin/api/v2/serializers.py @@ -487,6 +487,14 @@ class IntegrationConfigurationCreateUpdateSerializer(serializers.ModelSerializer class Meta: model = IntegrationConfiguration fields = ["id", "integration", "action", "data"] + # The model's UniqueConstraint(fields=["integration", "action"]) makes + # DRF auto-generate a UniqueTogetherValidator, which enforces both + # fields as required regardless of the explicit required=False above. + # The parent IntegrationCreateUpdateSerializer injects `integration` + # from the URL after validation, and `update_or_create(id=...)` + # plus the DB-level UniqueConstraint enforce uniqueness, so the + # serializer-level check is redundant. + validators = [] class IntegrationCreateUpdateSerializer(serializers.ModelSerializer): diff --git a/cdip_admin/api/v2/tests/test_integrations_api.py b/cdip_admin/api/v2/tests/test_integrations_api.py index e195b9a5..87f7395b 100644 --- a/cdip_admin/api/v2/tests/test_integrations_api.py +++ b/cdip_admin/api/v2/tests/test_integrations_api.py @@ -1499,6 +1499,34 @@ def test_update_or_create_integration_config_as_org_admin( ) +def test_patch_integration_config_with_portal_payload_shape( + api_client, org_admin_user, organization, provider_lotek_panthera, + lotek_action_auth, +): + # The portal omits `integration` from each configuration item (it is + # implied by the URL). Regression test for the UniqueTogetherValidator + # auto-generated from the model's UniqueConstraint(integration, action) + # rejecting the request with `{"integration": ["This field is required."]}`. + lotek_auth_config = lotek_action_auth.configurations_by_action.get(integration=provider_lotek_panthera) + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + { + "id": str(lotek_auth_config.id), + "action": str(lotek_action_auth.id), + "data": {"username": "user@lotek.com", "password": "NewPassword"}, + }, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_200_OK, response.json() + lotek_auth_config.refresh_from_db() + assert lotek_auth_config.data == {"username": "user@lotek.com", "password": "NewPassword"} + + def _test_get_integration_api_key( api_client, user, integration ): From 7a9b5bd73323f30aed941576ba6ebef580926f38 Mon Sep 17 00:00:00 2001 From: Chris Doehring Date: Mon, 11 May 2026 15:16:24 -0700 Subject: [PATCH 2/6] fix: catch duplicate-action configs at validation, not at IntegrityError Addresses PR feedback. With the auto-generated UniqueTogetherValidator disabled (previous commit), the (integration, action) UniqueConstraint could be hit at insert time and surface as an unhandled 500. Add explicit duplicate checks in IntegrationCreateUpdateSerializer.validate(): 1. On PATCH, a new (no-id) configuration item for an action that already has a config raises 400 with a clear message ("A configuration for action X already exists. Include 'id' in the payload to update it."). 2. Two configuration items in the same request targeting the same action raises 400 with "Duplicate configurations for action X in the request." Co-Authored-By: Claude Opus 4.7 (1M context) --- cdip_admin/api/v2/serializers.py | 19 ++++++++ .../api/v2/tests/test_integrations_api.py | 46 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/cdip_admin/api/v2/serializers.py b/cdip_admin/api/v2/serializers.py index 55c0305f..a1cb710c 100644 --- a/cdip_admin/api/v2/serializers.py +++ b/cdip_admin/api/v2/serializers.py @@ -525,6 +525,7 @@ def validate(self, data): """ Validate the configurations """ + seen_action_ids = set() for configuration in data.get("configurations", []): if self.instance and "id" in configuration: # Integration Update action = IntegrationConfiguration.objects.get(id=configuration["id"]).action @@ -532,6 +533,24 @@ def validate(self, data): if "action" not in configuration: raise drf_exceptions.ValidationError("The action id is required.") action = configuration["action"] + # On PATCH, a new (no-id) item for an action that already + # has a configuration would collide with the (integration, + # action) UniqueConstraint at the DB layer and surface as + # a 500. Surface it as a friendly 400 here. The validator- + # level UniqueTogetherValidator that would normally catch + # this is disabled in IntegrationConfigurationCreateUpdate + # Serializer.Meta because it incorrectly required the + # `integration` field that the URL implies. + if self.instance and self.instance.configurations.filter(action=action).exists(): + raise drf_exceptions.ValidationError( + f"A configuration for action '{action.value}' already exists on this " + f"integration. Include 'id' in the payload to update it." + ) + if action.id in seen_action_ids: + raise drf_exceptions.ValidationError( + f"Duplicate configurations for action '{action.value}' in the request." + ) + seen_action_ids.add(action.id) configuration_schema = action.schema if configuration_schema and not configuration: # Blank or None raise drf_exceptions.ValidationError("The configuration can't be null or empty") diff --git a/cdip_admin/api/v2/tests/test_integrations_api.py b/cdip_admin/api/v2/tests/test_integrations_api.py index 87f7395b..180e3992 100644 --- a/cdip_admin/api/v2/tests/test_integrations_api.py +++ b/cdip_admin/api/v2/tests/test_integrations_api.py @@ -1527,6 +1527,52 @@ def test_patch_integration_config_with_portal_payload_shape( assert lotek_auth_config.data == {"username": "user@lotek.com", "password": "NewPassword"} +def test_patch_integration_config_rejects_new_item_for_existing_action( + api_client, org_admin_user, organization, provider_lotek_panthera, + lotek_action_auth, +): + # A PATCH that adds a new (no-id) configuration for an action that + # already has a config would otherwise hit the (integration, action) + # DB UniqueConstraint mid-update and surface as a 500. Should be a 400. + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + { + "action": str(lotek_action_auth.id), + "data": {"username": "x", "password": "y"}, + }, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "already exists" in str(response.json()) + + +def test_patch_integration_config_rejects_duplicate_actions_in_payload( + api_client, org_admin_user, organization, provider_lotek_panthera, + lotek_action_auth, lotek_action_list_devices, +): + # Two items targeting the same action in one request would silently + # last-write-wins (in the natural-key sense) or hit IntegrityError. + # Should be a 400. + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + {"action": str(lotek_action_list_devices.id), "data": {"group_id": "1"}}, + {"action": str(lotek_action_list_devices.id), "data": {"group_id": "2"}}, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "Duplicate" in str(response.json()) + + def _test_get_integration_api_key( api_client, user, integration ): From bc0b2c6518f1f0cfd0daa7f1ee5bd6a5a58bf15a Mon Sep 17 00:00:00 2001 From: Chris Doehring Date: Mon, 11 May 2026 18:01:34 -0700 Subject: [PATCH 3/6] fix: address PR #412 review comments (scope config-id lookup; freeze action on id-update; tighten test assertions) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Copilot review comments on #412: 1. `IntegrationConfiguration.objects.get(id=...)` in validate() wasn't scoped to the integration being patched. A nonexistent id surfaced as a 500 from DoesNotExist, and an id belonging to a different integration was accepted and later repointed via update_or_create's defaults. Scope the lookup to `self.instance.configurations` and raise a clean 400 with "Configuration '...' was not found on this integration." 2. `update()` passed the full `config_data` (including any payload `action`) as `defaults` to `update_or_create`. When `id` was present that could repoint the row's action — colliding with the (integration, action) UniqueConstraint and bypassing the schema validation in validate(). Pop `action` from defaults when `id` is present so id-updates can only change `data`. 3. Tests asserted on `str(response.json())` — brittle. Assert against `response.json()["non_field_errors"][0]` instead. Added three regression tests: - unknown config id → 400 (not 500) - id belonging to a different integration → 400, original config untouched - payload `action` alongside `id` is ignored (action stays put) Co-Authored-By: Claude Opus 4.7 (1M context) --- cdip_admin/api/v2/serializers.py | 20 ++++- .../api/v2/tests/test_integrations_api.py | 81 ++++++++++++++++++- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/cdip_admin/api/v2/serializers.py b/cdip_admin/api/v2/serializers.py index a1cb710c..e0a22626 100644 --- a/cdip_admin/api/v2/serializers.py +++ b/cdip_admin/api/v2/serializers.py @@ -528,7 +528,17 @@ def validate(self, data): seen_action_ids = set() for configuration in data.get("configurations", []): if self.instance and "id" in configuration: # Integration Update - action = IntegrationConfiguration.objects.get(id=configuration["id"]).action + # Scope the lookup to this integration's configurations so + # an id from a different integration doesn't get repointed + # (and a missing id surfaces as a 400, not a 500 from + # DoesNotExist). + try: + existing_config = self.instance.configurations.get(id=configuration["id"]) + except IntegrationConfiguration.DoesNotExist: + raise drf_exceptions.ValidationError( + f"Configuration '{configuration['id']}' was not found on this integration." + ) + action = existing_config.action else: # Create a new integration or new config if "action" not in configuration: raise drf_exceptions.ValidationError("The action id is required.") @@ -597,6 +607,14 @@ def update(self, instance, validated_data): # Update or Create nested configurations if provided for config_data in configurations: # Usually less than 5-10 configs config_data["integration"] = self.instance + if config_data.get("id"): + # When updating by id, the row's `action` is immutable — + # repointing it would (a) collide with the (integration, + # action) UniqueConstraint, and (b) bypass the schema + # validation in validate() which keyed off the existing + # action. The portal sometimes echoes `action` back for + # client convenience; ignore it on id-updates. + config_data.pop("action", None) IntegrationConfiguration.objects.update_or_create( id=config_data.get("id"), defaults=config_data diff --git a/cdip_admin/api/v2/tests/test_integrations_api.py b/cdip_admin/api/v2/tests/test_integrations_api.py index 180e3992..ac0b6fc6 100644 --- a/cdip_admin/api/v2/tests/test_integrations_api.py +++ b/cdip_admin/api/v2/tests/test_integrations_api.py @@ -1548,7 +1548,7 @@ def test_patch_integration_config_rejects_new_item_for_existing_action( format="json", ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "already exists" in str(response.json()) + assert "already exists" in response.json()["non_field_errors"][0] def test_patch_integration_config_rejects_duplicate_actions_in_payload( @@ -1570,7 +1570,84 @@ def test_patch_integration_config_rejects_duplicate_actions_in_payload( format="json", ) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert "Duplicate" in str(response.json()) + assert "Duplicate" in response.json()["non_field_errors"][0] + + +def test_patch_integration_config_rejects_unknown_config_id( + api_client, org_admin_user, organization, provider_lotek_panthera, +): + # A PATCH referencing a configuration id that doesn't exist on this + # integration should return 400, not 500 from DoesNotExist. + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + { + "id": "00000000-0000-0000-0000-000000000000", + "data": {"username": "x", "password": "y"}, + }, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "not found" in response.json()["non_field_errors"][0] + + +def test_patch_integration_config_rejects_id_from_different_integration( + api_client, org_admin_user, organization, provider_lotek_panthera, provider_ats, +): + # A PATCH referencing a configuration id that belongs to a different + # integration must not repoint that row — it should 400 cleanly. + other_config = provider_ats.configurations.first() + assert other_config is not None + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + {"id": str(other_config.id), "data": {"hijacked": True}}, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "not found" in response.json()["non_field_errors"][0] + # The other integration's config must remain untouched. + other_config.refresh_from_db() + assert other_config.integration_id == provider_ats.id + assert other_config.data != {"hijacked": True} + + +def test_patch_integration_config_ignores_action_on_id_update( + api_client, org_admin_user, organization, provider_lotek_panthera, + lotek_action_auth, lotek_action_pull_positions, +): + # The portal sometimes echoes back `action` alongside `id` for client + # convenience. The row's action must not be repointed via update_or_create + # defaults — that would collide with the (integration, action) constraint. + lotek_auth_config = lotek_action_auth.configurations_by_action.get(integration=provider_lotek_panthera) + api_client.force_authenticate(org_admin_user) + response = api_client.patch( + reverse("integrations-detail", kwargs={"pk": provider_lotek_panthera.id}), + data={ + "configurations": [ + { + "id": str(lotek_auth_config.id), + # Try to repoint at a different action — must be ignored. + "action": str(lotek_action_pull_positions.id), + "data": {"username": "u", "password": "p"}, + }, + ], + }, + format="json", + ) + assert response.status_code == status.HTTP_200_OK, response.json() + lotek_auth_config.refresh_from_db() + # The row's action stayed put; only data updated. + assert lotek_auth_config.action_id == lotek_action_auth.id + assert lotek_auth_config.data == {"username": "u", "password": "p"} def _test_get_integration_api_key( From cc227718a3959c1b9afcdec2f4e50191527471b7 Mon Sep 17 00:00:00 2001 From: Anderson Dario Date: Wed, 13 May 2026 13:33:11 -0300 Subject: [PATCH 4/6] fix(DASOPS-2057): use serca-artifact-registry and ArgoCD deploys on release branch (#419) --- .github/workflows/main.yml | 53 +++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2b105d49..2bc4abdc 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,38 +19,43 @@ jobs: - id: vars run: | echo "tag=${{ github.head_ref || github.ref_name }}-$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT - echo "repository=us-central1-docker.pkg.dev/cdip-78ca/gundi/admin-portal" >> $GITHUB_OUTPUT + echo "repository=europe-west3-docker.pkg.dev/serca-artifact-registry/gundi/admin-portal" >> $GITHUB_OUTPUT build: - uses: PADAS/gundi-workflows/.github/workflows/build_docker.yml@v7 + uses: PADAS/gundi-workflows/.github/workflows/build_docker.yml@v11 needs: vars with: - environment: stage + workload_identity_provider: "projects/563316706574/locations/global/workloadIdentityPools/github-actions/providers/github-oidc" # pragma: allowlist secret repository: ${{ needs.vars.outputs.repository }} tag: ${{ needs.vars.outputs.tag }} + build-args: BASE_REGISTRY=europe-west3-docker.pkg.dev/serca-artifact-registry/virtual-docker/ deploy_dev: - uses: PADAS/gundi-workflows/.github/workflows/deploy_k8s.yml@v7 + # gundi-dev is reconciled by ArgoCD. This job bumps the image tag in + # PADAS/serca-argocd-apps:gundi/admin-portal/values-gundi-dev.yaml and lets + # ArgoCD's automated selfHeal pick up the change. + uses: PADAS/gundi-workflows/.github/workflows/deploy_argocd.yml@v8 if: startsWith(github.ref, 'refs/heads/main') needs: [vars, build] with: - environment: dev - chart_name: admin-portal - chart_version: '1.3.6' - repository: ${{ needs.vars.outputs.repository }} - tag: ${{ needs.vars.outputs.tag }} + app-name: admin-portal + environment: gundi-dev + image-repository: ${{ needs.vars.outputs.repository }} + image-tag: ${{ needs.vars.outputs.tag }} secrets: inherit deploy_stage: - uses: PADAS/gundi-workflows/.github/workflows/deploy_k8s.yml@v7 + # gundi-stage is reconciled by ArgoCD. This job bumps the image tag in + # PADAS/serca-argocd-apps:gundi/admin-portal/values-gundi-stage.yaml and lets + # ArgoCD's automated selfHeal pick up the change. + uses: PADAS/gundi-workflows/.github/workflows/deploy_argocd.yml@v8 if: startsWith(github.ref, 'refs/heads/release') needs: [vars, build] with: - environment: stage - chart_name: admin-portal - chart_version: '1.3.6' - repository: ${{ needs.vars.outputs.repository }} - tag: ${{ needs.vars.outputs.tag }} + app-name: admin-portal + environment: gundi-stage + image-repository: ${{ needs.vars.outputs.repository }} + image-tag: ${{ needs.vars.outputs.tag }} secrets: inherit deploy_dev_legacy: @@ -60,7 +65,7 @@ jobs: needs: [vars, build] strategy: matrix: - deployment: + deployment: - cdip-portal - celery-beat - celery-deployments-worker @@ -76,15 +81,17 @@ jobs: secrets: inherit deploy_prod: - uses: PADAS/gundi-workflows/.github/workflows/deploy_k8s.yml@v7 + # gundi-prod is reconciled by ArgoCD. This job bumps the image tag in + # PADAS/serca-argocd-apps:gundi/admin-portal/values-gundi-prod.yaml and lets + # ArgoCD's automated selfHeal pick up the change. + uses: PADAS/gundi-workflows/.github/workflows/deploy_argocd.yml@v8 if: startsWith(github.ref, 'refs/heads/release') needs: [vars, build, deploy_stage] with: - environment: prod - chart_name: admin-portal - chart_version: '1.3.6' - repository: ${{ needs.vars.outputs.repository }} - tag: ${{ needs.vars.outputs.tag }} + app-name: admin-portal + environment: gundi-prod + image-repository: ${{ needs.vars.outputs.repository }} + image-tag: ${{ needs.vars.outputs.tag }} secrets: inherit deploy_prod_legacy: @@ -93,7 +100,7 @@ jobs: needs: [vars, build, deploy_stage] strategy: matrix: - deployment: + deployment: - cdip-portal - celery-beat - celery-deployments-worker From ea8fcb35bd0a39e71036acea2a90d4ed117122c6 Mon Sep 17 00:00:00 2001 From: Anderson Dario Date: Wed, 13 May 2026 13:42:07 -0300 Subject: [PATCH 5/6] fix(DASOPS-2057): add approve_prod gate before prod deploy (#420) --- .github/workflows/main.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2bc4abdc..0db4b1c8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -80,13 +80,21 @@ jobs: namespace: cdip-v1 secrets: inherit + approve_prod: + runs-on: ubuntu-latest + environment: prod + if: startsWith(github.ref, 'refs/heads/release') + needs: [vars, build, deploy_stage] + steps: + - run: echo "Approved for prod" + deploy_prod: # gundi-prod is reconciled by ArgoCD. This job bumps the image tag in # PADAS/serca-argocd-apps:gundi/admin-portal/values-gundi-prod.yaml and lets # ArgoCD's automated selfHeal pick up the change. uses: PADAS/gundi-workflows/.github/workflows/deploy_argocd.yml@v8 if: startsWith(github.ref, 'refs/heads/release') - needs: [vars, build, deploy_stage] + needs: [vars, build, deploy_stage, approve_prod] with: app-name: admin-portal environment: gundi-prod @@ -97,7 +105,7 @@ jobs: deploy_prod_legacy: uses: PADAS/gundi-workflows/.github/workflows/update_k8s_image.yml@v7 if: startsWith(github.ref, 'refs/heads/release') - needs: [vars, build, deploy_stage] + needs: [vars, build, deploy_stage, approve_prod] strategy: matrix: deployment: From 6cfa5c0bd738cc7963bcb0961eade28225256392 Mon Sep 17 00:00:00 2001 From: Anderson Dario Date: Wed, 13 May 2026 15:08:55 -0300 Subject: [PATCH 6/6] =?UTF-8?q?ci(DASOPS-2057):=20fix=20deploy=5Fprod=5Fle?= =?UTF-8?q?gacy=20=E2=80=94=20use=20direct=20GKE=20credentials=20[release-?= =?UTF-8?q?20260508]=20(#422)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v12 uses direct gcloud container clusters get-credentials instead of Fleet Connect Gateway, fixing the deploy_prod_legacy failure caused by cdip-prod01 not being registered in the serca-tools Fleet. --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0db4b1c8..e050a713 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -103,7 +103,7 @@ jobs: secrets: inherit deploy_prod_legacy: - uses: PADAS/gundi-workflows/.github/workflows/update_k8s_image.yml@v7 + uses: PADAS/gundi-workflows/.github/workflows/update_k8s_image.yml@v12 if: startsWith(github.ref, 'refs/heads/release') needs: [vars, build, deploy_stage, approve_prod] strategy: