Enable user-added params in tenant cluster Cilium #917
Conversation
WalkthroughThis change introduces a new configuration section for the Cilium CNI plugin within the Kubernetes Helm chart. It centralizes default Cilium values in a reusable Helm template and allows users to override these defaults via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Cilium HelmRelease
User->>Helm Chart: Provide .Values.addons.cilium.valuesOverride (optional)
Helm Chart->>Helm Chart: Merge valuesOverride with defaultCiliumValues
Helm Chart->>Cilium HelmRelease: Apply merged Cilium values
Cilium HelmRelease->>Kubernetes: Deploy Cilium with final configuration
Suggested labels
Suggested reviewers
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: 0
🧹 Nitpick comments (1)
packages/apps/kubernetes/values.yaml (1)
45-49: Add Cilium addon configuration placeholder
A newciliumblock withvaluesOverride: {}has been introduced underaddons, following the pattern established by other plugins. This gives users a hook to supply custom Helm values for the Cilium deployment.
Consider whether an explicitenabledflag should be added for consistency with other addons, or document that Cilium is always installed by design.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/cilium.yaml(2 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/kubernetes/templates/helmreleases/cilium.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: Build and Test
🔇 Additional comments (5)
packages/apps/kubernetes/Chart.yaml (1)
19-19: Chart version bump to 0.20.0
Theversionfield has been correctly incremented from0.19.0to0.20.0to reflect the new Cilium addon feature. Ensure that any downstream automation, release notes, or documentation referencing the previous chart version are updated accordingly.packages/apps/versions_map (1)
62-63: Pinned previous version and added new version
The entry forkubernetes 0.19.0is now pinned to commit93bdf411, and a new0.20.0version is mapped toHEAD, aligning perfectly with the chart bump. This maintains reproducible deployments for existing users while allowing the new release to track the latest code.packages/apps/kubernetes/values.schema.json (1)
148-157: Schema for Cilium addon valuesOverride
The JSON schema now includes aciliumobject underaddonswith avaluesOverrideproperty defaulting to an empty object, mirroring the YAML defaults.
Please verify that omitting anenabledboolean is intentional, as it diverges from the pattern used by other addons.packages/apps/kubernetes/templates/helmreleases/cilium.yaml (2)
1-8: Introduce reusable defaultCiliumValues template
Thecozystack.defaultCiliumValuesHelm template centralizes all default Cilium configuration values, reducing duplication and streamlining future updates. This is a solid application of Helm library chart best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
43-43: Merge user overrides with default Cilium values
LeveragingdeepCopy,mergeOverwrite, andtoYamlensures that.Values.addons.cilium.valuesOverridetakes precedence over the built-in defaults. Make sure to runhelm lintorhelm templateacross various override scenarios to confirm the output is syntactically valid YAML.
3633cf7 to
5f84246
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/apps/kubernetes/templates/helmreleases/cilium.yaml (1)
1-8: Suppress YAMLlint false positives on Helm templates
YAMLlint is flagging the Jinja blocks as syntax errors (see static analysis hint). These are false positives in the context of Helm templating. Consider disabling or ignoring linting for this file (e.g., via a# yamllint disabledirective) or updating your linter configuration to recognize Helm template syntax and reduce noise.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/apps/bucket/README.md(1 hunks)packages/apps/bucket/values.schema.json(1 hunks)packages/apps/bucket/values.yaml(1 hunks)packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/cilium.yaml(2 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/apps/bucket/values.yaml
- packages/apps/bucket/README.md
- packages/apps/bucket/values.schema.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/apps/kubernetes/Chart.yaml
- packages/apps/kubernetes/values.yaml
- packages/apps/kubernetes/values.schema.json
- packages/apps/versions_map
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/kubernetes/templates/helmreleases/cilium.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: Build and Test
🔇 Additional comments (2)
packages/apps/kubernetes/templates/helmreleases/cilium.yaml (2)
1-8: Centralize default Cilium configuration
Great use of a reusable Helm template (cozystack.defaultCiliumValues) to define and centralize default Cilium settings. This will improve maintainability and ensure consistent defaults across all tenant clusters.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
43-43: Verify override merging precedence
The pipeline merges user-providedvaluesOverrideonto the default values template viamergeOverwrite. Please confirm that:
- If
valuesOverrideis unset or empty, the defaults fromcozystack.defaultCiliumValuesare still applied.- Any keys specified in
valuesOverridecorrectly override the defaults.You might add a brief inline comment here to clarify merge order, or run a quick
helm templatetest with a sample override to validate the behavior.
Users requested the possibility of passing custom values to the Cilium HelmRelease in tenant k8s clusters to enable its latest features, such as support for the Gateway API. This customization is now available via the `valuesOverride` field under `addons.cilium` in the kubernetes' app values. Additionally add dummy schema for S3 bucket, as it breaks the pre-commit checks. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
5f84246 to
a03cdef
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/apps/kubernetes/templates/helmreleases/cilium.yaml (2)
1-8: Centralize default values & suppress YAMLlint false positives
- Extracting default Cilium settings into the
cozystack.defaultCiliumValuestemplate is a strong DRY improvement.- YAMLlint flags the leading hyphens in
{{- define ... -}}as a syntax error, but this is valid Helm templating—ignore or disable YAMLlint for Helm templates.- Confirm that 8000
k8sServiceHost: {{ .Release.Name }}.{{ .Release.Namespace }}.svcresolves to your Kubernetes API server (commonlykubernetes.default.svc), or consider aligning with the upstream chart default to avoid DNS mismatches.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
43-43: Verify mergeOverwrite order & improve readability of override logic
EnsuremergeOverwriteis applied so that user-supplied values invaluesOverridealways take precedence. The one-liner works if the override map is piped first, but consider refactoring into intermediate variables for clarity:{{- $defaults := fromYaml (include "cozystack.defaultCiliumValues" .) }} {{- $overrides := deepCopy .Values.addons.cilium.valuesOverride }} {{- $merged := mergeOverwrite $overrides $defaults }} {{- toYaml $merged | nindent 4 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/apps/bucket/README.md(1 hunks)packages/apps/bucket/values.schema.json(1 hunks)packages/apps/bucket/values.yaml(1 hunks)packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/README.md(1 hunks)packages/apps/kubernetes/templates/helmreleases/cilium.yaml(2 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/apps/bucket/values.yaml
- packages/apps/kubernetes/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/apps/kubernetes/Chart.yaml
- packages/apps/versions_map
- packages/apps/bucket/README.md
- packages/apps/bucket/values.schema.json
- packages/apps/kubernetes/values.yaml
- packages/apps/kubernetes/values.schema.json
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/kubernetes/templates/helmreleases/cilium.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: Build and Test
Users requested the possibility of passing custom values to the Cilium HelmRelease in tenant k8s clusters to enable its latest features, such as support for the Gateway API. This customization is now available via the `valuesOverride` field under `addons.cilium` in the kubernetes' app values. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for custom override values for the Cilium addon, allowing users to configure Cilium settings via the values file. - **Chores** - Updated the Kubernetes chart version to 0.20.0. - Updated version mappings to reflect the new chart version. - **Documentation** - Updated Kubernetes managed service docs to include configuration details for Cilium addon overrides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit 0346dc0) Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Users requested the possibility of passing custom values to the Cilium HelmRelease in tenant k8s clusters to enable its latest features, such as support for the Gateway API. This customization is now available via the
valuesOverridefield underaddons.ciliumin the kubernetes' app values.Summary by CodeRabbit