OSAC-3704: Add regression tests for VirtualNetwork delete protection with typed references

[clobrano]

Summary

Adds regression test coverage to migration 90's test suite for the VirtualNetwork delete-protection trigger fix.

• Verifies check_virtual_network_not_in_use() correctly blocks VirtualNetwork deletion when Subnets reference it using typed reference format
• Verifies the same for SecurityGroups and NATGateways
• Follows the pattern from migration 55's tests, adapted for typed references

Context

Migration 90 fixed a JSON path mismatch in the check_virtual_network_not_in_use() trigger that allowed VirtualNetworks to be deleted while children still referenced them (OSAC-3704). The fix updated the path from data->'spec'->>'virtual_network' to data->'spec'->'virtual_network'->>'id' to handle typed reference objects. The migration's test suite verified backfill behavior but lacked tests for the trigger protection itself.

Test plan

• All three new tests pass locally
• Existing migration 90 tests still pass
• CI passes

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added integration coverage for preventing the soft deletion of virtual networks still referenced by subnets, security groups, or NAT gateways.
  • Improved validation that deletion errors identify the affected network and referencing resource type, providing clearer feedback when deletion is blocked.

[openshift-ci-robot]

@clobrano: This pull request references OSAC-3704 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Adds regression test coverage to migration 90's test suite for the VirtualNetwork delete-protection trigger fix.

• Verifies check_virtual_network_not_in_use() correctly blocks VirtualNetwork deletion when Subnets reference it using typed reference format
• Verifies the same for SecurityGroups and NATGateways
• Follows the pattern from migration 55's tests, adapted for typed references

Context

Migration 90 fixed a JSON path mismatch in the check_virtual_network_not_in_use() trigger that allowed VirtualNetworks to be deleted while children still referenced them (OSAC-3704). The fix updated the path from data->'spec'->>'virtual_network' to data->'spec'->'virtual_network'->>'id' to handle typed reference objects. The migration's test suite verified backfill behavior but lacked tests for the trigger protection itself.

Test plan

• All three new tests pass locally
• Existing migration 90 tests still pass
• CI passes

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: clobrano
Once this PR has been reviewed and has the lgtm label, please assign crystalchun for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• fulfillment-service/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e5bbcd2-95d0-4733-8f2b-0fcca2ac9c4c

📥 Commits

Reviewing files that changed from the base of the PR and between 508f78c and d31211d.

📒 Files selected for processing (1)

• fulfillment-service/internal/database/migrations/90_update_triggers_for_typed_references_test.go

---

Walkthrough

Migration 90 gains integration tests for typed subnet, security group, and NAT gateway references. Each test verifies that soft-deleting the referenced virtual network fails with PostgreSQL error Z0003 and identifies the related resources.

Changes

Typed reference trigger tests

Layer / File(s)	Summary
Reference error validation fulfillment-service/internal/database/migrations/90_update_triggers_for_typed_references_test.go	Added error inspection imports and tests for typed subnet, security group, and NAT gateway references. Each test checks error Z0003 and validates the referenced network and resource type.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d3121

This change adds localized regression coverage for typed VirtualNetwork references; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the regression tests for VirtualNetwork delete protection with typed references.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets	✅ Passed	The added migration test code contains only synthetic IDs, JSON fixtures, SQL, and PostgreSQL error checks; scans found no credential assignments, secret-shaped blobs, private keys, or credential U...
No-Weak-Crypto	✅ Passed	The PR changes only migration tests for PostgreSQL errors and typed references; the added code contains no weak-crypto algorithm, custom crypto, or secret/token comparison.
No-Injection-Vectors	✅ Passed	The only changed file adds Go tests. SQL uses fixed literals and a $1 JSON parameter; no string concatenation, shell execution, eval/exec, unsafe YAML, pickle, or HTML sink was introduced.
Container-Privileges	✅ Passed	The commit changes only one Go integration-test file; its diff adds no container or Kubernetes manifest and no privileged, host namespace, SYS_ADMIN, or privilege-escalation setting.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds only database tests. It introduces no logging calls, credentials, PII, hostnames, or customer data; asserted resource IDs are synthetic fixtures.
Ai-Attribution	✅ Passed	The PR mentions Claude Code, and its sole commit has an Assisted-by: Claude Code trailer with no Co-Authored-By trailer.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:13 AM UTC · Completed 7:23 AM UTC

Commit: d31211d · View workflow run →

[fullsend-ai-review]

Looks good to me