(k8s) configure containerd for client k8s cluster - #979
Conversation
|
""" WalkthroughThis update splits the monolithic Cluster API providers Helm chart into four separate charts for bootstrap, core, control plane, and infrastructure providers. It introduces new Helm chart files, manifests, and metadata for each subcomponent. Additionally, it adds support for using a custom secret to patch containerd in the Kubernetes chart and updates related documentation and configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes API
participant Secret Store
User->>Helm: Deploy Kubernetes chart with values
Helm->>Kubernetes API: Check for <release>-patch-containerd secret
alt useCustomSecretForPatchContainerd is false
Helm->>Secret Store: Lookup patch-containerd in cozy-system
alt Secret exists
Helm->>Kubernetes API: Create <release>-patch-containerd secret in release namespace
end
end
Helm->>Kubernetes API: Deploy/Update node group with containerd patch logic
Kubernetes API->>Node: Mount certs and run preKubeadmCommands if secret present
sequenceDiagram
participant User
participant Helm
participant K8s API
User->>Helm: Deploy paas-full bundle
Helm->>K8s API: Install capi-providers-bootstrap
Helm->>K8s API: Install capi-providers-core
Helm->>K8s API: Install capi-providers-cpprovider
Helm->>K8s API: Install capi-providers-infraprovider
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (33)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (30)
🧰 Additional context used🪛 YAMLlint (1.37.1)packages/apps/kubernetes/templates/copy-patch-containerd.yaml[error] 1-1: syntax error: expected the node content, but found '-' (syntax) ⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
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 (
|
2e7b34d to
235710e
Compare
235710e to
60a800c
Compare
60a800c to
9a2aa1e
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (11)
packages/apps/kubernetes/README.md (1)
89-89: Consistent description formatting
Consider capitalizing the description and adding a period foruseCustomSecretForPatchContainerdto align with other entries:-| `useCustomSecretForPatchContainerd` | if true, for patch containerd will be used secret: {{ .Release.Name }}-patch-containerd | `false` | +| `useCustomSecretForPatchContainerd` | If true, a custom secret named `{{ .Release.Name }}-patch-containerd` will be used for patching containerd. | `false` |packages/apps/kubernetes/templates/cluster.yaml (1)
232-232: Remove unnecessarysudoin preKubeadmCommands.
PreKubeadm commands run as root;sudois redundant and may not be available in the minimal init environment.packages/apps/kubernetes/templates/copy-patch-containerd.yaml (1)
1-15: Secret copy template correctly mirrors upstream secret.
This block conditionally clones thepatch-containerdsecret into{{ .Release.Namespace }}whenuseCustomSecretForPatchContainerdis unset or false, satisfying the PR objective.Consider adding a note in the chart README reminding users that when
useCustomSecretForPatchContainerdis true, they must provision the custom secret manually in the release namespace.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/capi-providers-cpprovider/templates/providers.yaml (2)
15-21: Consistent resource value quoting
Consider quoting all unit-suffixed values for consistency (e.g.,memory: "128Mi"alongsidememory: "1024Mi").
5-7: Parameterize provider version via Helm values
Hardcodingversion: v0.15.1-cpwill require chart edits on each release. You can simplify upgrades by templating this value:-version: v0.15.1-cp +version: {{ .Values.provider.version }}packages/system/capi-providers-bootstrap/templates/configmaps.yaml (2)
4-5: Consider templating the ConfigMap name
Hardcodingname: v1.10.1-bootstrapties the manifest to a specific version. You could derive this from chart metadata:name: {{ printf "%s-%s-bootstrap" .Chart.Name .Values.version }}
6-8: Add standard Helm labels
For improved consistency, consider including common Helm labels inmetadata.labels, e.g.:labels: app.kubernetes.io/name: {{ include "capi-providers-bootstrap.name" . }} helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version }} ...packages/system/capi-providers-bootstrap/templates/providers.yaml (1)
5-8: Parameterize provider version in values.yaml
To streamline future upgrades, you can templateversion: v1.10.1-bootstrapinvalues.yamlinstead of hardcoding:version: {{ .Values.provider.version }}packages/system/capi-providers-core/templates/configmaps.yaml (1)
3-8: Consider parameterizing the ConfigMap name
Hard-codingname: v1.10.1-coremay hinder chart reuse. Use a Helm template helper (e.g.,{{ include "fullname" . }}or{{ .Release.Name }}) for dynamic naming.packages/system/capi-providers-cpprovider/templates/configmaps.yaml (1)
3-8: Parameterize the ConfigMap name in Helm
Replace the staticname: v0.15.1-cpwith a template expression (e.g.,{{ .Release.Name }}or{{ include "fullname" . }}) to improve flexibility.packages/system/capi-providers-infraprovider/templates/configmaps.yaml (1)
1-14: Parameterize versioned ConfigMap name and mark as immutable
Hardcoding the version (v0.1.10-infraprovider) will require manual bumps on every release. You can derive the name from the chart metadata and.Chart.Version, and—since this object is always versioned—mark it asimmutable: trueto reduce etcd churn.Example diff:
metadata: - name: v0.1.10-infraprovider + name: {{ include "cozy-capi-providers-infraprovider.fullname" . }}-{{ .Chart.Version }} labels: infraprovider-components: cozy annotations: provider.cluster.x-k8s.io/compressed: "true" +immutable: true binaryData: components: {{ .Files.Get "files/components.gz" | b64enc | quote }} data: metadata: |- {{ .Files.Get "files/metadata.yaml" | indent 4 }}🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 14-14: syntax error: could not find expected ':'
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/system/capi-providers-bootstrap/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-core/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-cpprovider/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-infraprovider/files/components.gzis excluded by!**/*.gz
📒 Files selected for processing (32)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/README.md(1 hunks)packages/apps/kubernetes/templates/cluster.yaml(1 hunks)packages/apps/kubernetes/templates/copy-patch-containerd.yaml(1 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/core/platform/bundles/paas-full.yaml(1 hunks)packages/system/capi-providers-bootstrap/.helmignore(1 hunks)packages/system/capi-providers-bootstrap/Chart.yaml(1 hunks)packages/system/capi-providers-bootstrap/files/metadata.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/providers.yaml(1 hunks)packages/system/capi-providers-core/.helmignore(1 hunks)packages/system/capi-providers-core/Chart.yaml(1 hunks)packages/system/capi-providers-core/Makefile(1 hunks)packages/system/capi-providers-core/files/metadata.yaml(1 hunks)packages/system/capi-providers-core/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-core/templates/providers.yaml(1 hunks)packages/system/capi-providers-cpprovider/.helmignore(1 hunks)packages/system/capi-providers-cpprovider/Chart.yaml(1 hunks)packages/system/capi-providers-cpprovider/Makefile(1 hunks)packages/system/capi-providers-cpprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers-infraprovider/.helmignore(1 hunks)packages/system/capi-providers-infraprovider/Chart.yaml(1 hunks)packages/system/capi-providers-infraprovider/Makefile(1 hunks)packages/system/capi-providers-infraprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers/templates/providers.yaml(0 hunks)
💤 Files with no reviewable changes (1)
- packages/system/capi-providers/templates/providers.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (28)
packages/system/capi-providers-cpprovider/.helmignore (1)
1-1: Exclude component YAML files from Helm package
This pattern matches and omits all*-components.yamlfiles underfiles/, keeping the chart archive clean and consistent with sibling charts.packages/apps/kubernetes/Chart.yaml (1)
19-19: Chart version bump
The chart version has been correctly incremented from0.21.0to0.23.0to reflect the new parameter additions and templating logic.packages/apps/versions_map (1)
69-70: Pin previous version and add new mapping
The entry for version0.21.0is now pinned to commit6130f43dfor reproducibility, and a new mapping for0.23.0pointing toHEADhas been added.packages/apps/kubernetes/values.schema.json (1)
130-134: Add schema property for custom secret flag
The new boolean propertyuseCustomSecretForPatchContainerdwith a default offalseis correctly defined in the JSON schema, enabling validation of the custom secret toggle.packages/apps/kubernetes/values.yaml (1)
6-10: Define default foruseCustomSecretForPatchContainerd
The values file now includes theuseCustomSecretForPatchContainerd: falseparameter with corresponding@paramdocumentation, matching the schema and enabling users to toggle the custom secret behavior.packages/apps/kubernetes/README.md (1)
84-85: Introduce table header for parameters
The Markdown table header and separators have been added to list the common parameters in tabular form, enhancing readability.packages/system/capi-providers-core/Chart.yaml (1)
2-2: Chart name updated correctly
Renaming the chart tocozy-capi-providers-corealigns with the modular decomposition of the monolithic provider.packages/system/capi-providers-cpprovider/Chart.yaml (1)
2-2: Chart name defined properly
The chartcozy-capi-providers-cpproviderfollows the new split and naming conventions.packages/system/capi-providers-bootstrap/Chart.yaml (1)
2-2: Chart name defined properly
The chartcozy-capi-providers-bootstrapcorrectly reflects the bootstrap provider component.packages/system/capi-providers-infraprovider/Chart.yaml (1)
2-2: Chart name defined properly
The chartcozy-capi-providers-infraprovidercorrectly reflects the infrastructure provider component.packages/system/capi-providers-infraprovider/Makefile (1)
1-4: Makefile structure looks correct.Exporting
NAMEandNAMESPACEand including the sharedscripts/package.mkmatches the pattern of the other decomposed provider packages. No changes required here.packages/system/capi-providers-core/Makefile (1)
1-4: Makefile template is consistent.This Makefile correctly exports
NAMEandNAMESPACEand includes the common build logic inscripts/package.mk, aligning with its sibling packages. No action needed.packages/system/capi-providers-cpprovider/Makefile (1)
1-4: Makefile template is consistent.Matches the established pattern for the other provider subpackages by exporting
NAME/NAMESPACEand includingscripts/package.mk. All good here.packages/system/capi-providers-cpprovider/files/metadata.yaml (1)
1-53: Release series metadata appears correct.The mapping of each Kubernetes minor version to
v1beta1is accurate, and the header comments clearly explain update procedures. No issues detected.packages/system/capi-providers-cpprovider/templates/providers.yaml (1)
1-11: Provider CRD manifest looks correct
TheControlPlaneProviderresource for "kamaji" follows the expected API version, kind, metadata, and selector conventions.packages/system/capi-providers-core/files/metadata.yaml (1)
6-41: Metadata mapping is accurate
TheMetadataresource correctly lists Kubernetes 1.0 through 1.10 release series mapped tov1beta1.packages/system/capi-providers-bootstrap/templates/configmaps.yaml (2)
1-8: ConfigMap manifest is correct
TheConfigMapincludes both compressedbinaryDataand the uncompressedmetadatablock using Helm’s file functions.
9-13: Binary and data sections usage is valid
Embeddingcomponents.gzvia.Files.Get | b64encand inliningmetadata.yamlmatches the pattern used across other provider charts.packages/system/capi-providers-bootstrap/templates/providers.yaml (2)
1-4: BootstrapProvider manifest is well-formed
TheBootstrapProviderresource for "kubeadm" correctly defines the API, kind, and metadata.
9-11: Verify label consistency
Ensure that thematchLabels: bootstrap-components: cozyselector matches the labels defined in the bootstrapConfigMap(templates/configmaps.yaml) to avoid fetchConfig mismatches.packages/system/capi-providers-core/templates/providers.yaml (2)
1-12: PR objective mismatch
The PR title and objectives mention configuring containerd for a client Kubernetes cluster, but this diff focuses on splitting Cluster API providers into separate charts. Please confirm that the PR description has been updated to reflect this provider decomposition change.
1-12: CoreProvider manifest is valid
TheCoreProviderresource is correctly defined withapiVersion: operator.cluster.x-k8s.io/v1alpha2, versionv1.10.1-core, and selectorcore-components: cozy.packages/system/capi-providers-bootstrap/files/metadata.yaml (1)
1-41: Bootstrap Metadata mapping is comprehensive
TheMetadataresource accurately maps Kubernetes versions1.0through1.10tov1beta1, and the header comments clearly outline update instructions for future minor releases.packages/system/capi-providers-infraprovider/templates/providers.yaml (1)
1-11: InfrastructureProvider manifest is properly defined
TheInfrastructureProviderresource usesversion: v0.1.10-infraproviderand selects components labeledinfraprovider-components: cozy.packages/system/capi-providers-core/templates/configmaps.yaml (1)
9-13: Verify metadata indentation
Ensure that{{ .Files.Get "files/metadata.yaml" | indent 4 }}correctly indents underdata.metadatain the rendered manifest.packages/system/capi-providers-cpprovider/templates/configmaps.yaml (1)
3-13:Details
❓ Verification inconclusive
Ensure label consistency with ControlPlaneProvider
Verify that the ControlPlaneProvider template uses the same label keycp-components: cozyunderfetchConfig.selector.matchLabels.
🏁 Script executed:
#!/bin/bash # Verify label consistency for cp-components in the ControlPlaneProvider manifest rg -A3 "kind: ControlPlaneProvider" -n packages/system/capi-providers-cpprovider/templates/providers.yamlLength of output: 173
🏁 Script executed:
#!/bin/bash # Search for fetchConfig and selector.matchLabels in the ControlPlaneProvider template rg -n "fetchConfig" packages/system/capi-providers-cpprovider/templates/providers.yaml rg -n "selector" packages/system/capi-providers-cpprovider/templates/providers.yaml rg -n "matchLabels" packages/system/capi-providers-cpprovider/templates/providers.yamlLength of output: 312
#!/bin/bash # Display the matchLabels section in the ControlPlaneProvider manifest to verify the labels rg -n -A5 -B2 "matchLabels" packages/system/capi-providers-cpprovider/templates/providers.yaml
#!/bin/bash # Show context around fetchConfig and matchLabels in ControlPlaneProvider manifest rg -n -C5 "fetchConfig" packages/system/capi-providers-cpprovider/templates/providers.yaml
#!/bin/bash # Display 2 lines before and 4 lines after matchLabels in the ControlPlaneProvider manifest rg -n -B2 -A4 "matchLabels:" packages/system/capi-providers-cpprovider/templates/providers.yaml
Ensure label consistency between ConfigMap and ControlPlaneProvider
Please verify that thespec.fetchConfig.selector.matchLabelsblock in
packages/system/capi-providers-cpprovider/templates/providers.yamlincludes the same
cp-components: cozykey used in your ConfigMap (configmaps.yaml). If it’s missing, add it as shown:spec: fetchConfig: selector: matchLabels: - # other labels… + cp-components: cozyThis ensures the provider will correctly select the ConfigMap by its label.
packages/core/platform/bundles/paas-full.yaml (2)
287-293: New bootstrap provider release entry
This introducescapi-providers-bootstrapwith the correct releaseName, chart, namespace, and dependencies. Ensure that thecozy-capi-providers-bootstrapHelm chart is published and contains the expected CRDs and controllers.
294-300: New core provider release entry
Thecapi-providers-corerelease is correctly defined. Please confirm thatcozy-capi-providers-coreis available and its CRDs/implementations align with the expected Cluster API core contract.
| @@ -0,0 +1 @@ | |||
| files/.*-components.yaml | |||
There was a problem hiding this comment.
Fix glob pattern syntax to correctly ignore component YAML files.
The pattern files/.*-components.yaml treats the dot (.) literally and will only match filenames beginning with . (e.g., .foo-components.yaml). Helm’s .helmignore uses shell-style globs, not regex. To ignore all *-components.yaml files in the files/ directory, update the pattern as follows:
- files/.*-components.yaml
+ files/*-components.yaml📝 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.
| files/.*-components.yaml | |
| files/*-components.yaml |
🤖 Prompt for AI Agents
In packages/system/capi-providers-bootstrap/.helmignore at line 1, the glob
pattern 'files/.*-components.yaml' incorrectly uses a dot that matches only
filenames starting with a literal dot. Replace the pattern with
'files/*-components.yaml' to correctly ignore all files ending with
'-components.yaml' in the 'files/' directory using shell-style glob syntax.
| @@ -0,0 +1 @@ | |||
| files/.*-components.yaml | |||
There was a problem hiding this comment.
Incorrect .helmignore glob pattern
Helm ignore patterns use glob syntax, not regex. The pattern files/.*-components.yaml only matches files prefixed with a literal dot. To ignore all *-components.yaml under files/, replace it with:
-files/.*-components.yaml
+files/*-components.yaml📝 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.
| files/.*-components.yaml | |
| files/*-components.yaml |
🤖 Prompt for AI Agents
In packages/system/capi-providers-core/.helmignore at line 1, the pattern uses
regex syntax which is incorrect for .helmignore files that require glob
patterns. Replace the pattern `files/.*-components.yaml` with a proper glob
pattern like `files/*-components.yaml` to correctly ignore all files ending with
`-components.yaml` under the `files/` directory.
| @@ -0,0 +1 @@ | |||
| files/.*-components.yaml | |||
There was a problem hiding this comment.
Incorrect ignore pattern for component files.
Helm’s .helmignore uses glob syntax (like .gitignore), so the pattern files/.*-components.yaml only matches filenames beginning with a literal dot (e.g., .foo-components.yaml). To ignore all component manifests under files/, update it as follows:
-files/.*-components.yaml
+files/*-components.yamlThis change ensures files/bootstrap-components.yaml, files/core-components.yaml, etc., are excluded from the chart package.
📝 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.
| files/.*-components.yaml | |
| files/*-components.yaml |
🤖 Prompt for AI Agents
In packages/system/capi-providers-infraprovider/.helmignore at line 1, the
ignore pattern files/.*-components.yaml incorrectly matches only files starting
with a dot. Update the pattern to files/*-components.yaml to correctly ignore
all component manifest files like files/bootstrap-components.yaml and
files/core-components.yaml.
| releaseSeries: | ||
| - major: 0 | ||
| minor: 1 | ||
| contract: v1beta1 |
There was a problem hiding this comment.
Release series mapping appears incorrect
The metadata currently maps only major: 0, minor: 1, which doesn’t align with supported Kubernetes versions (e.g., 1.x). Please verify and update the infraprovider metadata to include the correct release series (e.g., major: 1, minor: X) matching your supported Kubernetes versions.
🤖 Prompt for AI Agents
In packages/system/capi-providers-infraprovider/files/metadata.yaml around lines
8 to 11, the releaseSeries mapping incorrectly uses major version 0 and minor 1,
which does not match supported Kubernetes versions. Update the releaseSeries to
use the correct major version 1 and the appropriate minor version(s) that
correspond to the supported Kubernetes versions for this infraprovider.
| - name: capi-providers-cpprovider | ||
| releaseName: capi-providers-cpprovider | ||
| chart: cozy-capi-providers-cpprovider | ||
| namespace: cozy-cluster-api | ||
| privileged: true | ||
| dependsOn: [cilium,kubeovn,capi-operator] | ||
|
|
There was a problem hiding this comment.
Missing dependency on core provider for control-plane CRDs
The cozy-capi-providers-cpprovider chart installs ControlPlaneProvider CRDs which extend core API types. It must wait for capi-providers-core to be deployed. Update the dependsOn array accordingly.
Apply this diff:
- dependsOn: [cilium,kubeovn,capi-operator]
+ dependsOn: [cilium,kubeovn,capi-operator,capi-providers-core]📝 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.
| - name: capi-providers-cpprovider | |
| releaseName: capi-providers-cpprovider | |
| chart: cozy-capi-providers-cpprovider | |
| namespace: cozy-cluster-api | |
| privileged: true | |
| dependsOn: [cilium,kubeovn,capi-operator] | |
| - name: capi-providers-cpprovider | |
| releaseName: capi-providers-cpprovider | |
| chart: cozy-capi-providers-cpprovider | |
| namespace: cozy-cluster-api | |
| privileged: true | |
| dependsOn: [cilium,kubeovn,capi-operator,capi-providers-core] |
🤖 Prompt for AI Agents
In packages/core/platform/bundles/paas-full.yaml around lines 301 to 307, the
dependsOn array for the capi-providers-cpprovider chart is missing the core
provider dependency. Add capi-providers-core to the dependsOn list to ensure the
core provider is deployed before capi-providers-cpprovider, maintaining proper
installation order for ControlPlaneProvider CRDs.
| - name: capi-providers-infraprovider | ||
| releaseName: capi-providers-infraprovider | ||
| chart: cozy-capi-providers-infraprovider | ||
| namespace: cozy-cluster-api | ||
| privileged: true | ||
| dependsOn: [cilium,kubeovn,capi-operator] |
There was a problem hiding this comment.
Missing dependency on core provider for infrastructure CRDs
The cozy-capi-providers-infraprovider chart defines InfrastructureProvider CRDs that rely on core API CRDs. Include capi-providers-core in the dependency list to ensure correct ordering.
Apply this diff:
- dependsOn: [cilium,kubeovn,capi-operator]
+ dependsOn: [cilium,kubeovn,capi-operator,capi-providers-core]📝 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.
| - name: capi-providers-infraprovider | |
| releaseName: capi-providers-infraprovider | |
| chart: cozy-capi-providers-infraprovider | |
| namespace: cozy-cluster-api | |
| privileged: true | |
| dependsOn: [cilium,kubeovn,capi-operator] | |
| - name: capi-providers-infraprovider | |
| releaseName: capi-providers-infraprovider | |
| chart: cozy-capi-providers-infraprovider | |
| namespace: cozy-cluster-api | |
| privileged: true | |
| - dependsOn: [cilium,kubeovn,capi-operator] | |
| + dependsOn: [cilium,kubeovn,capi-operator,capi-providers-core] |
🤖 Prompt for AI Agents
In packages/core/platform/bundles/paas-full.yaml around lines 308 to 313, the
dependency list for the capi-providers-infraprovider chart is missing the core
provider dependency. Add capi-providers-core to the dependsOn array to ensure
the core API CRDs are installed before the infrastructure CRDs, maintaining
correct installation order.
9a2aa1e to
6515782
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml (2)
1-1: Silence YAMLLint false positive
The syntax error reported by YAMLLint at line 1 is due to the Helm templating directive ({{-). These are not valid YAML until the chart is rendered. Please configure your linter to skip or ignore Go-template directives in this directory to avoid spurious complaints.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
4-9: Add a document separator and use consistent root context
- It’s best practice to prepend
---before each Kubernetes resource to ensure clean separation when multiple templates are concatenated.- For consistency (and to guard against future nesting), reference the release name with
{{ $.Release.Name }}rather than{{ .Release.Name }}.apiVersion: v1 kind: Secret +--- metadata: - name: {{ .Release.Name }}-patch-containerd + name: {{ $.Release.Name }}-patch-containerd namespace: {{ $.Release.Namespace }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/system/capi-providers-bootstrap/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-core/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-cpprovider/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-infraprovider/files/components.gzis excluded by!**/*.gz
📒 Files selected for processing (33)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/README.md(1 hunks)packages/apps/kubernetes/templates/cluster.yaml(1 hunks)packages/apps/kubernetes/templates/copy-patch-containerd.yaml(1 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/core/platform/bundles/paas-full.yaml(1 hunks)packages/system/capi-providers-bootstrap/.helmignore(1 hunks)packages/system/capi-providers-bootstrap/Chart.yaml(1 hunks)packages/system/capi-providers-bootstrap/files/metadata.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/providers.yaml(1 hunks)packages/system/capi-providers-core/.helmignore(1 hunks)packages/system/capi-providers-core/Chart.yaml(1 hunks)packages/system/capi-providers-core/Makefile(1 hunks)packages/system/capi-providers-core/files/metadata.yaml(1 hunks)packages/system/capi-providers-core/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-core/templates/providers.yaml(1 hunks)packages/system/capi-providers-cpprovider/.helmignore(1 hunks)packages/system/capi-providers-cpprovider/Chart.yaml(1 hunks)packages/system/capi-providers-cpprovider/Makefile(1 hunks)packages/system/capi-providers-cpprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers-infraprovider/.helmignore(1 hunks)packages/system/capi-providers-infraprovider/Chart.yaml(1 hunks)packages/system/capi-providers-infraprovider/Makefile(1 hunks)packages/system/capi-providers-infraprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers/templates/providers.yaml(0 hunks)scripts/migrations/13(1 hunks)
💤 Files with no reviewable changes (1)
- packages/system/capi-providers/templates/providers.yaml
✅ Files skipped from review due to trivial changes (4)
- scripts/migrations/13
- packages/system/capi-providers-core/templates/configmaps.yaml
- packages/system/capi-providers-bootstrap/templates/providers.yaml
- packages/system/capi-providers-infraprovider/templates/configmaps.yaml
🚧 Files skipped from review as they are similar to previous changes (27)
- packages/apps/kubernetes/Chart.yaml
- packages/system/capi-providers-bootstrap/.helmignore
- packages/system/capi-providers-core/files/metadata.yaml
- packages/system/capi-providers-core/.helmignore
- packages/system/capi-providers-infraprovider/Makefile
- packages/system/capi-providers-cpprovider/.helmignore
- packages/apps/kubernetes/values.schema.json
- packages/system/capi-providers-bootstrap/Chart.yaml
- packages/system/capi-providers-infraprovider/.helmignore
- packages/system/capi-providers-core/Chart.yaml
- packages/system/capi-providers-cpprovider/Makefile
- packages/apps/kubernetes/values.yaml
- packages/system/capi-providers-infraprovider/files/metadata.yaml
- packages/system/capi-providers-cpprovider/templates/providers.yaml
- packages/system/capi-providers-core/Makefile
- packages/apps/kubernetes/README.md
- packages/system/capi-providers-bootstrap/templates/configmaps.yaml
- packages/system/capi-providers-infraprovider/Chart.yaml
- packages/system/capi-providers-cpprovider/files/metadata.yaml
- packages/system/capi-providers-bootstrap/files/metadata.yaml
- packages/system/capi-providers-infraprovider/templates/providers.yaml
- packages/system/capi-providers-cpprovider/Chart.yaml
- packages/system/capi-providers-core/templates/providers.yaml
- packages/apps/versions_map
- packages/apps/kubernetes/templates/cluster.yaml
- packages/system/capi-providers-cpprovider/templates/configmaps.yaml
- packages/core/platform/bundles/paas-full.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (1)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml (1)
1-3: Verify Helmlookupusage and version compatibility
You’re using thelookupfunction (introduced in Helm 3.2.0) to fetch thepatch-containerdSecret. Make sure yourChart.yamlor documentation enforces a minimum Helm version (≥ 3.2.0) so this function is always available for your users.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
| data: | ||
| {{- range $key, $value := $sourceSecret.data }} | ||
| {{ $key }}: {{ $value | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve YAML structure when iterating secret data
Using {{- range … }} at the start of the line will trim the newline after data: and could collapse entries into the same line, invalidating the manifest. Instead, retain the newline and leverage Helm’s indent helper for correct alignment:
data:
-{{- range $key, $value := $sourceSecret.data }}
- {{ $key }}: {{ $value | quote }}
-{{- end }}
+{{- range $key, $value := $sourceSecret.data }}
+{{ printf "%s: %s" $key ($value | quote) | indent 2 }}
+{{- end }}This guarantees each key: "value" pair is properly indented under data:.
📝 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.
| data: | |
| {{- range $key, $value := $sourceSecret.data }} | |
| {{ $key }}: {{ $value | quote }} | |
| {{- end }} | |
| data: | |
| {{- range $key, $value := $sourceSecret.data }} | |
| {{ printf "%s: %s" $key ($value | quote) | indent 2 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In packages/apps/kubernetes/templates/copy-patch-containerd.yaml around lines 10
to 13, the use of `{{- range ... }}` trims the newline after `data:`, causing
YAML entries to collapse into the same line and break the manifest structure. To
fix this, remove the dash after `{{` in the range statement to preserve the
newline, and use Helm's `indent` function to properly indent each `key: "value"`
pair under `data:` ensuring valid YAML formatting.
505e9b2 to
f9930ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml (2)
2-3: Consider surfacing a warning when the source Secret is missing
Ifpatch-containerddoesn’t exist incozy-system, the template silently skips creation. You may want to log a warning (e.g., via{{ fail }}in a dry-run or a Helm hook) to alert users that no Secret was found.
4-9: Preserve source Secret’s metadata
Currently only.typeand.dataare copied. Consider also replicating importantlabels/annotations(or adding anownerReference) from the original Secret to maintain context and facilitate cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/system/capi-providers-bootstrap/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-core/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-cpprovider/files/components.gzis excluded by!**/*.gzpackages/system/capi-providers-infraprovider/files/components.gzis excluded by!**/*.gz
📒 Files selected for processing (33)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/README.md(1 hunks)packages/apps/kubernetes/templates/cluster.yaml(1 hunks)packages/apps/kubernetes/templates/copy-patch-containerd.yaml(1 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/core/platform/bundles/paas-full.yaml(1 hunks)packages/system/capi-providers-bootstrap/.helmignore(1 hunks)packages/system/capi-providers-bootstrap/Chart.yaml(1 hunks)packages/system/capi-providers-bootstrap/files/metadata.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-bootstrap/templates/providers.yaml(1 hunks)packages/system/capi-providers-core/.helmignore(1 hunks)packages/system/capi-providers-core/Chart.yaml(1 hunks)packages/system/capi-providers-core/Makefile(1 hunks)packages/system/capi-providers-core/files/metadata.yaml(1 hunks)packages/system/capi-providers-core/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-core/templates/providers.yaml(1 hunks)packages/system/capi-providers-cpprovider/.helmignore(1 hunks)packages/system/capi-providers-cpprovider/Chart.yaml(1 hunks)packages/system/capi-providers-cpprovider/Makefile(1 hunks)packages/system/capi-providers-cpprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-cpprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers-infraprovider/.helmignore(1 hunks)packages/system/capi-providers-infraprovider/Chart.yaml(1 hunks)packages/system/capi-providers-infraprovider/Makefile(1 hunks)packages/system/capi-providers-infraprovider/files/metadata.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/configmaps.yaml(1 hunks)packages/system/capi-providers-infraprovider/templates/providers.yaml(1 hunks)packages/system/capi-providers/templates/providers.yaml(0 hunks)scripts/migrations/13(1 hunks)
💤 Files with no reviewable changes (1)
- packages/system/capi-providers/templates/providers.yaml
✅ Files skipped from review due to trivial changes (2)
- scripts/migrations/13
- packages/system/capi-providers-infraprovider/templates/configmaps.yaml
🚧 Files skipped from review as they are similar to previous changes (29)
- packages/system/capi-providers-cpprovider/.helmignore
- packages/system/capi-providers-core/Chart.yaml
- packages/apps/kubernetes/Chart.yaml
- packages/system/capi-providers-infraprovider/Makefile
- packages/apps/versions_map
- packages/system/capi-providers-core/Makefile
- packages/apps/kubernetes/README.md
- packages/apps/kubernetes/values.schema.json
- packages/system/capi-providers-bootstrap/.helmignore
- packages/system/capi-providers-core/.helmignore
- packages/system/capi-providers-bootstrap/Chart.yaml
- packages/system/capi-providers-infraprovider/.helmignore
- packages/system/capi-providers-infraprovider/Chart.yaml
- packages/system/capi-providers-cpprovider/Chart.yaml
- packages/system/capi-providers-cpprovider/Makefile
- packages/system/capi-providers-core/files/metadata.yaml
- packages/system/capi-providers-cpprovider/files/metadata.yaml
- packages/apps/kubernetes/templates/cluster.yaml
- packages/system/capi-providers-bootstrap/templates/configmaps.yaml
- packages/system/capi-providers-cpprovider/templates/providers.yaml
- packages/system/capi-providers-core/templates/providers.yaml
- packages/system/capi-providers-bootstrap/templates/providers.yaml
- packages/apps/kubernetes/values.yaml
- packages/system/capi-providers-core/templates/configmaps.yaml
- packages/system/capi-providers-infraprovider/templates/providers.yaml
- packages/system/capi-providers-infraprovider/files/metadata.yaml
- packages/system/capi-providers-bootstrap/files/metadata.yaml
- packages/system/capi-providers-cpprovider/templates/configmaps.yaml
- packages/core/platform/bundles/paas-full.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (1)
packages/apps/kubernetes/templates/copy-patch-containerd.yaml (1)
1-3: Require Helm v3.1+ forlookupsupport
Thelookupfunction was introduced in Helm 3.1.0. Please confirm that yourChart.yamlusesapiVersion: v2and specifies a minimum Helm version of>= 3.1.0so this template will render correctly.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
| data: | ||
| {{- range $key, $value := $sourceSecret.data }} | ||
| {{ printf "%s: %s" $key ($value | quote) | indent 2 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Fix missing newline after data:
Using {{- range … }} on line 11 removes the newline after data:, collapsing the first data entry onto the same line. Remove the dash before range and before end so each key/value pair is on its own line:
data:
-{{- range $key, $value := $sourceSecret.data }}
- {{ printf "%s: %s" $key ($value | quote) | indent 2 }}
-{{- end }}
+{{ range $key, $value := $sourceSecret.data }}
+ {{ printf "%s: %s" $key ($value | quote) | indent 2 }}
+{{ end }}📝 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.
| data: | |
| {{- range $key, $value := $sourceSecret.data }} | |
| {{ printf "%s: %s" $key ($value | quote) | indent 2 }} | |
| {{- end }} | |
| data: | |
| {{ range $key, $value := $sourceSecret.data }} | |
| {{ printf "%s: %s" $key ($value | quote) | indent 2 }} | |
| {{ end }} |
🤖 Prompt for AI Agents
In packages/apps/kubernetes/templates/copy-patch-containerd.yaml around lines 10
to 13, the use of {{- range ... }} and {{- end }} removes the newline after the
'data:' key, causing the first data entry to be on the same line as 'data:'. To
fix this, remove the dashes before 'range' and 'end' so that the template
preserves the newline after 'data:' and each key/value pair appears on its own
line.
There was a problem hiding this comment.
| data: | |
| {{- range $key, $value := $sourceSecret.data }} | |
| {{ printf "%s: %s" $key ($value | quote) | indent 2 }} | |
| {{- end }} | |
| data: {{- toYaml $sourceSecret.data | nindent 2 }} |
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) Excellent suggestion! Using toYaml with nindent is much cleaner and more idiomatic than the manual iteration approach. This completely resolves the newline issue I flagged and is the proper way to handle YAML data serialization in Helm templates.
Your approach:
- Leverages Helm's built-in YAML functions
- Handles proper indentation automatically
- Is more concise and maintainable
- Avoids the formatting complexity of the current code
This is definitely the better solution. Thank you for the improvement!
f9930ef to
f10f845
Compare
Summary by CodeRabbit
New Features
Documentation
Chores
Refactor