Skip to content

[WIP] Strict indentantion requirement#462

Open
rporres wants to merge 1 commit into
mainfrom
strict-yaml-check
Open

[WIP] Strict indentantion requirement#462
rporres wants to merge 1 commit into
mainfrom
strict-yaml-check

Conversation

@rporres

@rporres rporres commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

to fix the different styles and remove ignored warnings from yamllint

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened schema validation (operators, platforms, control binaries) to enforce required fields.
    • Improved operator readiness checks for more robust deployment polling.
    • Standardized YAML formatting across configs for consistency.
  • Chores

    • Excluded test fixtures from YAML linting.
    • Reformatted test fixtures to align with updated schemas.

@rporres rporres changed the title Strict indentantion requirement [WIP] Strict indentantion requirement Jun 9, 2026
@github-actions github-actions Bot added deployment Deployment-related changes infrastructure Infrastructure setup (VMs, networks) validation Validation and testing operators Operator installation/config plugins labels Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 96f93891-7275-4e91-9cca-cf199ed6d9dc

📥 Commits

Reviewing files that changed from the base of the PR and between 8a28559 and 39f19df.

📒 Files selected for processing (27)
  • .yamllint.yml
  • defaults/platforms.yaml
  • operators/advanced-cluster-management/tasks.yaml
  • operators/cincinnati-operator/tasks.yaml
  • operators/multicluster-engine/tasks.yaml
  • playbooks/tasks/configure_operator.yaml
  • plugins/lvms/tasks/quay.yaml
  • plugins/nvidia-gpu/files/20-placementrule.yaml
  • plugins/nvidia-gpu/files/30-placementbinding.yaml
  • plugins/odf/tasks/quay.yaml
  • plugins/openshift-ai/files/20-placementrule.yaml
  • plugins/openshift-ai/files/30-placementbinding.yaml
  • plugins/vast-csi/tasks/quay.yaml
  • schemas/catalogs.yaml
  • schemas/control_binaries.yaml
  • schemas/definitions.yaml
  • schemas/operators.yaml
  • schemas/platforms.yaml
  • schemas/plugin.yaml
  • test-fixtures/schemas/global/invalid/duplicated-network-config.yaml
  • test-fixtures/schemas/global/invalid/extra-ipAddress-while-NMState.yaml
  • test-fixtures/schemas/global/invalid/extra-macAddress-while-NMState.yaml
  • test-fixtures/schemas/global/invalid/extra-networkConfig-while-no-NMState.yaml
  • test-fixtures/schemas/global/invalid/invalid-mapInterfaces-macAddress.yaml
  • test-fixtures/schemas/global/invalid/malformed-mapInterfaces.yaml
  • test-fixtures/schemas/global/invalid/missing-mapInterfaces.yaml
  • test-fixtures/schemas/global/valid/networkConfig.yaml

Walkthrough

Enforces stricter YAML formatting by removing a lenient yamllint indentation rule, adds fixture paths to yamllint ignore, tightens multiple JSON-schema required lists, and normalizes YAML indentation across operator tasks, plugins, and test fixtures.

Changes

YAML Schema Validation & Indentation Standardization

Layer / File(s) Summary
Linting Configuration & Fixture Ignore Rules
.yamllint.yml
Deleted the explicit rules.indentation definition and added src/tests/fixtures/*/*.yaml to the top-level ignore list.
Schema Required Field Declarations
schemas/catalogs.yaml, schemas/control_binaries.yaml, schemas/operators.yaml, schemas/platforms.yaml, schemas/plugin.yaml
Added or corrected required declarations (e.g., control binaries url/checksum, operator csvNames, catalogs mirror_*_catalog, platforms openshift_versions/version, plugin csvNames dependency).
HostEntry Schema Conditional Logic Restructure
schemas/definitions.yaml
Reworked definitions.hostEntry oneOf/not/anyOf structure so networkConfig+mapInterfaces required grouping scopes not exclusions for ipAddress/macAddress.
Configuration Data Structure Indentation
defaults/platforms.yaml
Re-indented openshift_versions entries so each is - version: ... and default: true nests under the intended version item.
Operator & Plugin Task YAML Formatting
operators/*/tasks.yaml, playbooks/tasks/configure_operator.yaml, plugins/*/tasks.yaml, plugins/*/files/*.yaml
Normalized YAML indentation and list formatting across many operator and plugin task files; added defensive checks in MultiClusterHub readiness until predicates; restructured PlacementRule/PlacementBinding subjects and QuayRegistry component blocks without changing semantics.
Test Fixture YAML Normalization
test-fixtures/schemas/global/invalid/*.yaml, test-fixtures/schemas/global/valid/networkConfig.yaml
Re-indented and normalized networkConfig and mapInterfaces fixtures (valid and invalid cases) to match updated schema expectations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • maorfr

Poem

✨ Indents aligned, schema bells chime,

YAML tidy, validation prime.
Fixtures neat, rules set straight,
Linting stricter—no more fate.
A small PR, a cleaner state.

🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Ai-Attribution ⚠️ Warning PR uses AI-generated content for summaries but lacks required "Assisted-by" or "Generated-by" trailers in commit message for proper Red Hat attribution per policy. Add "Assisted-by: CodeRabbit" or "Generated-by" trailer to commit message to properly attribute AI-assisted work.
Title check ❓ Inconclusive The title contains a typo ('indentantion' instead of 'indentation') and is marked [WIP], making it unclear if this is ready for review. It accurately describes the main change regarding strict indentation, but the WIP status and typo undermine clarity. Fix the typo ('indentantion' → 'indentation') and remove [WIP] prefix if the PR is ready for merge, or clarify the work-in-progress status in the description instead.
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 found in 19 modified files. Comprehensive scan across APIs keys, tokens, passwords, credentials, base64 strings, PEM keys, and URLs found zero matches.
No-Weak-Crypto ✅ Passed PR contains only YAML formatting/schema changes. No weak crypto algorithms, custom crypto implementations, or unsafe secret comparisons found in any code.
No-Injection-Vectors ✅ Passed PR modifies only YAML config, schema, and test fixture files—no code execution patterns, injection vectors, or malicious patterns detected in any modified files.
Container-Privileges ✅ Passed PR only reformats YAML indentation. No container privilege settings (privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, runAsUser: 0) found in modified files.
No-Sensitive-Data-In-Logs ✅ Passed PR contains only YAML formatting/indentation changes with no new logging statements or code that could expose sensitive data like passwords, tokens, or API keys.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch strict-yaml-check

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 @.yamllint.yml:
- Line 59: The ignore glob "src/tests/fixtures/*/*.yaml" in .yamllint.yml does
not match the actual fixture paths (e.g., test-fixtures/...), so linting still
runs against those files; update the ignore entry by replacing the existing glob
"src/tests/fixtures/*/*.yaml" with a glob that matches the real fixture
directory structure (for example, use a pattern covering "test-fixtures" and
nested subfolders) so fixture YAMLs are properly excluded from yamllint.

In `@playbooks/tasks/configure_operator.yaml`:
- Around line 138-139: The readiness conditions currently index
r_subscription.resources[0] directly which will raise an error if resources is
empty; update each check (the occurrences referencing
r_subscription.resources[0].status.installplan) to first assert
r_subscription.resources is defined and has length > 0, then inspect
resources[0].status.installplan—e.g. change the when/tests to combine a guard
like r_subscription.resources | length > 0 with the subsequent checks (or use a
safe default/filter) so the installplan checks only run when resources[0]
exists; apply the same guarded pattern to the other occurrences reported (the
checks around lines 174-175 and 190-192).

In `@plugins/vast-csi/tasks/quay.yaml`:
- Around line 112-113: Replace the ansible.builtin.shell task that runs "{{
workingDir }}/bin/oc patch ..." with ansible.builtin.command using argv so the
command and each argument are passed without shell evaluation; specifically
update the task that currently uses ansible.builtin.shell to call oc (the binary
referenced as {{ workingDir }}/bin/oc) and provide each token as a separate argv
element including 'patch', 'deployment', 'registry-quay-app', '-n',
'quay-enterprise', '--type=json' and the '-p' value using the templated '{{
quay_vast_patch | to_json }}' string as a single argv entry, ensuring no shell
piping or interpolation is used and retaining the same arguments and behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d7568324-befe-4b22-89dc-341a00f75ac7

📥 Commits

Reviewing files that changed from the base of the PR and between b69cc60 and 8a28559.

📒 Files selected for processing (27)
  • .yamllint.yml
  • defaults/platforms.yaml
  • operators/advanced-cluster-management/tasks.yaml
  • operators/cincinnati-operator/tasks.yaml
  • operators/multicluster-engine/tasks.yaml
  • playbooks/tasks/configure_operator.yaml
  • plugins/lvms/tasks/quay.yaml
  • plugins/nvidia-gpu/files/20-placementrule.yaml
  • plugins/nvidia-gpu/files/30-placementbinding.yaml
  • plugins/odf/tasks/quay.yaml
  • plugins/openshift-ai/files/20-placementrule.yaml
  • plugins/openshift-ai/files/30-placementbinding.yaml
  • plugins/vast-csi/tasks/quay.yaml
  • schemas/catalogs.yaml
  • schemas/control_binaries.yaml
  • schemas/definitions.yaml
  • schemas/operators.yaml
  • schemas/platforms.yaml
  • schemas/plugin.yaml
  • test-fixtures/schemas/global/invalid/duplicated-network-config.yaml
  • test-fixtures/schemas/global/invalid/extra-ipAddress-while-NMState.yaml
  • test-fixtures/schemas/global/invalid/extra-macAddress-while-NMState.yaml
  • test-fixtures/schemas/global/invalid/extra-networkConfig-while-no-NMState.yaml
  • test-fixtures/schemas/global/invalid/invalid-mapInterfaces-macAddress.yaml
  • test-fixtures/schemas/global/invalid/malformed-mapInterfaces.yaml
  • test-fixtures/schemas/global/invalid/missing-mapInterfaces.yaml
  • test-fixtures/schemas/global/valid/networkConfig.yaml

Comment thread .yamllint.yml
Comment thread playbooks/tasks/configure_operator.yaml
Comment thread plugins/vast-csi/tasks/quay.yaml
to fix the different styles and remove ignored warnings from yamllint

Signed-off-by: Rafa Porres Molina <rporresm@redhat.com>
@rporres rporres force-pushed the strict-yaml-check branch from 8a28559 to 39f19df Compare June 10, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment Deployment-related changes infrastructure Infrastructure setup (VMs, networks) operators Operator installation/config plugins validation Validation and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant