Skip to content

chore: drop the accessmanagement schema and its dead delegation readers - #3870

Open
howieandersen wants to merge 2 commits into
mainfrom
chore/3655_drop_accessmanagement_schema
Open

howieandersen wants to merge 2 commits into
mainfrom
chore/3655_drop_accessmanagement_schema

Conversation

@howieandersen

Copy link
Copy Markdown
Contributor

Adds Yuniql version v0.19-accessmanagement with a single script that removes the last remains of the old accessmanagement schema:

This is a Yuniql version and not an EF migration like the earlier DropArchiveSchema, because the ordering matters: EF migrations run before Yuniql on a fresh database, and a normally running app runs only Yuniql. An EF drop would therefore be undone by Yuniql recreating the schema on every fresh database. A Yuniql version sequences after the versions that created the objects, so fresh and already-migrated environments end up in the same state. The archive schema never had this problem since it was not created by any repo migration.

All statements are idempotent, so environments where any of the objects are already gone migrate cleanly.

Verification:

  • LegacyApiFixture-backed consent suite (full Yuniql replay on a fresh test database): 18/18 green, and __yuniql_schema_version records v0.19-accessmanagement as Successful right after v0.18
  • EF-only enduser suite (AuthorizedPartiesControllerTest): 19/19 green
  • Direct psql checks on the migrated test database: the schema, both objects in it, all six functions and the FK constraint are gone, while delegation.resourceregistrydelegationchanges and its resourceid_fk column remain
  • Applying the script a second time completes with skip notices and no errors

Deploy note: this assumes the code removal from #3859 is deployed first. A pod running a build older than #3859 would get errors from the resource mirroring if this migration lands in the same rollout.

Related to #3655 (the delegation schema drop stays gated on #3654). Part of #3342.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a new Yuniql migration version (v0.19-accessmanagement) to remove the remaining legacy accessmanagement database schema and related dead delegation reader functions/constraints, ensuring fresh and already-migrated environments converge to the same final state.

Changes:

  • Adds a Yuniql script to drop six legacy delegation.* functions that depended on accessmanagement.resource.
  • Removes the legacy FK resourceregistrydelegationchanges_resourceid_fk from delegation.resourceregistrydelegationchanges.
  • Drops the accessmanagement schema (CASCADE), removing remaining objects like resource and related routines.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/apps/Altinn.AccessManagement/src/Altinn.AccessManagement.Persistence/Migration/v0.19-accessmanagement/01-drop-accessmanagement-schema.sql:2

  • DROP FUNCTION IF EXISTS ... (delegation.delegationchangetype, ...) is not fully idempotent: if the delegation.delegationchangetype enum (or even the delegation schema) has already been removed in an environment, PostgreSQL errors while resolving the argument type before it can apply IF EXISTS. Dropping by name (without a signature) avoids referencing a potentially-missing type and still works here since there are no overloads of this function name in the repo migrations.
DROP FUNCTION IF EXISTS delegation.insert_resourceregistrydelegationchange(delegation.delegationchangetype, text, int, int, int, int, int, text, text, timestamp with time zone);

@jonkjetiloye jonkjetiloye left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vi må vel ha en Release (av både AM og RR) som deployes helt til Prod før denne, så mulig denne bør ventes med å merges inntil man har kontroll på hvilken release det er som skal deployes neste gang.

Er tatt med drop av funksjoner knytt til Delegation schema her men ikke selve schemaet, men dette antar jeg nok uansett er så stort at man ikke ønsker å kjøre drop av med migreringsscript, men noe som må/bør tas manuelt.

@howieandersen

Copy link
Copy Markdown
Contributor Author

Enig i å vente. Status per i dag: siste prod-deploy er 1d6745b fra 3. juli og siste tt02-deploy er a43af9a fra 22. juli (produksjonsfrysen gjennom ferien), så #3859 er ikke ute i noen av miljøene ennå. I tillegg brakk #3648 tfvars-filene for prod og tt02 (fikses i #3897), så første AM-deploy etter frysen må ha den inne for at Terraform-steget skal gå gjennom.

Forslag til rekkefølge når frysen oppheves: merge #3897, deploy AM og RR til tt02 og prod slik at #3859 kommer helt ut, og først deretter merger vi denne.

Og enig om delegation-skjemaet: dette scriptet dropper bare de døde funksjonene, selve skjemadroppet tar vi manuelt som egen DBA-operasjon når tiden kommer.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants