[AI] OSAC-1927: Resolve tenant storage classes at VM provision when tenant status is empty

[osac-jira-ai-issue-solver]

Resolves OSAC-1927

Summary

• When Tenant.status.storageClasses is empty (storage controller disabled or
  AAP jobs failed), the ComputeInstance controller now falls back to resolving
  labeled StorageClasses directly from the target cluster using the existing
  getTenantStorageClasses() function
• This is a read-only fallback — it does not write back to Tenant status; the
  storage controller remains authoritative when enabled
• When Tenant.status.storageClasses is already populated, behavior is
  unchanged

Test plan

• Unit test: empty status + labeled SCs on cluster → resolved and injected
  into provisioning context
• Unit test: empty status + no labeled SCs → proceeds without storage
  classes (AAP reports error)
• Unit test: populated status → uses status values, ignores cluster SCs
  (regression)
• Existing AAP provider tenant_storage_classes tests still pass
• make lint passes (0 issues)
• make test passes

Additional fixes

Fixed pre-existing test compilation errors in securitygroup_controller_test.go,
subnet_controller_test.go, and virtualnetwork_controller_test.go where
FabricManager field changed from string to *string (proto optional field).

[openshift-ci-robot]

@osac-jira-ai-issue-solver[bot]: This pull request references OSAC-1927 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:

Resolves OSAC-1927

Summary

• When Tenant.status.storageClasses is empty (storage controller disabled or
  AAP jobs failed), the ComputeInstance controller now falls back to resolving
  labeled StorageClasses directly from the target cluster using the existing
  getTenantStorageClasses() function
• This is a read-only fallback — it does not write back to Tenant status; the
  storage controller remains authoritative when enabled
• When Tenant.status.storageClasses is already populated, behavior is
  unchanged

Test plan

• Unit test: empty status + labeled SCs on cluster → resolved and injected
  into provisioning context
• Unit test: empty status + no labeled SCs → proceeds without storage
  classes (AAP reports error)
• Unit test: populated status → uses status values, ignores cluster SCs
  (regression)
• Existing AAP provider tenant_storage_classes tests still pass
• make lint passes (0 issues)
• make test passes

Additional fixes

Fixed pre-existing test compilation errors in securitygroup_controller_test.go,
subnet_controller_test.go, and virtualnetwork_controller_test.go where
FabricManager field changed from string to *string (proto optional field).

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.

[osac-jira-ai-issue-solver]

AI Session Costs

Session	Cost
New ticket	$4.94
Feedback (1) (no changes)	$0.41
Feedback (1) retry 1 (no changes)	$0.45
Feedback (1) retry 2	$1.38
Feedback (2) (no changes)	$0.61
Feedback (2) retry 1 (no changes)	$0.66
Feedback (2) retry 2 (no changes)	$0.76
Feedback (2) retry 3	$0.97
Feedback (3) (no changes)	$0.77
Feedback (3) retry 1 (no changes)	$0.55
Feedback (3) retry 2 (no changes)	$0.47
Feedback (3) retry 3	$1.30
Feedback (4) (no changes)	$0.54
Feedback (4) retry 1 (no changes)	$0.55
Feedback (4) retry 2	$0.99
Feedback (5) (no changes)	$0.69
Feedback (5) retry 1 (no changes)	$0.48
Feedback (5) retry 2 (no changes)	$0.53
Feedback (5) retry 3	$1.25
Feedback (6) (no changes)	$0.57
Feedback (6) retry 1 (no changes)	$0.71
Feedback (6) retry 2 (no changes)	$0.59
Feedback (6) retry 3	$1.70
Feedback (7) (no changes)	$0.71
Feedback (7) retry 1 (no changes)	$0.59
Feedback (7) retry 2 (no changes)	$0.65
Feedback (7) retry 3 (unable)	$0.64
Feedback (8)	$0.70
Feedback (9) (no changes)	$1.14
Feedback (9) retry 1 (no changes)	$0.73
Feedback (9) retry 2 (no changes)	$0.72
Feedback (9) retry 3 (unable)	$0.65
Feedback (10) (no changes)	$0.72
Feedback (10) retry 1	$0.92
Total	$30.04

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:39 PM UTC · Completed 7:51 PM UTC

Commit: d29a146 · View workflow run →

[fullsend-ai-review]

Review

Findings

Low

• [error handling / resilience] osac-operator/internal/controller/computeinstance_controller.go:800 — The fallback path returns a hard error when getTenantStorageClasses fails (e.g., transient network error listing StorageClasses from the target cluster), blocking provisioning entirely. This matches the established pattern in storage_controller.go (which also hard-errors on the same function), and controller-runtime will retry with backoff — so it is not truly fatal. However, the primary path silently proceeds with no storage classes when the status is empty, creating an asymmetry. Consider logging the error and proceeding without storage classes for improved resilience.

• [error-handling idiom] osac-operator/internal/controller/computeinstance_controller.go:800 — The fmt.Errorf wrapping at this error return is the only wrapped error in handleUpdate; all six other error returns in this method use raw return ctrl.Result{}, err. Consider matching the dominant raw-return pattern or wrapping consistently across the method. See also: [error handling / resilience] finding at this location.

• [edge case] osac-operator/internal/controller/computeinstance_controller.go:796 — When tenant.Status.StorageClasses is a non-nil empty slice (length 0), the code enters the fallback path. The ClusterStorageReady condition guard correctly prevents override when the storage controller has reconciled (condition is present), making this safe in practice. Noted as a design assumption — the storage controller currently never sets an empty slice explicitly.

• [scope-creep] osac-operator/internal/controller/securitygroup_controller_test.go:701 — The PR bundles mechanical test fixes for FabricManager string-to-*string type changes in 3 unrelated test files (securitygroup_controller_test.go, subnet_controller_test.go, virtualnetwork_controller_test.go). While harmless and correct (ptr.To() is the established pattern), these are not related to OSAC-1927. Consider splitting into a separate commit for auditability.

Previous run

Review

Findings

Medium

• [pattern-inconsistency] osac-operator/internal/controller/computeinstance_controller.go — The two new Eventf calls that emit eventReasonDuplicateStorageClass use eventActionReconcile as the action, while every existing use of this event reason in the codebase (storage_controller.go lines 331, 386, 556) consistently pairs it with eventActionDetectDuplicate. Using a different action for the same event reason breaks the codebase convention — operators filtering events by reason+action would categorize these differently.
  Remediation: Replace eventActionReconcile with eventActionDetectDuplicate in both Eventf calls.

Low

• [edge-case] osac-operator/internal/controller/computeinstance_controller.go:596 — A narrow race window exists between the tenant becoming Ready and the storage controller's first reconcile. During this window, the ComputeInstance controller may resolve StorageClasses via the fallback path. Once the storage controller sets the ClusterStorageReady condition, the fallback becomes permanently unreachable for that tenant. The window is self-healing and both paths use the same getTenantStorageClasses function, limiting practical impact.

• [error-handling] osac-operator/internal/controller/computeinstance_controller.go:599 — When getTenantStorageClasses returns an error during fallback resolution, the reconciler returns the error immediately (triggering requeue with backoff) but provides no visible signal on the ComputeInstance resource. This is consistent with how other handleUpdate errors are treated in this controller.

• [design-coupling] osac-operator/internal/controller/computeinstance_controller.go:596 — The ComputeInstanceReconciler now inspects the ClusterStorageReady condition owned by the StorageReconciler. If the StorageReconciler's condition lifecycle changes (e.g., always setting it before resolution), the fallback would silently stop triggering. The coupling is documented in the new code comments and in the AGENTS.md update.

Previous run (2)

Review

Findings

Low

• [Event action inconsistency] osac-operator/internal/controller/computeinstance_controller.go:601 — Uses eventActionReconcile for eventReasonDuplicateStorageClass events; the storage controller uses eventActionDetectDuplicate for the same reason. Consistent within this controller's own convention (all ComputeInstance events use eventActionReconcile) but differs from the storage controller.
  Remediation: Define a dedicated event action constant or reuse eventActionDetectDuplicate.

• [architectural-coherence] osac-operator/internal/controller/computeinstance_controller.go:596 — Narrow race window during tenant creation: when StorageReconciler is enabled but hasn't reconciled yet, both controllers may read StorageClasses simultaneously. Self-healing on next reconciliation; inline comments document the behavior.

• [edge-case] osac-operator/internal/controller/computeinstance_controller.go:598 — When fallback resolution returns zero resolved classes and no ambiguous tiers, the reconciler logs but emits no event. Intentional (no labeled StorageClasses is a normal state) but creates a minor observability asymmetry with the ambiguous-tiers path.

• [error-handling-idiom] osac-operator/internal/controller/computeinstance_controller.go:600 — Error wrap message omits the failed to prefix used by other fmt.Errorf calls in this file.
  Remediation: Change to failed to resolve tenant storage classes from target cluster for tenant %s: %w.

• [scope-creep] osac-operator/internal/controller/securitygroup_controller_test.go:701 — FabricManager field type change (string → *string) in 3 test files is a separate concern from OSAC-1927 but explicitly called out in PR body as compilation fixes.

Previous run (3)

Review

Findings

Medium

• [event-constant-convention] osac-operator/internal/controller/computeinstance_controller.go:606 — Every Recorder.Eventf call in computeinstance_controller.go uses eventActionReconcile as the action parameter. The new code uses eventActionDetectDuplicate (defined in tenant_names.go), which belongs to the storage controller's event vocabulary. This mixes event action constants across controller domains and may affect consumers filtering events by action.
  Remediation: Use eventActionReconcile to match all other Eventf calls in this controller, or define a new controller-specific action constant in computeinstance_names.go.

Low

• [edge-case] osac-operator/internal/controller/computeinstance_controller.go:596 — The fallback triggers whenever len(tenant.Status.StorageClasses) == 0, without distinguishing "StorageReconciler is disabled" from "StorageReconciler ran but resolved nothing." When the StorageReconciler is active and found no valid StorageClasses, the ComputeInstance controller redundantly re-runs the same resolution on every reconcile cycle.
  Remediation: Consider checking TenantConditionClusterStorageReady to only fall back when the condition is absent.

• [scope-creep] osac-operator/internal/controller/computeinstance_controller.go:601 — The fallback returns a hard error when getTenantStorageClasses fails (e.g., target cluster unreachable), blocking ComputeInstance reconciliation. Consistent with the storage controller's pattern, but since this fallback targets degraded environments, a softer failure mode may be more appropriate.

• [naming-convention] osac-operator/internal/controller/computeinstance_controller.go:600 — The return value resolution differs from existing call sites (result at storage_controller.go:380, scResult at :550). Minor inconsistency; resolution does avoid shadowing ctrl.Result.

• [logging-idiom] osac-operator/internal/controller/computeinstance_controller.go:612 — log.Error(nil, ...) logs a non-error condition (all tiers ambiguous) at error level with a nil error. All other log.Error calls in this controller pass non-nil error values. Consider log.Info with descriptive text.

• [unreachable-code] osac-operator/internal/controller/storage_tier_resolution.go:112 — Pre-existing: the case 0 branch is unreachable because groupByTier only creates map entries for tiers with at least one StorageClass.

Previous run (4)

Review

Findings

Medium

• [error handling / silent continuation] osac-operator/internal/controller/computeinstance_controller.go:598 — When getTenantStorageClasses returns zero resolved classes but has only ambiguous tiers, the fallback logs "no tenant storage classes resolved" and continues to provisioning without storage classes. The storage controller, by contrast, sets a condition and returns early when all tiers are ambiguous (storage_controller.go lines 334–345). Consider logging at warning level or emitting a distinct event when all tiers are ambiguous and zero classes are resolved, so operators can distinguish "no storage at all" from "storage exists but is misconfigured."

• [event constant consistency] osac-operator/internal/controller/computeinstance_controller.go:603 — The Eventf call for eventReasonDuplicateStorageClass uses eventActionReconcile, but the established pattern in storage_controller.go (lines 331, 386, 556) pairs this reason with eventActionDetectDuplicate. Using the same action string ensures consistent event filtering and monitoring.
  Remediation: Change eventActionReconcile to eventActionDetectDuplicate.

• [protected-path] osac-operator/AGENTS.md — This PR modifies a protected governance file (AGENTS.md). The change documents the new ComputeInstanceReconciler fallback behavior and is consistent with the linked issue (OSAC-1927). Human approval is always required for protected-path changes regardless of context.

Low

• [scope-creep] osac-operator/internal/controller/securitygroup_controller_test.go — The PR bundles compilation fixes for three networking test files (securitygroup_controller_test.go, subnet_controller_test.go, virtualnetwork_controller_test.go) that are unrelated to OSAC-1927. These FabricManager string-to-*string migration fixes are legitimate but are a separate concern from the storage class fallback feature. Ideally split into a separate commit.

Previous run (5)

Review

Findings

Low

• [error handling asymmetry] osac-operator/internal/controller/computeinstance_controller.go:601 — When getTenantStorageClasses returns an error (e.g., target cluster API unreachable), the fallback logs and continues without storage classes. This differs from storage_controller.go which treats the same error as fatal and requeues. If a transient failure occurs during the first provision, the VM may be provisioned without storage classes, and subsequent reconciles will take the kv != nil path — meaning the fallback is never retried for that instance. Consider returning the error or requeueing with a delay for consistency.

• [event action naming convention] osac-operator/internal/controller/computeinstance_controller.go:605 — All other Recorder.Eventf calls in the compute instance controller use eventActionReconcile. The new fallback uses eventActionDetectDuplicate (borrowed from the storage controller), creating an inconsistency in event actions emitted by this controller. Consider using eventActionReconcile for consistency, or document why the storage controller's event action is intentionally reused here.

Previous run (6)

Review

Findings

Low

• [test-adequacy] osac-operator/internal/controller/computeinstance_controller_test.go — The "should exclude ambiguous tiers" test verifies that ambiguous tiers are excluded from resolved storage classes (gotSCs is nil), but does not verify that a DuplicateStorageClass warning event was emitted via the Recorder. The production code emits events for duplicate messages in the fallback path, and this is the only test case where that event is expected to fire.

• [scope-creep] osac-operator/internal/controller/securitygroup_controller_test.go — The PR bundles fixes for pre-existing FabricManager string-to-*string compilation errors in three unrelated networking controller test files. These are not related to OSAC-1927's storage class resolution scope. While the fixes are mechanical and low-risk, they expand the PR's blast radius beyond the authorized issue. The PR description transparently calls this out as "Additional fixes".

• [incomplete-documentation] osac-operator/AGENTS.md:73 — AGENTS.md describes StorageClass discovery as handled by StorageReconciler in storage_controller.go. After this PR, the ComputeInstanceReconciler also performs StorageClass discovery as a fallback when Tenant.status.storageClasses is empty. Consider adding a note about this fallback path.

Previous run (7)

Review

Findings

Low

• [scope-creep] osac-operator/internal/controller/securitygroup_controller_test.go — The PR bundles fixes for pre-existing test compilation errors in three networking test files (FabricManager string → *string). These are legitimate mechanical fixes required for the test binary to compile, but are unrelated to the OSAC-1927 storage class fallback work. Consider splitting into a separate commit for bisectability.

• [error-handling] osac-operator/internal/controller/computeinstance_controller.go:599 — The fallback path treats any error from getTenantStorageClasses as a hard failure, returning the error and requeueing with exponential backoff. A transient failure to list StorageClasses on the target cluster will block VM provisioning entirely until the List succeeds. Since this is already a fallback path (tenant status was empty), consider logging the error and proceeding without storage classes instead.

• [test-adequacy] osac-operator/internal/controller/computeinstance_controller_test.go:1579 — The fallback test creates a StorageClass via the direct API client but the reconciler reads via the manager's cache-backed client with no Eventually/retry around the Reconcile call to account for cache propagation delay. This is an inherited pattern from existing tests, not a regression.

• [comment-style] osac-operator/internal/controller/computeinstance_controller.go:592 — The multi-line block comment uses trailing periods while the file has mixed comment style conventions. Minor inconsistency, not actionable.

Previous run (8)

Review

Findings

Medium

• [error-handling-gap] osac-operator/internal/controller/computeinstance_controller.go:606 — When getTenantStorageClasses encounters multiple StorageClasses for the same tier (ambiguous tier), it returns err == nil with resolution.duplicateMessages populated but resolution.resolved potentially empty. The new fallback code only checks len(resolution.resolved) > 0 and logs a generic "no tenant storage classes resolved" message, silently discarding the ambiguity information. Both existing callers in storage_controller.go explicitly iterate result.duplicateMessages and emit Kubernetes warning events via r.Recorder.Eventf. The ComputeInstanceReconciler has a Recorder field and uses it elsewhere in the same file, so emitting events here would be consistent.
  Remediation: After calling getTenantStorageClasses, iterate resolution.duplicateMessages and emit warning events via r.Recorder.Eventf, consistent with storage_controller.go.

Low

• [test-adequacy] osac-operator/internal/controller/computeinstance_controller_test.go:1555 — The new test suite covers three scenarios (successful fallback, no labeled SCs, status-populated regression) but does not test the ambiguous-tier scenario where multiple StorageClasses share the same tenant+tier labels. A test for this case would document expected behavior and catch regressions if ambiguity handling is improved.
• [architectural-coherence] osac-operator/internal/controller/computeinstance_controller.go:595 — The fallback reuses the shared getTenantStorageClasses() function from storage_tier_resolution.go rather than duplicating logic, which is architecturally sound. The code comments document the intentional fallback for when the storage controller is disabled or its jobs failed. Consider adding a brief note about the overlap with storage_controller.go's ownership to prevent future confusion.
• [scope-alignment] osac-operator/internal/controller/securitygroup_controller_test.go — The PR bundles FabricManager string→*string test fixes alongside the OSAC-1927 feature. These are necessary compilation fixes transparently documented in the PR body.

---

Labels: PR adds storage class fallback resolution in the ComputeInstance controller

[osac-jira-ai-issue-solver]

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

🤖 Finished Review · ✅ Success · Started 7:39 PM UTC · Completed 7:51 PM UTC

Commit: d29a146 · View workflow run →

No action needed — workflow status comment.

Addressed in 3cab0e4.

[osac-jira-ai-issue-solver]

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

Review

Findings

Medium

• [error-handling-gap] osac-operator/internal/controller/computeinstance_controller.go:606 — When getTenantStorageClasses encounters multiple StorageClasses for the same tier (ambiguous tier), it returns err == nil with resolution.duplicateMessages populated but resolution.resolved potentially empty. The new fallback code only checks len(resolution.resolved) > 0 and logs a generic "no tenant storage classes resolved" message, silently discarding the ambiguity information. Both existing callers in storage_controller.go explicitly iterate result.duplicateMessages and emit Kubernetes warning events via r.Recorder.Eventf. The ComputeInstanceReconciler has a Recorder field and uses it elsewhere in the same file, so emitting events here would be consistent.
  Remediation: After calling getTenantStorageClasses, iterate resolution.duplicateMessages and emit warning events via r.Recorder.Eventf, consistent with storage_controller.go.

Low

• [test-adequacy] osac-operator/internal/controller/computeinstance_controller_test.go:1555 — The new test suite covers three scenarios (successful fallback, no labeled SCs, status-populated regression) but does not test the ambiguous-tier scenario where multiple StorageClasses share the same tenant+tier labels. A test for this case would document expected behavior and catch regressions if ambiguity handling is improved.
• [architectural-coherence] osac-operator/internal/controller/computeinstance_controller.go:595 — The fallback reuses the shared getTenantStorageClasses() function from storage_tier_resolution.go rather than duplicating logic, which is architecturally sound. The code comments document the intentional fallback for when the storage controller is disabled or its jobs failed. Consider adding a brief note about the overlap with storage_controller.go's ownership to prevent future confusion.
• [scope-alignment] osac-operator/internal/controller/securitygroup_controller_test.go — The PR bundles FabricManager string→*string test fixes alongside the OSAC-1927 feature. These are necessary compilation fixes transparently documented in the PR body.

---

Labels: PR adds storage class fallback resolution in the ComputeInstance controller

Addressed all findings. [error-handling-gap]: Added iteration over resolution.duplicateMessages with r.Recorder.Eventf warning events, matching the pattern in storage_controller.go. [test-adequacy]: Added a fourth test case covering the ambiguous-tier scenario where two StorageClasses share the same tenant+tier labels — verifies the ambiguous tier is excluded from injection. [architectural-coherence]: The existing comment block already documents the fallback intent and relationship to the storage controller; no additional note needed. [scope-alignment]: Acknowledged — the FabricManager string→*string test fixes were necessary compilation fixes.

Addressed in 3cab0e4.

[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 akshaynadkarni. 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:

• osac-operator/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 7:58 PM UTC · Completed 8:14 PM UTC

Commit: 3cab0e4 · View workflow run →

[osac-jira-ai-issue-solver]

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

🤖 Finished Review · ✅ Success · Started 7:58 PM UTC · Completed 8:14 PM UTC

Commit: 3cab0e4 · View workflow run →

Addressed in 7461750.

[osac-jira-ai-issue-solver]

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

🤖 Review · Started 9:39 PM UTC

Commit: a6629ee · View workflow run →

Review status notification — no action needed.

Addressed in 1e8d4ff.

[osac-jira-ai-issue-solver]

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

See the review comment for full details.

Review comment pointer — no action needed.

Addressed in 1e8d4ff.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:05 PM UTC · Completed 10:22 PM UTC

Commit: 1e8d4ff · View workflow run →

[osac-jira-ai-issue-solver]

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

🤖 Review · Started 10:05 PM UTC

Commit: 1e8d4ff · View workflow run →

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:33 PM UTC · Completed 10:50 PM UTC

Commit: eba9f77 · View workflow run →

[osac-jira-ai-issue-solver]

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

🤖 Finished Review · ✅ Success · Started 10:33 PM UTC · Completed 10:50 PM UTC

Commit: eba9f77 · 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:

See the review comment for full details.

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

[osac-jira-ai-issue-solver]

CI failures addressed in 1c7e9c3.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:58 PM UTC · Completed 12:17 AM UTC

Commit: 1c7e9c3 · View workflow run →

[fullsend-ai-review]

Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:

• osac-operator/internal/controller/computeinstance_controller.go (file-level): Line 800 · [low] error handling / resilience

The fallback path returns a hard error when getTenantStorageClasses fails, blocking provisioning entirely for transient target-cluster connectivity issues. While this matches the storage_controller.go pattern and controller-runtime retries with backoff, the asymmetry with the primary path (which silently proceeds without storage classes) suggests the fallback could also gracefully degrade.

Suggested fix: Log the error and proceed without storage classes instead of returning a hard error.

• osac-operator/internal/controller/computeinstance_controller.go (file-level): Line 800 · [low] error-handling idiom

The fmt.Errorf wrapping at this error return is the only wrapped error in handleUpdate; all six other error returns in this method use raw return ctrl.Result{}, err. This creates an inconsistency within the method.

Suggested fix: Either wrap consistently across handleUpdate error returns or return the raw error to match the dominant pattern.

• osac-operator/internal/controller/computeinstance_controller.go (file-level): Line 796 · [low] edge case

When tenant.Status.StorageClasses is a non-nil empty slice (length 0), the code enters the fallback path. The ClusterStorageReady condition guard correctly prevents override when the storage controller has reconciled, making this safe. Noted as a design assumption.

• osac-operator/internal/controller/securitygroup_controller_test.go:701: [low] scope-creep

The PR bundles mechanical test fixes for FabricManager string-to-*string type changes in 3 unrelated test files. While harmless and correct, these are not related to OSAC-1927.

Suggested fix: Consider splitting the FabricManager ptr.To() test fixes into a separate commit for auditability.