[AI] OSAC-4213: InstanceType: empty name allowed — latent primary-key corruption

[osac-jira-ai-issue-solver]

Resolves OSAC-4213

Summary

• Add database-level CHECK (name <> '') constraint to the instance_types table via new migration 102, matching the pattern already used by disk_images (migration 99) and bare_metal_instance_types (migration 88)
• Include a pre-migration safety check that fails loudly if any existing rows have an empty name, preventing silent data issues
• Add migration tests covering constraint enforcement, normal inserts, and the pre-existing bad data safety check

Context

The instance_types table uses name-as-primary-key (id = metadata.name), so an empty name would produce an empty primary key. While the proto-level validation (Metadata.name min_len: 1) prevents this via the normal API path, the database itself had no defense-in-depth constraint. This migration closes that gap.

The archived table is intentionally excluded from the constraint, consistent with disk_images scope.

Test plan

• Verify migration 102 applies cleanly on a fresh database
• Verify INSERT INTO instance_types with an empty name is rejected with instance_types_name_not_empty violation
• Verify INSERT INTO instance_types with a valid name succeeds
• Verify migration fails with a clear error if pre-existing empty-name rows exist
• Run ginkgo run -r internal to confirm no regressions

🤖 Generated with Claude Code

[osac-jira-ai-issue-solver]

AI Session Costs

Session	Cost
New ticket	$1.71
Feedback (1) (no changes)	$0.91
Feedback (2) (no changes)	$0.26
Feedback (2) retry 1 (no changes)	$0.31
Feedback (2) retry 2 (no changes)	$0.35
Feedback (2) retry 3 (unable)	$0.39
Feedback (3) (no changes)	$19.02
Feedback (3) retry 1	$14.91
Total	$37.86

[openshift-ci-robot]

@osac-jira-ai-issue-solver[bot]: This pull request references OSAC-4213 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.1.0" version, but no target version was set.

Details

In response to this:

Resolves OSAC-4213

Summary

• Add database-level CHECK (name <> '') constraint to the instance_types table via new migration 102, matching the pattern already used by disk_images (migration 99) and bare_metal_instance_types (migration 88)
• Include a pre-migration safety check that fails loudly if any existing rows have an empty name, preventing silent data issues
• Add migration tests covering constraint enforcement, normal inserts, and the pre-existing bad data safety check

Context

The instance_types table uses name-as-primary-key (id = metadata.name), so an empty name would produce an empty primary key. While the proto-level validation (Metadata.name min_len: 1) prevents this via the normal API path, the database itself had no defense-in-depth constraint. This migration closes that gap.

The archived table is intentionally excluded from the constraint, consistent with disk_images scope.

Test plan

• Verify migration 102 applies cleanly on a fresh database
• Verify INSERT INTO instance_types with an empty name is rejected with instance_types_name_not_empty violation
• Verify INSERT INTO instance_types with a valid name succeeds
• Verify migration fails with a clear error if pre-existing empty-name rows exist
• Run ginkgo run -r internal to confirm no regressions

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:32 AM UTC · Completed 7:45 AM UTC

Commit: 49f677b · View workflow run →

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Low

• [edge-case] fulfillment-service/internal/database/migrations/102_add_instance_types_name_check.up.sql:29 — The migration adds CHECK (name <> '') but does not remove the contradictory column default of '' inherited from migration 51 (name text not null default ''). After this migration, any raw SQL INSERT that omits the name column will hit the CHECK constraint rather than receiving a useful "not-null without default" error. While the application's protobuf validation (Metadata.name min_len: 1) prevents this path in normal operation, the contradictory default is misleading for schema inspection and ad-hoc queries. Consider adding ALTER TABLE instance_types ALTER COLUMN name DROP DEFAULT inside the DO block after the ADD CONSTRAINT statement.

Previous run (2)

Looks good to me

---

Labels: PR adds a database constraint to prevent empty primary keys in instance_types — this is a bug fix for a latent data integrity issue

[osac-jira-ai-issue-solver]

In comment, @fullsend-ai-review[bot] said:

🤖 Finished Review · ✅ Success · Started 7:32 AM UTC · Completed 7:45 AM UTC

Commit: 49f677b · View workflow run →

I was unable to produce code changes to address this comment after multiple attempts.

[osac-jira-ai-issue-solver]

In comment, @fullsend-ai-review[bot] said:

Looks good to me

---

Labels: PR adds a database constraint to prevent empty primary keys in instance_types — this is a bug fix for a latent data integrity issue

I was unable to produce code changes to address this comment after multiple attempts.

[osac-jira-ai-issue-solver]

CI failures addressed in 40ce53d.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: osac-jira-ai-issue-solver[bot]
Once this PR has been reviewed and has the lgtm label, please ask for approval from ygalblum. 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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:29 AM UTC · Completed 10:42 AM UTC

Commit: 40ce53d · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:06 AM UTC · Completed 11:20 AM UTC

Commit: e0c985b · View workflow run →