OSAC-3712: fix LVMS namespace resolution and tenant_name for hub storage

[omer-vishlitzky]

Summary

• Fix LVMS storage role namespace resolution: add service account namespace file lookup so the role uses the pod's actual namespace instead of hardcoded osac-system
• Remove osac-system hardcoded fallback from both lvms_storage and vast_storage roles — missing namespace propagates as empty string and fails at the k8s API call rather than silently targeting the wrong namespace
• Fix tenant_name and tenant_namespace in cluster storage playbooks: use explicit elif Tenant dispatch instead of else fallback, so unknown resource kinds produce an empty value caught by downstream validation

Root Cause

The LVMS role had a 2-tier namespace fallback (env var → osac-system) that skipped the SA namespace file the pod already mounts. In CI (deployed to osac-e2e-ci), backend jobs targeted osac-system (404), cascading: no backends → no StorageTiers → empty storage_tier_definitions → ClusterStorageReady stays False → all VMaaS tests fail.

Separately, tenant_name read osac.openshift.io/tenant annotation unconditionally, but Tenant CRs don't carry it — only ClusterOrders do. And all kind-dispatch vars used else fallbacks that would silently accept unknown resource kinds.

Design decisions

No osac-system fallback. The SA namespace file is always available in-cluster (mounted via downward API in the storage-operations-ig pod spec). If both the env var and the file are unavailable, that's a deployment bug that should fail — not silently target a hardcoded namespace. The file lookup uses errors='ignore' (not errors='strict') because integration tests run outside a pod on GH Actions runners where the SA file doesn't exist; those tests set OSAC_STORAGE_CONFIG_NAMESPACE explicitly via environment: on each task.

Explicit kind dispatch. tenant_name and tenant_namespace use elif Tenant instead of else, so an unexpected resource kind produces an empty value caught by downstream validation rather than silently taking the Tenant path.

Test plan

• VMaaS CI (e2e-vmaas-full-install) passes with this change
• LVMS backend jobs target correct namespace (osac-e2e-ci in CI)
• ClusterStorageReady condition becomes True
• CaaS path (ClusterOrder) still works (annotation-based tenant_name preserved)
• Integration tests pass (env var set explicitly, no SA file needed)

Jira: OSAC-3712

🤖 Generated with Claude Code

[openshift-ci-robot]

@omer-vishlitzky: This pull request references OSAC-3712 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

• Fix LVMS storage role namespace resolution: add service account namespace file fallback (matching vast_storage pattern) so the role uses the pod's actual namespace instead of hardcoded osac-system
• Fix tenant_name in cluster storage playbooks: add kind-based conditional (Tenant → metadata.name, ClusterOrder → annotation) matching existing tenant_namespace pattern

Root Cause

The LVMS role's 2-tier namespace fallback (env var → osac-system) missed the SA namespace file the pod already mounts. In CI (deployed to osac-e2e-ci), backend jobs targeted osac-system (404), cascading: no backends → no StorageTiers → empty storage_tier_definitions → ClusterStorageReady stays False → all VMaaS tests fail.

Separately, tenant_name read osac.openshift.io/tenant annotation unconditionally, but Tenant CRs don't carry it — only ClusterOrders do.

Test plan

• VMaaS CI (e2e-vmaas-full-install) passes with this change
• LVMS backend jobs target correct namespace (osac-e2e-ci in CI)
• ClusterStorageReady condition becomes True
• CaaS path (ClusterOrder) still works (annotation-based tenant_name preserved)

Jira: OSAC-3712

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

[coderabbitai]

Warning

Review limit reached

@omer-vishlitzky, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 837bb270-cf7e-4ef4-af07-2c72111c7f84

📥 Commits

Reviewing files that changed from the base of the PR and between 1dff977 and 3f8ba69.

📒 Files selected for processing (9)

• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml
• osac-aap/collections/ansible_collections/osac/templates/roles/vast_storage/defaults/main.yaml
• osac-aap/playbook_osac_create_tenant_cluster_storage.yml
• osac-aap/playbook_osac_delete_tenant_cluster_storage.yml
• osac-aap/tests/integration/targets/storage_provider_ensure_sc/tasks/main.yml
• osac-aap/tests/integration/targets/storage_provider_onboarding/tasks/main.yml
• osac-aap/tests/integration/targets/storage_provider_setup/tasks/main.yml
• osac-aap/tests/integration/targets/storage_provider_setup_rollback/tasks/main.yml
• osac-aap/tests/integration/targets/storage_provider_teardown/tasks/main.yml

Walkthrough

Changes

Storage routing updates

Layer / File(s)	Summary
Namespace fallback resolution osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml	lvms_storage_config_namespace checks OSAC_STORAGE_CONFIG_NAMESPACE, the service-account namespace file, and osac-system.
Kind-specific tenant-name selection osac-aap/playbook_osac_create_tenant_cluster_storage.yml, osac-aap/playbook_osac_delete_tenant_cluster_storage.yml	ClusterOrder resources use the tenant annotation. Other resource kinds use metadata.name.

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

Possibly related PRs

• osac-project/osac#97: Updates tenant cluster storage playbook inputs and selection logic.
• osac-project/osac#131: Updates the same storage defaults file.

Suggested labels: storage

Suggested reviewers: rgolangh, wgordon17, akshaynadkarni

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
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 three changed files add no API keys, tokens, passwords, private keys, embedded credentials, or long base64/hex literals; the namespace path is a standard Kubernetes service-account file.
No-Weak-Crypto	✅ Passed	The commit only adds a service-account namespace lookup and kind-based tenant-name selection; no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons were added.
No-Injection-Vectors	✅ Passed	The PR additions only read a fixed service-account file and select resource metadata; scans found no shell, eval/exec, pickle, unsafe YAML, os.system, SQL, or HTML injection constructs.
Container-Privileges	✅ Passed	The PR changes only Ansible defaults and playbook variables; the added lines contain no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	The PR adds no logging statements. New values are used for namespace and tenant selection; existing metadata debug output and role messages are unchanged and do not log credentials.
Ai-Attribution	✅ Passed	AI use is disclosed, and the sole PR commit has Assisted-by: Claude Code <noreply@anthropic.com> with no AI Co-Authored-By trailer.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies both main fixes: LVMS namespace resolution and tenant_name handling for hub storage.

✨ 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: 1

🤖 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 `@osac-aap/playbook_osac_create_tenant_cluster_storage.yml`:
- Around line 17-22: Validate the resource kind before deriving tenant_name in
both osac-aap/playbook_osac_create_tenant_cluster_storage.yml:17-22 and
osac-aap/playbook_osac_delete_tenant_cluster_storage.yml:16-21: accept only
Tenant and ClusterOrder, and reject any other kind. For ClusterOrder, require a
non-empty osac.openshift.io/tenant annotation before accessing it; retain
metadata.name as the Tenant source and apply identical validation before
creation and teardown.

🪄 Autofix

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: CHILL

Plan: Pro Plus

Run ID: 12fb3703-f19b-4410-859e-252f459d3aec

📥 Commits

Reviewing files that changed from the base of the PR and between 0fec1ac and 1dff977.

📒 Files selected for processing (3)

• osac-aap/collections/ansible_collections/osac/templates/roles/lvms_storage/defaults/main.yaml
• osac-aap/playbook_osac_create_tenant_cluster_storage.yml
• osac-aap/playbook_osac_delete_tenant_cluster_storage.yml

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Validate the resource kind and ClusterOrder annotation before deriving tenant_name.

Both playbooks treat every kind other than ClusterOrder as Tenant, and both index a possibly absent annotation. This can target the wrong tenant or fail before the storage role runs.

• osac-aap/playbook_osac_create_tenant_cluster_storage.yml#L17-L22: accept only Tenant and ClusterOrder, and require a non-empty osac.openshift.io/tenant annotation for ClusterOrder.
• osac-aap/playbook_osac_delete_tenant_cluster_storage.yml#L16-L21: apply the same validation before teardown.

📍 Affects 2 files

• osac-aap/playbook_osac_create_tenant_cluster_storage.yml#L17-L22 (this comment)
• osac-aap/playbook_osac_delete_tenant_cluster_storage.yml#L16-L21

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

In `@osac-aap/playbook_osac_create_tenant_cluster_storage.yml` around lines 17 -
22, Validate the resource kind before deriving tenant_name in both
osac-aap/playbook_osac_create_tenant_cluster_storage.yml:17-22 and
osac-aap/playbook_osac_delete_tenant_cluster_storage.yml:16-21: accept only
Tenant and ClusterOrder, and reject any other kind. For ClusterOrder, require a
non-empty osac.openshift.io/tenant annotation before accessing it; retain
metadata.name as the Tenant source and apply identical validation before
creation and teardown.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: omer-vishlitzky, zszabo-rh

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:

• osac-aap/OWNERS [omer-vishlitzky,zszabo-rh]

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