This repository was archived by the owner on Sep 9, 2026. It is now read-only.
OSAC-43: adds VAST storage provider - #296
Merged
openshift-merge-bot[bot] merged 6 commits intoMay 27, 2026
Merged
openshift-merge-bot[bot] merged 6 commits into
openshift-merge-bot[bot] merged 6 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
@wgordon17: This pull request references MGMT-23930 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
wgordon17
changed the title
|
@wgordon17: This pull request references OSAC-43 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
wgordon17
force-pushed
the
MGMT-23930-ansible-storage-mvp
branch
from
May 13, 2026 21:05
017c5f9 to
33907a7
Compare
wgordon17
changed the title
|
@wgordon17: This pull request references MGMT-23930 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
wgordon17
changed the title
wgordon17
changed the title
|
@wgordon17: This pull request references OSAC-43 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 epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
wgordon17
force-pushed
the
MGMT-23930-ansible-storage-mvp
branch
2 times, most recently
from
May 15, 2026 19:40
02b9a70 to
3a64be2
Compare
wgordon17
force-pushed
the
MGMT-23930-ansible-storage-mvp
branch
13 times, most recently
from
May 17, 2026 18:51
09f1235 to
79db59d
Compare
Contributor
|
/retest |
1 similar comment
Contributor
|
/retest |
Signed-off-by: Will Gordon <wgordon@redhat.com> Generated-By: Claude Code (Anthropic)
…rovider Correctness: moves _provider_tiers into vars: on include_role, moves _vast_vms_conn before VIP pool creation, adds backward-compat default tier list in delete playbook, changes SC short-circuit to subset check, creates per-protocol view policies with proper name/ID separation, fixes view policy deletion for per-protocol loop with failure accumulator, fails explicitly on missing view_policy_name. Style: extracts _unique_providers and _dispatch_action to main.yaml, removes dead code. Testing: adds block tier and encryption passphrase coverage, adds admin cred absence assertion, fixes test assertions, adds max_tiers validation, removes smoke test from tracking. Signed-off-by: Will Gordon <wgordon@redhat.com> Generated-By: Claude Code (Anthropic) Signed-off-by: Will Gordon <wgordon@redhat.com>
…s and label selectors
akshaynadkarni
force-pushed
the
MGMT-23930-ansible-storage-mvp
branch
from
May 26, 2026 19:27
0015679 to
acfdba3
Compare
|
Warning Review limit reached
More reviews will be available in 50 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis PR introduces a comprehensive storage provider framework with VAST Data as the first implementation. It adds just-in-time storage provisioning during compute instance creation, event-driven tenant lifecycle management, a mock VMS REST API server for testing, and extensive integration test coverage alongside AAP controller configuration for automated storage operations. ChangesStorage Provider Framework & VAST Implementation
Compute Instance & Workflow Integration
Mock VMS Server & Integration Test Scaffold
Storage Provider Integration Tests
Configuration, Documentation & Dependencies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This PR is substantial and heterogeneous: it introduces a new architectural pattern (pluggable storage providers via dispatcher role), a complete VAST provider implementation with complex credential/VMS API interactions, event-driven playbooks, a realistic mock server for integration testing (348 lines of Python), comprehensive test coverage (multiple integration test playbooks), and AAP configuration changes. While most individual pieces follow clear patterns, the interconnections between storage dispatch, JIT provisioning, teardown/rollback, and test infrastructure demand careful review to ensure credential isolation, state cleanup, idempotency, and error handling are sound. Risk severity is medium-high: improperly managed VMS credentials, incomplete rollback, or race conditions in short-circuit logic could leave orphaned resources or expose credentials. Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/tests.yml (1)
19-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSecurity risk: GitHub Actions must be pinned by full commit SHA, not mutable tags.
Actions are currently pinned by tags (v6, v7, v1) which are mutable references. An attacker who compromises the action repository or gains control of the tag could inject malicious code into your CI pipeline.
Risk severity: Major
Impact: Supply chain attack vector enabling arbitrary code execution in CI environment with access to repository secrets and GITHUB_TOKEN.🔒 Recommended fix: Pin all actions by full SHA
- name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@<full-sha> # v6 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@<full-sha> # v7 - name: "Set up Python" - uses: actions/setup-python@v6 + uses: actions/setup-python@<full-sha> # v6 - name: Install kind - uses: helm/kind-action@v1 + uses: helm/kind-action@<full-sha> # v1Look up the current commit SHAs for each action version and replace
<full-sha>with the 40-character commit hash. Keep the version tag in a comment for readability.As per coding guidelines: "Pin actions by full SHA, not tag" for CI/CD security.
🤖 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 @.github/workflows/tests.yml around lines 19 - 57, Replace mutable tag pins for GitHub Actions with immutable full commit SHAs: find every uses: entry referencing actions/checkout@v6, astral-sh/setup-uv@v7, actions/setup-python@v6, and helm/kind-action@v1 (appearing in both jobs) and replace the tag with the corresponding 40-character commit SHA (you may keep the original tag as a comment for readability); ensure you look up and paste the exact commit SHA for each action and apply the same change to any other action usages added later to eliminate mutable tag usage.tests/integration/setup_test_env.sh (1)
77-80: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPin external manifest sources to immutable refs.
Severity: major (supply-chain/reliability). Impact: fetching CRDs from
maincan silently change behavior and destabilize test security posture. Use tagged release URLs or commit SHAs.🤖 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 `@tests/integration/setup_test_env.sh` around lines 77 - 80, The test script currently applies CRDs from mutable "main" branches using the two kubectl apply commands that fetch URLs for stolostron/managedcluster-import-controller and kubevirt/hyperconverged-cluster-operator; change those URLs to point to immutable refs (a specific release tag or commit SHA) or introduce variables (e.g., MANAGED_CLUSTER_CRD_REF and HCO_CRD_REF) that default to a known release tag/sha and are referenced in the kubectl apply commands, and update the echo messages accordingly so the CRDs are fetched from pinned URLs instead of the main branch.
🤖 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
`@collections/ansible_collections/osac/config_as_code/roles/aap/templates/storage-operations-ig.j2`:
- Around line 6-31: Add explicit pod/container hardening for the "worker"
container in the storage-operations-ig.j2 template: set a
podSecurityContext/runAsNonRoot (e.g., runAsNonRoot: true and optional runAsUser
non-zero) and add a container-level securityContext for the "worker" container
that sets readOnlyRootFilesystem: true, allowPrivilegeEscalation: false and
drops all capabilities (capabilities.drop: ["ALL"]). Ensure these controls are
added adjacent to the "containers - name: worker" block so the pod and
container-level security contexts are enforced.
In
`@collections/ansible_collections/osac/service/roles/storage_provider/tasks/_dispatch_provider.yaml`:
- Around line 30-33: The dynamic include_role call that builds
"osac.templates.{{ _current_provider }}_storage" must be guarded by an explicit
allow-list check: validate _current_provider against a predefined list (e.g.,
storage_providers_allowed or providers_map) before calling
ansible.builtin.include_role; if the value is not in the allow-list, fail or
default to a safe provider. Update the task that uses include_role (the block
referencing _current_provider and _dispatch_action) to perform the allow-list
membership test (using a when or prereq task that sets/validates a
safe_provider) and only include the role when the provider is allowed. Ensure
the allow-list variable name is clear and documented and that failure paths
produce a descriptive error rather than proceeding with an unchecked provider.
In
`@collections/ansible_collections/osac/service/roles/storage_provider/tasks/main.yaml`:
- Around line 81-83: The when expression currently assumes
storage_provider_provisioning_target and storage_provider_action exist and can
raise an error if unset; update the conditional checks in the tasks that use
these variables (the when clauses around storage_provider_provisioning_target
not in [...] and the later checks for storage_provider_action) to first ensure
the variable is defined (e.g. "storage_provider_provisioning_target is defined
and storage_provider_provisioning_target not in [...]" and
"storage_provider_action is defined and storage_provider_action in [...]" or use
a safe default like "| default('')" ), so the fail task can run predictably and
validation logic won’t error when inputs are missing.
- Around line 14-16: The current when condition only checks defined/non-empty
for storage_provider_tiers and can allow non-list types; update the when clause
that guards the task using the storage_provider_tiers variable to also assert it
is a sequence/list and has length > 0 (e.g., require storage_provider_tiers is
defined and storage_provider_tiers is sequence and storage_provider_tiers |
length > 0) so downstream loops receive a proper list; if you prefer stricter
failure handling, add an assert task that checks storage_provider_tiers is
sequence before any tasks that iterate over it.
In
`@collections/ansible_collections/osac/service/roles/storage_provider/tasks/teardown.yaml`:
- Line 13: The task currently using "ignore_errors: true" hides individual
provider teardown failures; instead, remove that global ignore and register each
provider's teardown result (e.g., register: teardown_result in the per-provider
task or loop over providers), collect failures by inspecting
teardown_result.results | selectattr('failed') into a list, and at the end add a
final task that fails with the fail module if any failures exist, including
context (provider name and error message) so the playbook surfaces teardown
errors rather than masking them.
In `@collections/ansible_collections/osac/templates/README.md`:
- Around line 212-214: The README incorrectly states that a hardcoded provider
allowlist exists in osac.service.storage_provider/tasks/main.yaml; update the
documentation to reflect the actual dispatcher validation/dispatch model by
removing or correcting the claim about a hardcoded allowlist and describing how
the dispatcher currently validates providers (mentioning the dispatcher
component and any runtime/variable-driven checks it uses). Reference
osac.service.storage_provider/tasks/main.yaml and the dispatcher when editing
the text so provider authors see the accurate source of truth and behavior.
- Around line 187-192: Fix the markdownlint MD031/MD022 spacing issues in
README.md by ensuring proper blank lines around the touched fenced code blocks
and headings: add a single blank line between headings and the opening fenced
blocks (e.g., before the ```bash and ```yaml blocks shown) and ensure a blank
line after closing fenced blocks where required; apply the same spacing
correction to the other reported fenced blocks (the occurrences flagged at the
other touched spots) and remove any extra trailing blank lines so the file
conforms to MD031/MD022.
In
`@collections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_qos_policy.yaml`:
- Around line 60-80: The task "Create QoS policies that do not yet exist" is
using validate_certs: "{{ _vast_vms_conn.validate_certs }}" which allows
disabling TLS verification and exposes bearer tokens (Authorization header) to
MITM; update the role to require validate_certs be true for the
ansible.builtin.uri call (or replace the templated value with a hard true) and
add a pre-check (e.g., a task that fails when _vast_vms_conn.validate_certs is
false) so the POST to /api/qospolicies/ never runs with validate_certs disabled;
ensure the change is applied to the task registering _vast_qos_create_results
and any related token-fetching requests that use _vast_vms_conn.validate_certs.
- Around line 45-58: The task "Check if QoS policy already exists per tier"
currently uses the external flag _vast_vms_conn.validate_certs which can disable
TLS certificate validation and expose the Authorization bearer token; change the
task to enforce secure TLS by replacing the variable with validate_certs: true
(or add a pre-task that fails if _vast_vms_conn.validate_certs is not true) so
the ansible.builtin.uri call always validates certificates when querying
/api/qospolicies/ (the task that registers _vast_qos_check_results and loops
over _vast_qos_tiers should be updated accordingly).
- Around line 32-43: The task currently passes validate_certs: "{{
_vast_vms_conn.validate_certs }}" to ansible.builtin.uri, allowing TLS
verification to be disabled via vast_storage_validate_certs/VAST_VALIDATE_CERTS;
change this to enforce TLS verification by either setting validate_certs: true
unconditionally in create_qos_policy.yaml or add an explicit pre-task that fails
(with a clear error) when vast_storage_validate_certs is false (and reference
the variable _vast_vms_conn.validate_certs and the ansible.builtin.uri call so
reviewers can find where to apply the change).
In
`@collections/ansible_collections/osac/templates/roles/vast_storage/tasks/delete_tenant_manager.yaml`:
- Around line 103-107: The current set_fact uses Jinja tests comparing variables
to the literal "failed", which misses real failure flags; change the expression
assigning _vast_manager_delete_failed to explicitly check the registered result
objects' .failed attributes and guard existence: e.g. set
_vast_manager_delete_failed to true if (_vast_manager_delete_result is defined
and _vast_manager_delete_result.failed) or (_vast_manager_lookup is defined and
_vast_manager_lookup.failed); ensure both _vast_manager_delete_result and
_vast_manager_lookup are checked with "is defined" before accessing .failed so
callers accurately see teardown failures.
In
`@collections/ansible_collections/osac/templates/roles/vast_storage/tasks/ensure_storage_class.yaml`:
- Around line 3-4: The top-of-file security comment is misleading: although
admin credentials are not placed into the CSI Secret, this file does load admin
credentials via the include of read_credentials.yaml (see the
include/read_credentials.yaml usage around the VMS API tasks), so update the
comment to accurately state that admin credentials are referenced/loaded here
but are never written into the CSI Secret (only tenant manager credentials are
stored in the CSI Secret). Change the sentence "Admin credentials are NEVER
loaded or referenced in this file." to a precise statement referencing the
include (read_credentials.yaml) and clarifying the actual guarantee (admin creds
are used for VMS API operations but never injected into the CSI Secret).
In
`@collections/ansible_collections/osac/templates/roles/vast_storage/tasks/read_tenant_credentials.yaml`:
- Around line 58-67: The current set_fact decodes several secret fields without
checking they exist on _vast_tenant_config_secret, which causes opaque errors
later; add an explicit pre-validation task (e.g., ansible.builtin.assert or
ansible.builtin.fail) that checks _vast_tenant_config_secret.resources[0].data
contains required keys like vast_tenant_id, tenant_manager_username,
tenant_manager_password, vip_pool_name, vast_endpoint (and optionally
view_policy_names/block_encryption_passphrase if required), and only then run
the ansible.builtin.set_fact that decodes into _vast_tenant_id,
_vast_tenant_csi_username, _vast_tenant_csi_password, _vast_tenant_vip_pool,
_vast_tenant_endpoint, _vast_view_policy_name,
_vast_tenant_block_encryption_passphrase, and _vast_view_policy_names_raw;
ensure the assert/fail produces a clear message naming the missing key(s).
In `@config/base/secret-storage-operations-ig-example.yaml`:
- Around line 3-5: The example Secret manifest omits metadata.namespace which
can create the Secret in the cluster default namespace; update the manifest for
the Secret named "storage-operations-ig" (type Opaque) to include an explicit
metadata.namespace (e.g., the intended ops namespace) so it is pinned to a known
namespace; ensure any consumers/RBAC or deployment manifests reference that same
namespace after you add metadata.namespace.
In `@playbook_osac_configure_tenant_storage.yml`:
- Around line 13-15: The playbook currently logs the entire EDA event payload
(ansible_eda.event.payload), which may expose secrets; change the debug to avoid
printing the full payload and instead output only non-sensitive identifiers such
as tenant name and namespace (e.g., ansible_eda.event.payload.tenant_name and
ansible_eda.event.payload.namespace or equivalent fields), or redact sensitive
keys before logging; update the task referenced by the debug call (the "Show EDA
Event" task) to only emit those safe identifiers and remove any full-payload
debug statements.
In `@playbook_osac_create_compute_instance.yml`:
- Around line 59-65: The fail task "Fail on malformed STORAGE_TIERS JSON"
currently echoes the raw value _storage_tiers_raw which may leak secrets; update
this task to redact or truncate the value before logging (e.g., show only a safe
prefix and suffix or replace with "[REDACTED]" when length/exposure thresholds
are met) so the ansible.builtin.fail msg still indicates malformed JSON but
never prints the full _storage_tiers_raw; ensure the logic used to produce the
redacted string is applied where _storage_tiers_raw is referenced for error
output.
In `@playbook_osac_delete_tenant_storage.yml`:
- Around line 12-14: Remove or redact the debug task "Show EDA Event" that
prints ansible_eda.event.payload in playbook_osac_delete_tenant_storage.yml;
either delete the ansible.builtin.debug task entirely or replace it with a
targeted debug that logs only non-sensitive, whitelisted fields (e.g., event id
or status) and explicitly omit tenant/customer fields, ensuring no full payload
or PII is emitted.
In `@tests/integration/mock_vms_server.py`:
- Around line 323-324: The log_message method currently uses a parameter named
format which shadows the Python builtin; rename the parameter in the
BaseHTTPRequestHandler override from format to fmt (i.e., def log_message(self,
fmt, *args)) and update any references inside log_message (if any) to use fmt so
linters won't complain while preserving the original behavior of the log_message
override.
In `@tests/integration/setup_test_env.sh`:
- Around line 168-177: The script currently materializes plaintext credentials
into .storage_env (variables like VAST_PASSWORD, VAST_USERNAME and
STORAGE_TIERS); change it to read secrets from the environment at runtime
instead of hardcoding them into the heredoc, and ensure the created file is
protected (set umask 077 before writing or explicitly chmod 600 after creation).
Specifically, replace hardcoded string literals for VAST_* and STORAGE_TIERS
with references that pull from existing env vars (e.g., ${VAST_PASSWORD}) so
tests rely on externally supplied secrets, and add a file-permissions step
around the write of .storage_env to prevent world-readable artifacts.
In `@tests/integration/targets/storage_provider_setup_rollback/tasks/main.yml`:
- Around line 41-45: The playbook currently hardcodes credentials in the
environment block (VAST_USERNAME, VAST_PASSWORD, VAST_ENDPOINT,
VAST_VIP_POOL_NAME); replace these string literals with environment-driven or
injected variables so secrets aren’t committed: read VAST_USERNAME and
VAST_PASSWORD from the runtime environment or test harness variables (with safe
empty/defaults or vault lookup) and make VAST_ENDPOINT and VAST_VIP_POOL_NAME
configurable via vars or CI-provided envs instead of fixed strings so tests use
injected values rather than "admin"/"admin".
In `@tests/integration/targets/storage_provider_setup/tasks/main.yml`:
- Around line 12-14: The playbook currently hardcodes credentials (VAST_USERNAME
and VAST_PASSWORD) and the endpoint (VAST_ENDPOINT); replace these literals by
reading them from environment/CI variables or test fixtures and fail fast if
missing — update the task that sets VAST_ENDPOINT, VAST_USERNAME, and
VAST_PASSWORD to use lookup of env vars (or variables injected from fixtures)
and add a pre-check that aborts the run with a clear error when any of those
variables is undefined or empty.
In `@tests/integration/targets/storage_provider_teardown/tasks/main.yml`:
- Around line 92-93: Replace the hardcoded secrets (tenant_manager_password,
tenant_manager_id, CSI token, and any admin env credentials) with
runtime-injected variables from the test harness or environment (e.g., Ansible
lookups or test fixture variables) so credentials are not committed; update
occurrences of tenant_manager_password, tenant_manager_id and the CSI
token/admin env vars in this task (and the other occurrences called out) to
reference injected vars (environment/test-harness variables or vaulted vars) and
ensure defaults are not hardcoded in the YAML.
---
Outside diff comments:
In @.github/workflows/tests.yml:
- Around line 19-57: Replace mutable tag pins for GitHub Actions with immutable
full commit SHAs: find every uses: entry referencing actions/checkout@v6,
astral-sh/setup-uv@v7, actions/setup-python@v6, and helm/kind-action@v1
(appearing in both jobs) and replace the tag with the corresponding 40-character
commit SHA (you may keep the original tag as a comment for readability); ensure
you look up and paste the exact commit SHA for each action and apply the same
change to any other action usages added later to eliminate mutable tag usage.
In `@tests/integration/setup_test_env.sh`:
- Around line 77-80: The test script currently applies CRDs from mutable "main"
branches using the two kubectl apply commands that fetch URLs for
stolostron/managedcluster-import-controller and
kubevirt/hyperconverged-cluster-operator; change those URLs to point to
immutable refs (a specific release tag or commit SHA) or introduce variables
(e.g., MANAGED_CLUSTER_CRD_REF and HCO_CRD_REF) that default to a known release
tag/sha and are referenced in the kubectl apply commands, and update the echo
messages accordingly so the CRDs are fetched from pinned URLs instead of the
main branch.
🪄 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: 7fde91bd-f28b-4388-adb7-bd01632fb54e
⛔ Files ignored due to path filters (113)
vendor/ansible_collections/vastdata.vms-1.2.0.info/GALAXY.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/CHANGELOG.mdis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/FILES.jsonis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/MANIFEST.jsonis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/README.mdis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/docs/.gitkeepis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/meta/runtime.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/_build_info.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/auth.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/client.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/diff.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/errors.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/module_customizations.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/protectionpolicies_utils.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/resource.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/s3_attachment_utils.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/schema_overrides.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/sub_endpoint_resource.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/timeouts.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/module_utils/vast/version.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/dns.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/eventdefinitionconfigs.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/globalsnapstreams.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/groups.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/ldaps.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/nativereplicationremotetargets.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/nonlocal_group.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/nonlocal_user.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/protectedpaths.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/protectionpolicies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/quotas.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/s3policies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/snapshots.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/tenants.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/user_key.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/users.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/viewpolicies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/views.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/plugins/modules/vippools.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/.gitignoreis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/requirements.txtis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/.gitkeepis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/cnodes/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/cnodes/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/dns/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/dns/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/eventdefinitionconfigs/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/eventdefinitionconfigs/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/globalsnapstreams/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/globalsnapstreams/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/groups/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/groups/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/ldaps/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/ldaps/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nativereplicationremotetargets/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nativereplicationremotetargets/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nonlocal_group/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nonlocal_group/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nonlocal_user/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/nonlocal_user/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/protectedpaths/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/protectedpaths/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/protectionpolicies/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/protectionpolicies/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/quotas/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/quotas/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3_policy_attachment_group/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3_policy_attachment_group/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3_policy_attachment_user/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3_policy_attachment_user/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3policies/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/s3policies/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/snapshots/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/snapshots/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/tenants/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/tenants/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/user_key/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/user_key/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/users/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/users/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/viewpolicies/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/viewpolicies/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/views/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/views/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/vippools/defaults/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/integration/targets/vippools/tasks/main.ymlis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/.gitkeepis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/module_utils/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/module_utils/test_delete_params.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/__init__.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_dns.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_eventdefinitionconfigs.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_globalsnapstreams.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_groups.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_ldaps.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_nativereplicationremotetargets.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_nonlocal_group.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_nonlocal_user.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_protectedpaths.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_protectionpolicies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_quotas.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_s3policies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_snapshots.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_tenants.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_user_key.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_users.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_viewpolicies.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_views.pyis excluded by!vendor/**vendor/ansible_collections/vastdata/vms/tests/unit/plugins/modules/test_vippools.pyis excluded by!vendor/**
📒 Files selected for processing (53)
.github/workflows/tests.yml.gitignorecollections/ansible_collections/osac/config_as_code/roles/aap/templates/compute-instance-operations-ig.j2collections/ansible_collections/osac/config_as_code/roles/aap/templates/storage-operations-ig.j2collections/ansible_collections/osac/config_as_code/roles/aap/vars/controller.ymlcollections/ansible_collections/osac/service/plugins/filter/find_template_roles.pycollections/ansible_collections/osac/service/roles/storage_provider/meta/argument_specs.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tasks/_dispatch_provider.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tasks/ensure_storage_class.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tasks/main.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tasks/setup.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tasks/teardown.yamlcollections/ansible_collections/osac/service/roles/storage_provider/tests/test.ymlcollections/ansible_collections/osac/templates/README.mdcollections/ansible_collections/osac/templates/roles/ocp_virt_vm/tasks/create_resources.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/defaults/main.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/meta/osac.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_qos_policy.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_quotas.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_tenant_manager.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_view_policy.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/create_views.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/delete_tenant_manager.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/delete_vms_resources_by_tenant.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/ensure_csi_operator.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/ensure_storage_class.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/read_credentials.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/read_tenant_credentials.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/setup.yamlcollections/ansible_collections/osac/templates/roles/vast_storage/tasks/teardown.yamlcollections/ansible_collections/osac/workflows/playbooks/compute_instance/create.ymlcollections/requirements.ymlconfig/base/configmap-storage-operations-ig-example.yamlconfig/base/secret-storage-operations-ig-example.yamlplaybook_osac_configure_tenant_storage.ymlplaybook_osac_create_compute_instance.ymlplaybook_osac_delete_tenant_storage.ymltests/integration/fixtures/storage/secret-vast-tenant-config-test.yamltests/integration/fixtures/storage/storageclass-vast-test.yamltests/integration/fixtures/storage/tenant-storage-test.yamltests/integration/integration_config.yml.templatetests/integration/mock_vms_server.pytests/integration/run_tests.shtests/integration/setup_test_env.shtests/integration/targets/compute_instance_create/tasks/baseline.ymltests/integration/targets/compute_instance_create/tasks/overrides.ymltests/integration/targets/compute_instance_with_gpu_create/tasks/baseline.ymltests/integration/targets/storage_provider_ensure_sc/tasks/main.ymltests/integration/targets/storage_provider_onboarding/tasks/main.ymltests/integration/targets/storage_provider_setup/tasks/main.ymltests/integration/targets/storage_provider_setup_rollback/tasks/main.ymltests/integration/targets/storage_provider_teardown/tasks/main.ymltests/integration/teardown_test_env.sh
The vendored vastdata.vms collection pins requires_ansible to >=2.19.0,<2.20.0, but ansible-core 2.21.0 is now the latest stable release. The collection only uses ansible.module_utils.basic.AnsibleModule, which is stable across all 2.x versions, so the upper bound is overly conservative. Widen to <3.0.0 to unblock the EE build. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
The comment claimed admin credentials are never loaded in this file, but the file includes read_credentials.yaml which loads them from env vars for VMS API calls. The actual guarantee is that admin credentials are never written into the CSI Secret or tenant-namespace resources. Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com> Assisted-by: Cursor/Claude Signed-off-by: akshaynadkarni <25892229+akshaynadkarni@users.noreply.github.com>
akshaynadkarni
dismissed
coderabbitai[bot]’s stale review
May 27, 2026 03:38
All findings addressed or refuted with rationale. Follow-ups tracked in OSAC-1042, OSAC-1043, OSAC-1044.
Contributor
|
/override ci/prow/temp |
|
@akshaynadkarni: Overrode contexts on behalf of akshaynadkarni: ci/prow/temp DetailsIn response to this:
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 kubernetes-sigs/prow repository. |
akshaynadkarni
approved these changes
May 27, 2026
akshaynadkarni
left a comment
akshaynadkarni
left a comment
Contributor
There was a problem hiding this comment.
Changes LGTM overall. However, this is a big PR. In case something breaks, we will have to do a fast follow-up. cc: @zszabo-rh .
I don't want to keep this PR open too long as then there's more work:
- merge conflicts
- coderabbit comments
etc.
Approving the PR.
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 tasks
This was referenced Jun 23, 2026
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests