Skip to content

Fix storage plugin config not loaded due to ordering in load-vars#465

Merged
maorfr merged 1 commit into
mainfrom
fix/load-vars-storage-plugin-order
Jun 10, 2026
Merged

Fix storage plugin config not loaded due to ordering in load-vars#465
maorfr merged 1 commit into
mainfrom
fix/load-vars-storage-plugin-order

Conversation

@eurijon

@eurijon eurijon commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Move storage_plugin union into enabled_plugins before plugin config loading in load-vars.yaml
  • Previously, config/plugins/odf.yaml was never loaded when odf was set via storage_plugin but not explicitly listed in enabled_plugins, leaving odfExternalConfig undefined
  • The ODF deploy task silently skipped StorageCluster creation due to the undefined variable
  • Users should not need to add the storage plugin to enabled_plugins — setting storage_plugin should be sufficient

Root Cause

In load-vars.yaml, the plugin config loading filtered by enabled_plugins, but the storage_plugin union happened after that block. When odf was only set via storage_plugin and not in enabled_plugins, its config file was never read.

Test plan

  • Deploy with storage_plugin: odf and odf not in enabled_plugins — verify StorageCluster is created
  • Deploy with odf explicitly in enabled_plugins — verify no regression
  • Deploy with LVMS as storage_plugin — verify no regression

🤖 Generated with Claude Code

The storage_plugin union into enabled_plugins happened after plugin
config files were loaded, so config/plugins/odf.yaml was never read
when odf was only set via storage_plugin and not explicitly listed in
enabled_plugins. This left odfExternalConfig undefined, causing the
ODF deploy task to silently skip StorageCluster creation.

Move the storage_plugin union before the plugin config loading block
so the storage plugin's config file is always included.

Assisted-by: Claude Code <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The playbook's variable-loading sequence is reordered so that storage_plugin is added to enabled_plugins immediately after common configuration is included, rather than later in the flow. This allows subsequent plugin discovery and configuration tasks to access the complete, updated enabled_plugins list when selecting which plugin configurations to load.

Changes

Plugin enablement reordering in variable loading

Layer / File(s) Summary
Relocate storage plugin enablement in variable-loading sequence
playbooks/common/load-vars.yaml
The set_fact task unioning storage_plugin into enabled_plugins is moved earlier in the load sequence (lines 50–53), enabling downstream plugin discovery and configuration loading tasks to use the updated plugin list. The subsequent OpenShift version section shifts accordingly (lines 78–79).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes


Security & Risk Assessment

⚠️ Severity: Low — This is a task reordering change with no logic modifications.

Impact:

  • Plugin enablement now occurs earlier in the variable-loading sequence, making the updated enabled_plugins list available to downstream plugin discovery and configuration tasks.
  • No changes to the logic itself; purely a sequencing adjustment.

Risk Factors:

  • Low functional risk: The reordering should have minimal impact if downstream tasks only depend on the presence of storage_plugin in enabled_plugins, not on its absence or timing.
  • Validation requirement: Verify that plugin discovery and configuration loading correctly consume the earlier enabled_plugins update and do not have implicit ordering dependencies that this change breaks.
  • No breaking changes to exported entities: No public playbook outputs or shared variables are modified in signature.

🎭 A task steps forward in the load queue's dance,
So plugins find enablement faster than perchance,
The OpenShift section yields its place with grace,
Variable flows now reach the finish line with haste!

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: moving storage plugin config ordering in load-vars to ensure it loads before plugin config loading, fixing the bug where storage plugin config was not being loaded.
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 No hardcoded secrets detected. PR contains only code reordering with no new sensitive data, credentials, API keys, tokens, passwords, or encoded strings introduced.
No-Weak-Crypto ✅ Passed No cryptographic operations detected. PR only reorders Ansible playbook tasks; no weak ciphers, MD5, SHA1, DES, RC4, custom crypto, or insecure comparisons present.
No-Injection-Vectors ✅ Passed PR reorders a task in load-vars.yaml using safe Ansible modules. No SQL, shell=True, eval/exec, pickle, yaml.load, os.system, or dangerouslySetInnerHTML patterns found.
Container-Privileges ✅ Passed PR reorders Ansible playbook tasks without introducing privileged containers. Existing podman --privileged usage for ISO tool is operational necessity.
No-Sensitive-Data-In-Logs ✅ Passed The PR moves a task to an earlier position without adding logging statements. No sensitive data is exposed; the task operates only on plugin names like 'odf' and 'lvms'.
Ai-Attribution ✅ Passed AI tool usage (Claude Code) is properly attributed with "Assisted-by" trailer in commit message; no improper Co-Authored-By usage for AI detected.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/load-vars-storage-plugin-order

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@maorfr

maorfr commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

connected e2e will be fixed in #469

@maorfr maorfr merged commit 62a6f74 into main Jun 10, 2026
25 of 28 checks passed
@maorfr maorfr deleted the fix/load-vars-storage-plugin-order branch June 10, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants