Revert "Downgrade CAPI operator" - #946
Conversation
WalkthroughThis update transitions the Helm chart templates for the cluster-api-operator from string-based configuration to structured map-based configuration for all provider types. It introduces support for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant ChartTemplates
participant Kubernetes
User->>Helm: Install/upgrade cluster-api-operator chart with values.yaml
Helm->>ChartTemplates: Render templates using values.yaml (map-based config)
ChartTemplates->>Kubernetes: Create Provider resources (Core, Bootstrap, Infra, etc.)
alt If manifestPatches or additionalManifests present
ChartTemplates->>Kubernetes: Create ConfigMap(s) for additionalManifests
end
Kubernetes-->>User: Provider resources and ConfigMaps deployed
Poem
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: 5
♻️ Duplicate comments (3)
packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml (3)
2846-2852: Duplicate: Removal ofadditionalArgsalso required in these segments
These hunks mirror the sameadditionalArgsadditions across multiple provider specs and should likewise be reverted to align with the PR’s revert objective.Also applies to: 6074-6080, 7609-7619, 10842-10849, 12380-12387, 15609-15616, 17147-17154, 20377-20384, 21915-21922, 23507-23514, 25045-25052, 26638-26645, 28176-28183
4735-4760: Duplicate: Revert schema constraint hunk
These segments duplicate the schema tightening above across several condition definitions and must be restored to their original state.Also applies to: 7828-7857, 9499-9528, 12597-12625, 14269-14297, 17365-17393, 19034-19062, 22133-22161, 25262-25290, 28394-28422
4767-4772: Duplicate: Remove type field length constraints
These hunks replicate thetypefield length constraints in multiple occurrences and should be reverted to satisfy the revert-only scope.Also applies to: 7866-7868, 9540-9541, 12636-12637, 14305-14306, 17401-17404, 19072-19074, 22170-22172, 25299-25301, 28431-28433
🧹 Nitpick comments (8)
packages/system/capi-operator/Makefile (1)
6-11: Review and harden theupdatetarget
Theupdatetarget now removes the entirechartsdirectory and pulls the chart fresh. To improve robustness:
- Use
helm repo add capi-operator ... --force-updateto avoid errors if the repo already exists.- Declare
updateas a.PHONYtarget to prevent conflicts with anyupdatefile.- Confirm that wiping
charts/won’t delete unrelated artifacts.
Finally, the pipeline failure cites an error in theimagetarget for the MySQL app—please verify that other inherited targets fromscripts/package.mkremain functional after this change.packages/system/capi-operator/charts/cluster-api-operator/values.schema.json (1)
1-47: Define schema for provider configuration and extras
This JSON Schema covers top-level provider maps (core,bootstrap, etc.) but doesn't restrict additional top-level fields or nested fields likemanifestPatches,additionalManifests,manager, etc. Consider:
- Adding
"additionalProperties": falseor explicitly listing other supported properties to prevent typos.- Defining schemas for nested structures (
manifestPatches,additionalManifests, etc.) to validate their expected shape.- Removing redundant
"type": "object"at line 12 (sinceoneOfalready enforces object|null).packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (1)
55-57: Ensure correct indentation formanifestPatches
ThemanifestPatchesblock is currently nindented by 4 spaces; verify that it aligns under thespec:section in the rendered YAML. Ifspec:is indented two spaces, you may need to increase the indentation, e.g.,| nindent 6, to maintain valid YAML structure.packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (1)
52-61: Generate ConfigMap for additionalManifests.
Creating a separateConfigMapfor extra manifests ensures clear separation. As an enhancement, consider adding anownerReferencesblock pointing to the BootstrapProvider custom resource to enable automatic cleanup on deletion.packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (1)
61-69: Separate ConfigMap for core additionalManifests.
Creating thisConfigMapis consistent with other providers. Optionally, add anownerReferencessection to tie it to the CoreProvider for automated cleanup.packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (1)
78-87: Create ConfigMap for infra additionalManifests.
Consistent pattern with other providers. As an improvement, consider setting anownerReferencesentry to the InfrastructureProvider CR for proper lifecycle management.packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (1)
51-60: Provision ConfigMap for addon additionalManifests.
Creating aConfigMaphere follows the pattern of other providers. Optionally, you can addownerReferenceslinking back to the AddonProvider for cleanup.packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (1)
65-74: Generate ConfigMap for control-plane additionalManifests.
Consistent with other provider templates. For enhanced resource hygiene, consider adding anownerReferencesto the ControlPlaneProvider CR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 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(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml(2 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(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml(36 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.schema.json(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.yaml(2 hunks)scripts/package.mk(0 hunks)
💤 Files with no reviewable changes (1)
- scripts/package.mk
🧰 Additional context used
🪛 GitHub Actions: Pull Request
packages/system/capi-operator/Makefile
[error] 13-13: Make build failed due to error in image target for mysql app.
🪛 YAMLlint (1.35.1)
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] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: 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] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: 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] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (20)
packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml (2)
2-2: Confirm version bump consistency
TheappVersionwas updated to0.19.0. Ensure this aligns with the upstream Cluster API Operator release and that downstream deployments reference this version.
6-6: Confirm chart version increment
The Helm chart version was incremented to0.19.0. Verify that all dependent charts or consumers (e.g., umbrella charts) are updated to reference this new chart version.packages/system/capi-operator/charts/cluster-api-operator/values.yaml (2)
4-24: Migrate provider configs to structured maps
Switching from empty strings to{}enables nested provider configuration. Ensure the templates iterate safely over these maps when keys are unset (e.g., usingdefaultorhasKey). Also verify thatvalues.schema.jsondeclares these top-level properties to allow object values.
42-42: Update operator image tag
The manager image tag was bumped tov0.19.0. Confirm that corresponding container images are available in the registry and that any downstream automation or documentation is updated to reference this new tag.packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (1)
61-67: ValidateadditionalManifestsspec fields
The provider spec references onlynameandnamespaceforadditionalManifestsbut omits the actualmanifestslist. Confirm that the IPAMProvider CRD expects a ConfigMap reference rather than embedding the manifests directly. If direct embedding is supported, consider includingmanifestsunder the spec instead of only generating the ConfigMap.packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (3)
2-5: Structured iteration for bootstrap providers is applied correctly.
The migration from a semicolon- and colon-separated string to iterating over a structured map (.Values.bootstrap) improves readability and maintainability.
Please verify thatvalues.yamlandvalues.schema.jsonhave been updated to declare.bootstrapas a map with the newnamespace,version,manifestPatches, andadditionalManifestsfields.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
42-44: Add support for manifest patches in BootstrapProvider spec.
The conditional rendering of.manifestPatcheswithtoYaml | nindent 4is correct and maintains YAML structure. Ensure the JSON schema (values.schema.json) includes validation for themanifestPatchesarray.
45-50: RenderadditionalManifestsmetadata within the BootstrapProvider spec.
Good use of defaulting and conditional namespace inclusion for additional manifests. Confirm thatvalues.yamldefines theadditionalManifestsobject and the schema reflectsname,namespace, andmanifestsproperties.packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (3)
2-5: Transition to structured map for core providers.
Iterating over.Values.corestreamlines the template and removes brittle string parsing. Please check thatvalues.yamlandvalues.schema.jsonare updated to definecoreas a map with the expected nested fields.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
51-53: Include manifest patches in CoreProvider spec.
The use oftoYaml | nindent 4under themanifestPatcheskey is correct. Ensure the JSON schema includesmanifestPatchesunder each core entry.
54-60: Render additionalManifests configuration inside the CoreProvider spec.
Nice conditional block foradditionalManifestswith name and optional namespace. Verifyvalues.yamland the schema definition foradditionalManifests.packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (3)
2-5: Switch infra provider config to a structured map.
The refactoring from positional parsing to iterating over.Values.infrastructureis clear and maintainable. Please confirm updates invalues.yamlandvalues.schema.jsonfor the newinfrastructuremap.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
68-70: ApplymanifestPatchesto InfrastructureProvider spec.
Good use oftoYaml | nindent 4under the conditional block. Ensure the schema file definesmanifestPatchesfor infrastructure entries.
71-77: AddadditionalManifestsspec for infrastructure.
This block correctly rendersnameand optionalnamespace. Verifyvalues.yamland JSON schema reflect these new fields.packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (3)
2-5: Adopt structured map for addon providers.
Moving from a delimited string to map iteration over.Values.addonenhances clarity. Confirm thatvalues.yamlandvalues.schema.jsonwere updated to declareaddonas an object map.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
41-43: EmbedmanifestPatchesin AddonProvider spec.
The conditional block withtoYaml | nindent 4is correct. Make sure the JSON schema includesmanifestPatchesunder each addon entry.
44-50: RenderadditionalManifestsin AddonProvider spec.
Well-structured conditional fornameand optionalnamespace. Verify corresponding schema and defaults invalues.yaml.packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (3)
2-5: Iterate over structured.Values.controlPlanemap.
This update removes error-prone string parsing and makes the template more declarative. Please ensurevalues.yamland the JSON schema definecontrolPlaneas an object map.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
55-57: InjectmanifestPatchesinto ControlPlaneProvider spec.
Correct use oftoYaml | nindent 4within the conditional. Verify that the schema includesmanifestPatchesfor control plane entries.
58-64: SpecifyadditionalManifestsin ControlPlaneProvider spec.
The conditional block is accurately rendered withnameand optionalnamespace. Confirm thatvalues.yamland schema are updated accordingly.
| {{- range $name, $ipam := $.Values.ipam }} | ||
| {{- $ipamNamespace := default ( printf "%s-%s" $name "ipam-system" ) (get $ipam "namespace") }} | ||
| {{- $ipamName := $name }} | ||
| {{- $ipamVersion := get $ipam "version" }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard iteration over potentially null values
Since ipam can be null per the JSON schema, iterating directly with range may cause a runtime error when Values.ipam is null. Wrap with a guard or default to an empty map, e.g.:
{{- with default dict .Values.ipam }}
{{- range $name, $ipam := . }}
…
{{- end }}
{{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
🤖 Prompt for AI Agents
In packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml
lines 2 to 5, the code iterates directly over $.Values.ipam which can be null,
causing runtime errors. To fix this, wrap the iteration with a guard using
'with' and 'default dict' to ensure the value is always a map, like '{{- with
default dict .Values.ipam }}' before the range loop and close it after the loop
with '{{- end }}'.
| {{- if $ipam.additionalManifests }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $ipam.additionalManifests.name }} | ||
| namespace: {{ default $ipamNamespace $ipam.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $ipam.additionalManifests.manifests | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor ConfigMap data.manifests to a literal block
ConfigMap data values must be strings; dumping an array/map via toYaml may render invalid types. Use a literal block for manifests:
data:
- manifests: {{- toYaml $ipam.additionalManifests.manifests | nindent 4 }}
+ manifests: |
+{{ toYaml $ipam.additionalManifests.manifests | indent 4 }}📝 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 $ipam.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $ipam.additionalManifests.name }} | |
| namespace: {{ default $ipamNamespace $ipam.additionalManifests.namespace }} | |
| data: | |
| manifests: {{- toYaml $ipam.additionalManifests.manifests | nindent 4 }} | |
| {{- end }} | |
| {{- if $ipam.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $ipam.additionalManifests.name }} | |
| namespace: {{ default $ipamNamespace $ipam.additionalManifests.namespace }} | |
| data: | |
| manifests: | | |
| {{ toYaml $ipam.additionalManifests.manifests | indent 4 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml
around lines 68 to 77, the ConfigMap data field `manifests` is rendered using
`toYaml` which may produce invalid types since ConfigMap data values must be
strings. Refactor the `manifests` field to use a YAML literal block scalar
(using the `|` character) to ensure the entire YAML content is treated as a
string. Adjust the indentation accordingly to maintain valid YAML structure.
| additionalArgs: | ||
| additionalProperties: | ||
| type: string | ||
| description: |- | ||
| AdditionalArgs is a map of additional options that will be passed | ||
| in as container args to the provider's controller manager. | ||
| type: object |
There was a problem hiding this comment.
Mismatch with PR Objective: Unexpected addition of additionalArgs
The PR is intended to revert a previous CAPI operator change, yet this hunk introduces a new additionalArgs map under the provider manager spec. These feature additions are out-of-scope for a revert and will break backward compatibility. Please remove these lines to restore the original template.
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml
around lines 1308 to 1314, remove the newly added `additionalArgs` map under the
provider manager spec. This addition is out-of-scope for the revert PR and
breaks backward compatibility. Revert this section to match the original
template before the change.
| maxLength: 256 | ||
| minLength: 1 | ||
| type: string |
There was a problem hiding this comment.
Mismatch with PR Objective: Added type field length constraints
The introduction of maxLength/minLength on the condition type field is a breaking schema change that doesn’t belong in a revert PR. Remove these lines to return to the prior template.
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml
around lines 3100 to 3102, remove the added maxLength and minLength constraints
on the condition type field to revert the schema to its previous state, as these
length constraints introduce a breaking change not suitable for a revert PR.
| lastTransitionTime is the last time the condition transitioned from one status to another. | ||
| This should be when the underlying condition changed. If that is not known, then using the time when | ||
| the API field changed is acceptable. | ||
| format: date-time | ||
| type: string | ||
| message: | ||
| description: |- | ||
| A human readable message indicating details about the transition. | ||
| message is a human readable message indicating details about the transition. | ||
| This field may be empty. | ||
| maxLength: 10240 | ||
| minLength: 1 | ||
| type: string | ||
| reason: | ||
| description: |- | ||
| The reason for the condition's last transition in CamelCase. | ||
| reason is the reason for the condition's last transition in CamelCase. | ||
| The specific API may choose whether or not this field is considered a guaranteed API. | ||
| This field may be empty. | ||
| maxLength: 256 | ||
| minLength: 1 | ||
| type: string | ||
| severity: | ||
| description: |- | ||
| severity provides an explicit classification of Reason code, so the users or machines can immediately | ||
| understand the current situation and act accordingly. | ||
| The Severity field MUST be set only when Status=False. | ||
| maxLength: 32 | ||
| type: string |
There was a problem hiding this comment.
Mismatch with PR Objective: Schema constraints tightened
This hunk enhances condition schema validation by adding minLength/maxLength on message, reason, and severity. These refinements introduce breaking changes and are unrelated to reverting the operator downgrade. Please revert to the previous schema definition.
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml
between lines 3065 and 3091, the schema validation for condition fields message,
reason, and severity was tightened by adding minLength and maxLength
constraints, which introduces breaking changes unrelated to the intended
operator downgrade revert. To fix this, revert the schema definition for these
fields back to the previous version without the minLength and maxLength
constraints to maintain compatibility and align with the PR objective.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
8ff2cb9 to
8ad010d
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (2)
2-5: Guard iteration over potentially null.Values.ipam
Since$.Values.ipamcan benullper the values schema, iterating directly may cause a runtime error. Wrap the range with a default empty map, for example:{{- with default dict .Values.ipam }} {{- range $name, $ipam := . }} … {{- end }} {{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
68-77: Refactor ConfigMapdata.manifeststo a literal block scalar
ConfigMapdatavalues must be strings. Rendering the manifests viatoYamldirectly may produce non-string types. Use a literal block to ensure valid YAML:data: - manifests: {{- toYaml $ipam.additionalManifests.manifests | nindent 4 }} + manifests: | +{{ toYaml $ipam.additionalManifests.manifests | indent 4 }}
🧹 Nitpick comments (4)
packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (1)
2-5: Consider guarding iteration over.Values.addon
For consistency and safety, wrap the range in a default empty map in case.Values.addonisnull:{{- with default dict .Values.addon }} {{- range $name, $addon := . }} … {{- end }} {{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (1)
2-5: Consider guarding iteration over.Values.core
Wrap the range in a default map to avoid issues if.Values.coreisnull:{{- with default dict .Values.core }} {{- range $name, $core := . }} … {{- end }} {{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (1)
2-5: Consider guarding iteration over.Values.bootstrap
To prevent errors when.Values.bootstrapisnull, wrap the range with a default empty map:{{- with default dict .Values.bootstrap }} {{- range $name, $bootstrap := . }} … {{- end }} {{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (1)
2-5: Consider guarding iteration over.Values.controlPlane
Wrap the range in a default empty map to handle cases where.Values.controlPlaneisnull:{{- with default dict .Values.controlPlane }} {{- range $name, $controlPlane := . }} … {{- end }} {{- end }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (11)
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(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml(2 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(2 hunks)packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml(36 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.schema.json(1 hunks)packages/system/capi-operator/charts/cluster-api-operator/values.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/system/capi-operator/Makefile
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/system/capi-operator/charts/cluster-api-operator/Chart.yaml
- packages/system/capi-operator/charts/cluster-api-operator/values.yaml
- packages/system/capi-operator/charts/cluster-api-operator/values.schema.json
- packages/system/capi-operator/charts/cluster-api-operator/templates/operator-components.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: 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] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: 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] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: 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 (11)
packages/system/capi-operator/charts/cluster-api-operator/templates/ipam.yaml (2)
55-57: Manifest patches rendering looks good
The conditional inclusion ofmanifestPatchesusingtoYamlandnindentcorrectly injects the patch list into theIPAMProviderspec.
61-67: AdditionalManifests metadata in spec is correct
Including only thenameand optionalnamespaceof the additional manifests in the provider spec aligns with other provider templates.packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml (2)
41-43: Manifest patches rendering looks good
Conditionally injectingmanifestPatchesusingtoYamlandnindentcorrectly adds the patch list to theAddonProviderspec.
44-49: AdditionalManifests metadata in spec is correct
The spec section correctly includes thenameand optionalnamespacefields for additional manifests, matching the pattern used elsewhere.packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml (2)
51-52: Manifest patches rendering looks good
UsingtoYamlandnindentunder the conditional to injectmanifestPatchesinto theCoreProviderspec is correct.
54-60: AdditionalManifests metadata in spec is correct
The spec block correctly includesnameand optionalnamespacefor additional manifests.packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml (2)
42-44: Manifest patches rendering looks good
The use oftoYamlwithnindentunder the conditional correctly populatesmanifestPatchesin theBootstrapProviderspec.
45-51: AdditionalManifests metadata in spec is correct
This block properly includes thenameand conditionalnamespacefields for additional manifests.packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml (2)
55-57: Manifest patches rendering looks good
Conditionally addingmanifestPatchesviatoYamlandnindentis correct for theControlPlaneProviderspec.
58-64: AdditionalManifests metadata in spec is correct
Includingnameand optionalnamespacefields for additional manifests aligns with the pattern used for other providers.packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml (1)
2-5: Structured iteration setup looks correctThe new
rangeover$.Values.infrastructureand local assignments for namespace, name, and version follow the same pattern as other provider templates. This change simplifies configuration parsing and is implemented correctly.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
| {{- if $addon.additionalManifests }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $addon.additionalManifests.name }} | ||
| namespace: {{ default $addonNamespace $addon.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $addon.additionalManifests.manifests | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor ConfigMap data.manifests to a literal block scalar
Ensure the manifests are stored as a string in the ConfigMap:
data:
- manifests: {{- toYaml $addon.additionalManifests.manifests | nindent 4 }}
+ manifests: |
+{{ toYaml $addon.additionalManifests.manifests | indent 4 }}📝 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 $addon.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $addon.additionalManifests.name }} | |
| namespace: {{ default $addonNamespace $addon.additionalManifests.namespace }} | |
| data: | |
| manifests: {{- toYaml $addon.additionalManifests.manifests | nindent 4 }} | |
| {{- end }} | |
| {{- if $addon.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $addon.additionalManifests.name }} | |
| namespace: {{ default $addonNamespace $addon.additionalManifests.namespace }} | |
| data: | |
| manifests: | | |
| {{ toYaml $addon.additionalManifests.manifests | indent 4 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/addon.yaml
around lines 51 to 60, the ConfigMap's data.manifests field currently uses
toYaml which may not preserve the manifests as a literal string. Refactor this
to use a literal block scalar (using |) to store the manifests as a string,
ensuring proper formatting and preservation of newlines in the ConfigMap.
| {{- if $core.additionalManifests }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $core.additionalManifests.name }} | ||
| namespace: {{ default $coreNamespace $core.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $core.additionalManifests.manifests | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor ConfigMap data.manifests to a literal block scalar
ConfigMap data values must be strings. Update to:
data:
- manifests: {{- toYaml $core.additionalManifests.manifests | nindent 4 }}
+ manifests: |
+{{ toYaml $core.additionalManifests.manifests | indent 4 }}📝 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 $core.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $core.additionalManifests.name }} | |
| namespace: {{ default $coreNamespace $core.additionalManifests.namespace }} | |
| data: | |
| manifests: {{- toYaml $core.additionalManifests.manifests | nindent 4 }} | |
| {{- end }} | |
| {{- if $core.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $core.additionalManifests.name }} | |
| namespace: {{ default $coreNamespace $core.additionalManifests.namespace }} | |
| data: | |
| manifests: | | |
| {{ toYaml $core.additionalManifests.manifests | indent 4 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In packages/system/capi-operator/charts/cluster-api-operator/templates/core.yaml
around lines 61 to 70, the ConfigMap's data.manifests field is currently
rendered using toYaml which may not produce a proper string value. To fix this,
refactor the manifests field to use a literal block scalar (|) so that the YAML
content is treated as a string literal. This ensures the ConfigMap data value is
a valid string as required.
| {{- if $bootstrap.additionalManifests }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $bootstrap.additionalManifests.name }} | ||
| namespace: {{ default $bootstrapNamespace $bootstrap.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $bootstrap.additionalManifests.manifests | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor ConfigMap data.manifests to a literal block scalar
Ensure the manifests list is treated as a string:
data:
- manifests: {{- toYaml $bootstrap.additionalManifests.manifests | nindent 4 }}
+ manifests: |
+{{ toYaml $bootstrap.additionalManifests.manifests | indent 4 }}📝 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 $bootstrap.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $bootstrap.additionalManifests.name }} | |
| namespace: {{ default $bootstrapNamespace $bootstrap.additionalManifests.namespace }} | |
| data: | |
| manifests: {{- toYaml $bootstrap.additionalManifests.manifests | nindent 4 }} | |
| {{- end }} | |
| {{- if $bootstrap.additionalManifests }} | |
| --- | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ $bootstrap.additionalManifests.name }} | |
| namespace: {{ default $bootstrapNamespace $bootstrap.additionalManifests.namespace }} | |
| data: | |
| manifests: | | |
| {{ toYaml $bootstrap.additionalManifests.manifests | indent 4 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/bootstrap.yaml
lines 52 to 61, the ConfigMap data.manifests field should be refactored to use a
literal block scalar (|) to ensure the manifests list is treated as a string.
Replace the current inline YAML rendering with a literal block scalar followed
by the indented YAML content to preserve formatting and avoid parsing issues.
| {{- if $controlPlane.additionalManifests }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $controlPlane.additionalManifests.name }} | ||
| namespace: {{ default $controlPlaneNamespace $controlPlane.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $controlPlane.additionalManifests.manifests | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor ConfigMap data.manifests to a literal block scalar
Convert the YAML array to a string in the ConfigMap:
data:
- manifests: {{- toYaml $controlPlane.additionalManifests.manifests | nindent 4 }}
+ manifests: |
+{{ toYaml $controlPlane.additionalManifests.manifests | indent 4 }}🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/control-plane.yaml
around lines 65 to 74, the ConfigMap's data.manifests field currently uses
toYaml which outputs a YAML array, but it should be refactored to use a literal
block scalar to properly represent the manifests as a string. Replace the toYaml
function with a pipe (|) followed by the manifests content indented correctly to
format it as a literal block scalar string in the ConfigMap.
| {{- if $infra.manifestPatches }} | ||
| manifestPatches: {{- toYaml $infra.manifestPatches | nindent 4 }} | ||
| {{- end }} {{/* if $infra.manifestPatches */}} |
There was a problem hiding this comment.
Spec guard missing for new fields
The newly added manifestPatches and additionalManifests fields live under the spec: block, but the existing if or … guard (around line 29) doesn’t include them. If an infrastructure entry only defines patches or additional manifests, spec: won’t render, resulting in invalid YAML.
Proposed update to include the two new fields in the guard:
-{{- if or $infrastructureVersion $.Values.configSecret.name $.Values.manager $.Values.additionalDeployments }}
+{{- if or $infrastructureVersion $.Values.configSecret.name $.Values.manager $.Values.additionalDeployments $infra.manifestPatches $infra.additionalManifests }}Also applies to: 71-77
🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml
around lines 29 and 68-77, the conditional guard controlling the rendering of
the spec block does not include the new fields manifestPatches and
additionalManifests. To fix this, update the existing if or condition near line
29 to also check for the presence of $infra.manifestPatches and
$infra.additionalManifests, ensuring the spec block renders whenever these
fields are defined. This prevents invalid YAML when only these new fields are
present.
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ $infra.additionalManifests.name }} | ||
| namespace: {{ default $infrastructureNamespace $infra.additionalManifests.namespace }} | ||
| data: | ||
| manifests: {{- toYaml $infra.additionalManifests.manifests | nindent 4 }} |
There was a problem hiding this comment.
Ensure ConfigMap data.manifests is a string
Kubernetes expects each data value in a ConfigMap to be a string. Currently, the template embeds the YAML list directly:
data:
manifests:
- …
- …This will fail schema validation for map[string]string. Wrap the rendered YAML in a block scalar so it’s stored as a single string:
- manifests: {{- toYaml $infra.additionalManifests.manifests | nindent 4 }}
+ manifests: |-
{{ toYaml $infra.additionalManifests.manifests | indent 4 }}🤖 Prompt for AI Agents
In
packages/system/capi-operator/charts/cluster-api-operator/templates/infra.yaml
around lines 79 to 86, the ConfigMap's data.manifests field is currently
rendered as a YAML list, which violates Kubernetes' requirement that ConfigMap
data values must be strings. To fix this, wrap the output of toYaml
$infra.additionalManifests.manifests in a block scalar (e.g., using the pipe
symbol |) so that the entire YAML content is treated as a single string value
under manifests.
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>
Reverts #942
Summary by CodeRabbit
New Features
Enhancements
Updates
Documentation