OSAC-1992: wire Tier API into storage controller for validation and AAP extra_vars

[wgordon17]

Summary

• Wires a new Tier API gRPC client into the storage controller to query defined storage tiers, validate that resolved StorageClasses cover every tier (missing tiers reported via a Warning Event + Tenant condition, not a hard failure), and pass tier definitions plus backend connection details through to AAP via extra_vars on both the tenant create and delete paths.
• Absorbs OSAC-1957's outstanding "operator passes backend connection details to AAP" acceptance criterion, since neither an open nor a merged PR currently delivers it.
• Depends on a follow-on osac-aap PR to read the new storage_tier_definitions/storage_backend_connections extra_vars keys instead of the static STORAGE_TIERS Secret — that PR must land after this one.

Summary by CodeRabbit

• New Features
  • Inject storage tier definitions and backend connection details into provisioning/deprovisioning context and automation event extra-vars (QoS limits, quota, endpoint, credentials).
  • Enhanced tier-to-StorageClass validation with explicit handling for ambiguous tier matches.
• Bug Fixes
  • Improved readiness messaging with clearer warnings for missing tiers and continued reconciliation behavior when tier/backend data is partially unavailable.
  • Ensured storage finalizers are removed during deletion even when tier resolution fails (non-retryable errors included).
• Tests
  • Expanded coverage for tier/backend resolution, warning/event behavior, extra-vars injection, and delete-path scenarios.

[openshift-ci-robot]

@wgordon17: This pull request references OSAC-1992 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 story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• Wires a new Tier API gRPC client into the storage controller to query defined storage tiers, validate that resolved StorageClasses cover every tier (missing tiers reported via a Warning Event + Tenant condition, not a hard failure), and pass tier definitions plus backend connection details through to AAP via extra_vars on both the tenant create and delete paths.
• Absorbs OSAC-1957's outstanding "operator passes backend connection details to AAP" acceptance criterion, since neither an open nor a merged PR currently delivers it.
• Depends on a follow-on osac-aap PR to read the new storage_tier_definitions/storage_backend_connections extra_vars keys instead of the static STORAGE_TIERS Secret — that PR must land after this one.

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.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The storage controller optionally resolves Tier and Backend data through fulfillment gRPC clients, injects the results into provisioning and deprovisioning contexts, reports missing or ambiguous StorageClasses, and serializes the data into AAP event extra variables.

Changes

Storage tier propagation

Layer / File(s)	Summary
Provisioning context and AAP payload pkg/provisioning/extra_vars_context.go, pkg/provisioning/aap_provider.go, pkg/provisioning/*_test.go	Adds context models and accessors for tier definitions and backend connections, then serializes them into AAP event extra variables.
Controller resolution and validation internal/controller/storage_controller.go, internal/controller/storage_tier_definitions.go, internal/controller/storage_tier_resolution.go, internal/controller/tenant_names.go, internal/controller/storage_controller_test.go	Resolves tiers and backends with caching and error handling, injects data during update and delete reconciliation, tracks ambiguous tiers, and emits missing-tier warnings and condition messages.
gRPC client wiring cmd/main.go	Passes the gRPC connection into storage-controller setup and conditionally assigns private Tier and Backend clients.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant main
  participant StorageReconciler
  participant StorageTiersLister
  participant StorageBackendsGetter
  participant AAPProvider
  main->>StorageReconciler: Configure Tier and Backend clients
  StorageReconciler->>StorageTiersLister: List storage tiers
  StorageReconciler->>StorageBackendsGetter: Get backend connections
  StorageReconciler->>AAPProvider: Trigger provisioning with context data
  AAPProvider->>AAPProvider: Build storage extra variables

Loading

Possibly related PRs

• osac-project/osac-operator#354: Both changes wire fulfillment/private v1 clients into the storage reconciler.

Suggested reviewers: jhernand, akshaynadkarni, zszabo-rh

🚥 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 summarizes the main change: wiring Tier API into the storage controller for validation and AAP extra vars.
Docstring Coverage	✅ Passed	Docstring coverage is 80.95% which is sufficient. The required threshold is 80.00%.
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	Only credential literals are test fixture values in _test.go with test- identifiers; no hardcoded secrets, embedded creds, or secret-like blobs were found in changed code.
No-Weak-Crypto	✅ Passed	Inspected the changed controller/provisioning files: no MD5/SHA1/DES/RC4/ECB/custom crypto or secret/token comparisons; config hashing uses FNV-64a, not crypto.
No-Injection-Vectors	✅ Passed	No SQL/shell/eval/yaml.load/HTML sinks were added; dynamic data is only marshaled into typed maps/context and gRPC/AAP payloads.
Container-Privileges	✅ Passed	PR only changes Go controller/provisioning code; no container/K8s manifests were modified and no privileged settings were introduced.
No-Sensitive-Data-In-Logs	✅ Passed	No new logs emit passwords, tokens, API keys, or payloads; added logs only report metadata like tenant, tier, backendID, and secret names.
Ai-Attribution	✅ Passed	AI-assisted commits use Red Hat Assisted-by: trailers, and no Co-Authored-By AI attribution was found in the branch.

✨ Finishing Touches

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/controller/storage_controller.go`:
- Around line 236-246: The resolve-and-inject logic is duplicated between
handleUpdate and handleDelete. Extract it into a Controller helper named
withResolvedStorageContext(ctx) returning (context.Context, error), move the
TiersClient check, tier resolution, error propagation, and provisioning context
injection there, then replace both call sites with the helper so delete-path
leniency remains centralized.
- Around line 648-658: The tier-resolution failure in the reconcile flow should
be best-effort during deletion. Update the block calling resolveTierDefinitions
to detect the delete path, log the error, retain empty tierDefinitions and
backendConnections, and continue so storageFinalizer removal proceeds; preserve
the existing error return for non-delete reconciles.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c0a83f2f-92bd-4389-bd64-d5bf915682fb

📥 Commits

Reviewing files that changed from the base of the PR and between a5a5c5b and 59e92f2.

📒 Files selected for processing (10)

• cmd/main.go
• internal/controller/storage_controller.go
• internal/controller/storage_controller_test.go
• internal/controller/storage_tier_definitions.go
• internal/controller/storage_tier_resolution.go
• internal/controller/tenant_names.go
• pkg/provisioning/aap_provider.go
• pkg/provisioning/aap_provider_test.go
• pkg/provisioning/extra_vars_context.go
• pkg/provisioning/extra_vars_context_test.go

[akshaynadkarni]

Thanks for addressing the comment. Loved ❤️ the Backends cache. It will speed up processing 👍 .

Changes LGTM.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akshaynadkarni, wgordon17

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [akshaynadkarni]

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