Downgrade CAPI operator - #942
Conversation
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
WalkthroughThis change refactors the Helm chart configuration and update process for the CAPI operator. It downgrades the operator version from 0.19.0 to 0.18.1, simplifies provider configuration formats from maps to colon- and semicolon-delimited strings, removes support for manifest patches and additional manifests, and adjusts Makefile and packaging scripts to support the new workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Helm
participant Chart Directory
User->>Makefile: make update
Makefile->>Makefile: clean (remove charts/cluster-api-operator/charts/)
Makefile->>Makefile: capi-operator-update
Makefile->>Helm: helm repo add / helm repo update
Makefile->>Helm: helm pull (with chart version)
Helm->>Chart Directory: Updates chart files
sequenceDiagram
participant User
participant Helm Chart
participant Values.yaml
User->>Helm Chart: Install/Upgrade with values.yaml
Helm Chart->>Values.yaml: Reads provider config (now string format)
Helm Chart->>Helm Chart: Parses colon/semicolon-delimited strings
Helm Chart->>Helm Chart: Renders Namespace and Provider resources
Assessment against linked issues
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (1)
8-23: Refactored parsing logic with improved error handlingThe template now splits each control plane entry by colons and provides different handling based on the number of arguments (1, 2, or 3), with sensible defaults for namespace when not explicitly provided. The explicit failure message helps users understand the expected format.
A minor formatting issue: there are some indentation and hyphen spacing inconsistencies in the template logic according to YAMLlint, but these won't affect functionality since they're within template directives.
Consider standardizing the indentation in template directives for better readability. For lines with two spaces before
{{-, either remove them or make them consistent throughout.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
[warning] 23-23: too many spaces after hyphen
(hyphens)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (1)
2-6: Validate and sanitize.Values.bootstrapinput
You’ve switched from a map to a semicolon-delimited string for bootstrap providers and initialized your variables cleanly. To harden this, consider trimming whitespace and filtering out any empty entries in case of trailing semicolons. For example, you could pipesplit ";" .Values.bootstrapthrough awhereto drop empty strings.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
packages/system/capi-operator/Makefile (2)
3-6: Pinning the chart version with a caret range
Using^0.18allows any0.18.xpatch, but the PR specifically downgraded to0.18.1. If you need to lock to exactly0.18.1, replace^0.18with0.18.1. Otherwise, this caret range will float to future0.18.*releases.
10-11: Makefileupdatetarget structure
Refactoringupdateto depend oncleanand the newcapi-operator-updatetarget is a nice modularization. Consider adding.PHONY: update clean capi-operator-updateat the top for clarity, and ensure the recipe line uses a literal tab (not spaces) to avoid Makefile parsing issues.packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (2)
2-6: Sanitize.Values.addonbefore splitting
The shift to a semicolon-delimited string is cleaner, but to avoid empty entries (e.g., trailing;), filter out empty strings after thesplit. This prevents accidental attempts to parse blank addons.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
53-58: Inconsistent naming:secretNamevsconfigSecret
Other provider templates use.Values.configSecret.name/.namespace, while addon uses.Values.secretName/.secretNamespace. For consistency and to reduce confusion, consider unifying this naming across all provider types.packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (1)
2-6: Sanitize.Values.infrastructureinput
As with other providers, filter out empty strings aftersplit ";" .Values.infrastructureto guard against trailing semicolons or accidental empty entries.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (2)
2-9: Consider sanitizing and validating the.Values.ipaminput
Splitting on";"and iterating directly may yield empty entries (e.g., trailing semicolon) or whitespace-only strings, which will in turn cause unexpected behavior in your range loop. Consider:
- Trimming whitespace around each segment (e.g.,
trim $ipam).- Filtering out empty strings before processing.
- Validating that each non-empty entry at least matches the minimal expected pattern.
For example, you could introduce a helper like this before the loop:
{{- $raw := split ";" .Values.ipam }} -{{- $ipams := $raw }} +{{- $ipams := where (list) $raw "@" "ne" "" | list }} # remove empty stringsand then inside the range trim each element:
{{- $ipam := trim ";" $ipam }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
22-23: Enhance the invalid-format error message
The currentfailstring only mentions two formats and omits the single-segment case, and doesn’t tell users which entry was invalid. Consider updating it to list all three supported patterns and include the offending value. For example:-{{- fail "ipam provider argument should have the following format in-cluster:v1.0.0 or mynamespace:in-cluster:v1.0.0" }} +{{- fail (printf "Invalid IPAM provider entry '%s': must be one of "+ + "<provider>, "+ + "<provider>:<version>, "+ + "<namespace>:<provider>:<version>" $ipam) }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml (1)
3048-3076: Enhance description clarity and enforce expected formats
The revised multi-line descriptions are more concise and user-friendly. To further strengthen schema validation and user guidance:
- Explicitly reference RFC3339 for
lastTransitionTimeto clarify the date-time format.- For
status, add anenumconstraint (["True","False","Unknown"]) or a regex pattern to enforce valid values.- Consider re-introducing length constraints (e.g.,
maxLength) onmessageandreasonif they are critical for downstream systems.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/system/capi-operator/Makefile(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml(11 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.schema.json(0 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.yaml(2 hunks)scripts/package.mk(1 hunks)
💤 Files with no reviewable changes (1)
- packages/system/capi-operator/charts/cluster-api-operator/values.schema.json
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: too many spaces after hyphen
(hyphens)
[warning] 14-14: wrong indentation: expected 0 but found 2
(indentation)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: too many spaces after hyphen
(hyphens)
[warning] 18-18: wrong indentation: expected 0 but found 2
(indentation)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 21-21: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: too many spaces after hyphen
(hyphens)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
[warning] 23-23: too many spaces after hyphen
(hyphens)
packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test
🔇 Additional comments (24)
packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml (1)
2-2:Details
✅ Verification successful
Consistent downgrade of CAPI operator to v0.18.1
The chart has been downgraded from 0.19.0 to 0.18.1, aligning with the image tag downgrade in values.yaml. This appears to be a deliberate version rollback, likely to address stability issues or compatibility requirements with other components.
Also applies to: 6-6
🏁 Script executed:
#!/bin/bash # Verify that both Chart.yaml and values.yaml use the same version echo "Checking version consistency between Chart.yaml and values.yaml..." CHART_VERSION=$(grep "appVersion:" packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml | awk '{print $2}') VALUES_VERSION=$(grep "tag:" packages/system/capi-operator/charts/cluster-api-operator/values.yaml | awk '{print $2}' | tr -d 'v') echo "Chart appVersion: $CHART_VERSION" echo "Values image tag: $VALUES_VERSION" if [ "$CHART_VERSION" = "$VALUES_VERSION" ]; then echo "✅ Versions are consistent." else echo "❌ Version mismatch detected!" fiLength of output: 619
Approve CAPI operator downgrade to v0.18.1
Both the Helm chart and the image tag have been aligned to v0.18.1, confirming the intentional rollback:
- packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml:
appVersion: 0.18.1- packages/system/capi-operator/charts/cluster-api-operator/values.yaml:
image: tag: v0.18.1✅ Versions are consistent.
packages/system/capi-operator/charts/cluster-api-operator/values.yaml (2)
4-9: Provider configuration format changed to string-basedThe configuration format for all CAPI providers has been changed from map-based to string-based. Empty strings now replace the empty maps, which aligns with the template refactoring in control-plane.yaml (and likely other provider templates).
Would it be helpful to add comments with examples of the new string format (e.g.,
# Format: "namespace:name:version") to make it clearer for users how to configure these values?
24-24: Consistent image tag downgrade to v0.18.1The image tag has been downgraded from v0.19.0 to v0.18.1, matching the chart version downgrade in Chart.yaml.
scripts/package.mk (2)
33-35: Added clean target for charts directoryThe clean target removes the charts directory, which is useful for refreshing charts before updating.
36-39:Details
✅ Verification successful
Added pattern rule for Helm chart updates
This pattern rule adds a generic mechanism to update Helm charts from repositories, which enhances reusability across packages. It requires environment variables
REPO_NAME,REPO_URL,CHART_NAME, andCHART_VERSIONto be set for proper operation.
🏁 Script executed:
#!/bin/bash # Check if this pattern rule is used in the capi-operator Makefile echo "Checking if the pattern rule is used in capi-operator Makefile..." grep -A5 "update:" packages/system/capi-operator/Makefile || echo "Pattern not found in capi-operator Makefile"Length of output: 277
Helm update pattern rule applied successfully
- Verified that
packages/system/capi-operator/Makefile’supdate: clean capi-operator-updatedependency invokes thecapi-operator-updatetarget, which matches the%-updatepattern inscripts/package.mk.- No changes required; the generic Helm chart update rule is in use.
packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (1)
2-7: Changed to string-based provider configurationThe template now parses a semicolon-separated string from
.Values.controlPlaneinstead of using a map-based configuration. This simplifies the input format but changes how users must structure their configuration.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (6)
7-13: Review loop and 3-part argument parsing
Therangeover$bootstrapsand theif eq … 3branch correctly assigns namespace, name, and version. This aligns with the goal of supportingnamespace:name:version. Good use of index variables (._0,._1,._2).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
14-17: Default namespace suffix for 2-part arguments
When onlyname:versionis provided, you append-bootstrap-systemto the namespace and use the name directly. This matches the pattern in other provider templates.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
18-20: Handle single-part argument (no version)
For the one-part case you set only namespace and name, leaving version empty. Be aware that downstream thespec:block is omitted unlessconfigSecretis provided. Verify that the CRD will accept an empty spec when no version is given, or consider defaulting to a known version.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
22-22: Clear usage error on invalid argument count
Thefailmessage is explicit and instructive, guiding users to the correct format (kubeadm:v1.0.0or with namespace).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
47-52: Conditionalspec:andversion:block
You conditionally emitspec:when a version or config secret is present, and only renderversion:if explicitly set. This keeps the YAML concise. Ensure this behavior is documented so users know when to include a version.
53-59: OptionalconfigSecretinjection
TheconfigSecretstanza is correctly gated on.Values.configSecret.name. The nested namespace block is only rendered if provided. This mirrors other provider templates.packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (4)
7-13: Parse three-part addon spec correctly
Theif eq … 3branch cleanly assigns namespace, name, and version. This matches expected inputnamespace:name:version.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
14-17: Default namespace suffix for two-part addons
Forname:versionentries you append-addon-systemto the namespace. This is consistent with other templates.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
18-22: Single-part and invalid argument handling
You handle one-part entries by defaulting namespace and name but omit version, and explicitlyfailon invalid counts. Ensure that omitting the version aligns with your desired fallback behavior (e.g., defaulting to latest).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
47-52: Conditionalspec:andversion:block for addon
Thespec:section only appears when a version orsecretNameis set, andversion:is only rendered if provided. This keeps the resource minimal.packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (4)
7-13: Parse three-part infrastructure spec
Theif eq … 3branch correctly assigns namespace, name, and version fornamespace:name:version.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
14-17: Default namespace suffix for two-part infra entries
Appending-infrastructure-systemis consistent with the pattern applied to other providers.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
18-20: Handle single-part infra entries
Defaulting namespace and name without version is okay if your CRD supports a default version. Verify downstream behavior whenversion:is omitted.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
22-22: Fail on invalid infra argument format
Thefailmessage is clear and instructs users on the correct input format (aws:v1.0.0or with a namespace).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 22-22: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (4)
9-13: Correct parsing of corenamespace:name:version
Theif eq … 3branch accurately assigns namespace, name, and version for three-part input, with sensible defaults.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: too many spaces after hyphen
(hyphens)
14-17: Default namespace for two-part core entries
Using"capi-system"for core providers is intentional and distinct from other providers. This specialization is clear and documented.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 14-14: wrong indentation: expected 0 but found 2
(indentation)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: too many spaces after hyphen
(hyphens)
18-20: Single-part core input handling
Defaulting to"capi-system"and setting name only is acceptable if the CRD will assume a default version. Ensure this behavior is documented.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 18-18: wrong indentation: expected 0 but found 2
(indentation)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
21-22: Fail on invalid core argument format
Thefailmessage clearly indicates the required format (cluster-api:v1.0.0or with a namespace).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 21-21: wrong indentation: expected 0 but found 2
(indentation)
[warning] 22-22: too many spaces after hyphen
(hyphens)
| {{- if .Values.core }} | ||
| {{- $coreArgs := split ":" .Values.core }} | ||
| {{- $coreArgsLen := len $coreArgs }} | ||
| {{- $coreVersion := "" }} | ||
| {{- $coreNamespace := "" }} | ||
| {{- $coreName := "" }} | ||
| {{- $coreVersion := "" }} |
There was a problem hiding this comment.
Remove duplicate $coreVersion declaration
You declare $coreVersion both on line 5 and again on line 8. The second declaration shadows the first—one of them should be removed to avoid confusion.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
| {{- if eq $ipamArgsLen 3 }} | ||
| {{- $ipamNamespace = $ipamArgs._0 }} | ||
| {{- $ipamName = $ipamArgs._1 }} | ||
| {{- $ipamVersion = $ipamArgs._2 }} | ||
| {{- else if eq $ipamArgsLen 2 }} | ||
| {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }} | ||
| {{- $ipamName = $ipamArgs._0 }} | ||
| {{- $ipamVersion = $ipamArgs._1 }} | ||
| {{- else if eq $ipamArgsLen 1 }} | ||
| {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }} | ||
| {{- $ipamName = $ipamArgs._0 }} | ||
| {{- else }} |
There was a problem hiding this comment.
Invalid element access on the split slice; use index instead
Helm/Go templates do not support property notation like $ipamArgs._0. To retrieve items from the slice you need the index function. Without this fix, the template will fail to render.
Apply a diff such as:
-{{- if eq $ipamArgsLen 3 }}
- {{- $ipamNamespace = $ipamArgs._0 }}
- {{- $ipamName = $ipamArgs._1 }}
- {{- $ipamVersion = $ipamArgs._2 }}
+{{- if eq $ipamArgsLen 3 }}
+ {{- $ipamNamespace = index $ipamArgs 0 }}
+ {{- $ipamName = index $ipamArgs 1 }}
+ {{- $ipamVersion = index $ipamArgs 2 }}
-{{- else if eq $ipamArgsLen 2 }}
- {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }}
- {{- $ipamName = $ipamArgs._0 }}
- {{- $ipamVersion = $ipamArgs._1 }}
+{{- else if eq $ipamArgsLen 2 }}
+ {{- $ipamNamespace = printf "%s-ipam-system" (index $ipamArgs 0) }}
+ {{- $ipamName = index $ipamArgs 0 }}
+ {{- $ipamVersion = index $ipamArgs 1 }}
-{{- else if eq $ipamArgsLen 1 }}
- {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }}
- {{- $ipamName = $ipamArgs._0 }}
+{{- else if eq $ipamArgsLen 1 }}
+ {{- $ipamNamespace = printf "%s-ipam-system" (index $ipamArgs 0) }}
+ {{- $ipamName = index $ipamArgs 0 }}
{{- else }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if eq $ipamArgsLen 3 }} | |
| {{- $ipamNamespace = $ipamArgs._0 }} | |
| {{- $ipamName = $ipamArgs._1 }} | |
| {{- $ipamVersion = $ipamArgs._2 }} | |
| {{- else if eq $ipamArgsLen 2 }} | |
| {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }} | |
| {{- $ipamName = $ipamArgs._0 }} | |
| {{- $ipamVersion = $ipamArgs._1 }} | |
| {{- else if eq $ipamArgsLen 1 }} | |
| {{- $ipamNamespace = print $ipamArgs._0 "-ipam-system" }} | |
| {{- $ipamName = $ipamArgs._0 }} | |
| {{- else }} | |
| {{- if eq $ipamArgsLen 3 }} | |
| {{- $ipamNamespace = index $ipamArgs 0 }} | |
| {{- $ipamName = index $ipamArgs 1 }} | |
| {{- $ipamVersion = index $ipamArgs 2 }} | |
| {{- else if eq $ipamArgsLen 2 }} | |
| {{- $ipamNamespace = printf "%s-ipam-system" (index $ipamArgs 0) }} | |
| {{- $ipamName = index $ipamArgs 0 }} | |
| {{- $ipamVersion = index $ipamArgs 1 }} | |
| {{- else if eq $ipamArgsLen 1 }} | |
| {{- $ipamNamespace = printf "%s-ipam-system" (index $ipamArgs 0) }} | |
| {{- $ipamName = index $ipamArgs 0 }} | |
| {{- else }} |
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 2
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
[warning] 14-14: too many spaces after hyphen
(hyphens)
[warning] 15-15: wrong indentation: expected 0 but found 2
(indentation)
[warning] 16-16: wrong indentation: expected 0 but found 2
(indentation)
[warning] 17-17: wrong indentation: expected 0 but found 2
(indentation)
[warning] 18-18: too many spaces after hyphen
(hyphens)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 2
(indentation)
Reverts #942 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for specifying manifest patches and additional manifests for all provider types, enabling more flexible customization. - Introduced an optional property to pass additional arguments to provider controller managers. - Added a JSON schema for validating chart values. - **Enhancements** - Provider configuration now uses structured maps instead of strings, simplifying customization and reducing errors. - Improved validation and descriptions for condition fields in resource schemas. - **Updates** - Upgraded Cluster API Operator chart and app versions to 0.19.0. - Updated default image tag for the manager container to v0.19.0. - **Documentation** - Added example configurations in the values file for easier setup. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Reverts #942 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for specifying manifest patches and additional manifests for all provider types, enabling more flexible customization. - Introduced an optional property to pass additional arguments to provider controller managers. - Added a JSON schema for validating chart values. - **Enhancements** - Provider configuration now uses structured maps instead of strings, simplifying customization and reducing errors. - Improved validation and descriptions for condition fields in resource schemas. - **Updates** - Upgraded Cluster API Operator chart and app versions to 0.19.0. - Updated default image tag for the manager container to v0.19.0. - **Documentation** - Added example configurations in the values file for easier setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit c54567a) Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Resolves #940.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores
Style