fix: Fix v8.3.0 ETAC Table Rename BED-7077

[mvlipka]

Description

This simply replaces the existing ETAC table rename in the v8.3.0 migration with a much more idempotent alternative.
An error was occurring if the user's database ended up in a weird state where the previous rename occurred, but the original table still existed.

Motivation and Context

Resolves: BED-7077

Why is this change required? What problem does it solve?

How Has This Been Tested?

I ran all the migrations, then created a table called environment_access_control (the old table name)
I then deleted the migrations back to v8.2.0 in order for the v8.3.0 migration to run at the next startup
I then started up the app and the migration successfully completed

Screenshots (optional):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Database Migrations

Checklist:

• I have met the contributing prerequisites
  • Assigned myself to this PR
  • Added the appropriate labels
  • Associated an issue: Issues and Pull Requests README #672
  • Read the Contributing guide: https://github.com/SpecterOps/BloodHound/wiki/Contributing
• I have ensured that related documentation is up-to-date
  • Open API docs
  • Code comments (GoDocs / JSDocs)
• I have followed proper test practices
  • Added/updated tests to cover my changes
  • All new and existing tests passed

Summary by CodeRabbit

• Bug Fixes
  • Migration now renames the legacy access control table only when the source exists and the target does not, preventing errors on repeated runs.
  • Added a defensive cleanup step to drop any leftover legacy table if present, ensuring idempotent, error-free migrations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Replaces an unconditional rename in v8.3.0 with a guarded DO block that renames only if the source exists and the target does not, and adds a defensive conditional drop of environment_access_control in v8.5.0 to remove any leftover legacy table.

Changes

Cohort / File(s)	Summary
Guarded rename migration cmd/api/src/database/migration/migrations/v8.3.0.sql	Replaces an unconditional ALTER TABLE RENAME with a DO $$ BEGIN ... END $$; block that checks pg_tables and performs the rename only if source exists and target does not, making the migration idempotent.
Defensive cleanup cmd/api/src/database/migration/migrations/v8.5.0.sql	Appends a DO $$ BEGIN ... END $$; block that conditionally drops environment_access_control if the legacy environment_targeted_access_control exists, ensuring cleanup without error.

Sequence Diagram(s)

(Skipped — changes are localized SQL migration adjustments with no multi-component control flow.)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: BED-5933 - fix bad migration from 7.4.0 #1990: Modifies the v8.3.0 migration area around environment_targeted_access_control / rename behavior (related change set).

Suggested labels

bug, dbmigration

Suggested reviewers

• bsheth711
• superlinkx
• LawsonWillard

Poem

🐰 I hopped through schema, soft and light,
I checked each table before the flight.
A careful rename, no crash or fright,
and stray old rows tucked out of sight. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly references the specific issue (BED-7077) and summarizes the main change: fixing the ETAC table rename to be idempotent.
Description check	✅ Passed	The description includes all required template sections: a clear problem statement, motivation/context with ticket reference, detailed testing methodology, and completed checklist items confirming prerequisites and testing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch BED-7077/Fix-ETAC-Table-Rename-Idempotency

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2d51c7 and 6d8bb74.

📒 Files selected for processing (1)

• cmd/api/src/database/migration/migrations/v8.5.0.sql

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build BloodHound Container Image / Build and Package Container
• GitHub Check: build-ui
• GitHub Check: run-tests
• GitHub Check: run-analysis

🔇 Additional comments (1)

cmd/api/src/database/migration/migrations/v8.5.0.sql (1)

174-186: The v8.3.0 migration already contains the idempotent rename logic and is not being modified in this PR. Its DO block checks if the old table exists AND the new table does not exist before renaming. The v8.5.0 cleanup is an appropriate defensive safeguard that handles the edge case where both tables exist due to a failed migration.

The complete solution is sound: v8.3.0 handles the rename with proper idempotency, and v8.5.0 adds a secondary cleanup layer to address any leftover tables from incomplete migrations. Both migrations are idempotent and can run multiple times safely.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}