Update cilium v1.17.8 - #1473
Conversation
WalkthroughThe PR bumps Cilium-related versions to 1.17.8, updates image tags/digests, adds Helm extension hooks for dnsPolicy/volumes, adjusts config conditions in the ConfigMap, reorders extraEnv in clustermesh-apiserver, introduces podSecurityContext.seccompProfile, removes externalIPs fields, and updates schemas/types accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Helm as Helm Chart
participant Tpl as _extensions.tpl
participant DS as cilium-agent/daemonset
Note over Helm,Tpl: Render cilium-agent DaemonSet with extension hooks
User->>Helm: helm install/upgrade
Helm->>Tpl: include "cilium-agent.dnsPolicy"
alt .Values.dnsPolicy set
Tpl-->>Helm: dnsPolicy: <value>
else not set
Tpl-->>Helm: (no output)
end
Helm->>Tpl: include "cilium-agent.volumeMounts.extra"
Tpl-->>Helm: (render extra mounts or empty)
Helm->>Tpl: include "cilium-agent.volumes.extra"
Tpl-->>Helm: (render extra volumes or empty)
Helm->>DS: compose final DaemonSet manifest
DS-->>User: Applied DaemonSet spec
sequenceDiagram
autonumber
participant Helm as Helm Chart
participant CM as cilium-configmap.yaml
Note over Helm,CM: Evaluate kubeProxyReplacement and Hubble IPv6 preferences
Helm->>CM: Render ConfigMap
alt kubeProxyReplacement == "true"
CM-->>Helm: add healthz bind address
else
CM-->>Helm: omit healthz bind address
end
alt kubeProxyReplacement == "false"
CM-->>Helm: enable hostPort and nodePort
else
CM-->>Helm: disable hostPort and nodePort
end
alt hubble.preferIpv6 == true OR ipv4.enabled == false
CM-->>Helm: set hubble-prefer-ipv6: "true"
else
CM-->>Helm: omit/false
end
Note right of CM: externalIPs block removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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. Comment |
Summary of ChangesHello Andrei Kvapil (@kvaps), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on updating the Cilium network plugin to version v1.17.8. This update encompasses bumping the versions and updating image digests for the main Cilium agent and several of its critical sub-components, ensuring the deployment uses the latest stable releases. Beyond version upgrades, the changes also introduce greater flexibility in Helm chart configurations for the Cilium agent, specifically around volume management and DNS policy, and refine certain network service configurations related to 'kube-proxy-replacement' and 'externalIPs'. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
0f79eb5 to
174f8cc
Compare
There was a problem hiding this comment.
Code Review
This pull request updates Cilium to version 1.17.8. The changes primarily consist of version bumps in Chart.yaml, image tags, and digests in values.yaml and other related files. It also includes updates to the Helm chart templates and schemas to align with the upstream Cilium chart for this version. This includes refactoring for extensibility, schema corrections, and adjustments to configuration logic, particularly around kubeProxyReplacement settings. The changes appear correct and consistent with a standard version upgrade. I have not found any issues of medium or higher severity. It would be beneficial to add a release note to the PR description summarizing the update for users.
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
a6fa329 to
1753df5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/system/cilium/charts/cilium/templates/cilium-configmap.yaml (1)
71-75: Restore upgradeCompatibility behaviour for kubeProxyReplacementThe new guard forces
kubeProxyReplacementto be exactly"true"or"false"even when the value came from theupgradeCompatibilitydefaults. As soon as someone follows the documented upgrade path (e.g.upgradeCompatibility: "1.12"), the computed default becomes"disabled"and Helm now fails rendering with “kubeProxyReplacement must be explicitly set…”. We need to keep honoring the legacy defaults unless the user explicitly overrides the value.Please gate the
failso it only trips when the user actually set.Values.kubeProxyReplacementto an unsupported value:-{{- if and (ne $kubeProxyReplacement "true") (ne $kubeProxyReplacement "false") }} +{{- if and (not (kindIs "invalid" .Values.kubeProxyReplacement)) (ne $kubeProxyReplacement "true") (ne $kubeProxyReplacement "false") }}packages/system/cilium/charts/cilium/values.yaml (1)
513-527: bpf.events default values: null vs documented 0 mismatch.Values set rateLimit/burstLimit to null here, but the docs state default 0. Pick one to avoid confusion. If 0 is intended, set explicit zeros.
Apply this diff if you want 0 as the actual default:
- rateLimit: ~ + rateLimit: 0 @@ - burstLimit: ~ + burstLimit: 0Also applies to: 523-523, 524-532
🧹 Nitpick comments (4)
packages/system/cilium/charts/cilium/values.yaml (1)
269-276: New seccompProfile defaults to Unconfined; confirm necessity or consider RuntimeDefault.If Unconfined isn’t strictly required, RuntimeDefault is a safer baseline. Ensure templates plumb this value correctly to pod.spec.securityContext.seccompProfile.
packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml (1)
772-772: dnsPolicy include is safer than emitting empty stringRendering only when set avoids invalid dnsPolicy: "" and preserves K8s defaults with hostNetwork. Good change.
Consider defaulting .Values.dnsPolicy to null (~) in values.yaml for clarity since it’s now omitted when unset.
packages/system/cilium/charts/cilium/values.yaml.tmpl (2)
518-537: Enforce rateLimit/burstLimit pairing in cilium chartpackages/system/cilium/charts/cilium/values.schema.json currently only types both as null|integer without co-dependency. Add an
if/thenblock in the schema so that when one is > 0 the other is required:"if": { "properties": { "rateLimit": { "exclusiveMinimum": 0 } } }, "then": { "required": ["burstLimit"] }, "else": { "if": { "properties": { "burstLimit": { "exclusiveMinimum": 0 } } }, "then": { "required": ["rateLimit"] } }Alternatively, add a Helm fail-fast check in cilium/templates/cilium-configmap.yaml:
{{- $rl := int (default 0 .Values.bpf.events.default.rateLimit) -}} {{- $bl := int (default 0 .Values.bpf.events.default.burstLimit) -}} {{- if and (gt $rl 0) (le $bl 0) -}} {{- fail "bpf.events.default.burstLimit must be > 0 when rateLimit > 0" -}} {{- end -}} {{- if and (gt $bl 0) (le $rl 0) -}} {{- fail "bpf.events.default.rateLimit must be > 0 when burstLimit > 0" -}} {{- end -}}
274-276: seccompProfile default matches upstream v1.17.8
Upstream uses Unconfined for cilium-agent; ensure this aligns with your Pod Security Admission settings. For PSA baseline/restricted clusters, consider switching to RuntimeDefault:- seccompProfile: - type: "Unconfined" + seccompProfile: + type: "RuntimeDefault"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/system/cilium/charts/cilium/Chart.yaml(2 hunks)packages/system/cilium/charts/cilium/README.md(12 hunks)packages/system/cilium/charts/cilium/templates/_extensions.tpl(1 hunks)packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml(3 hunks)packages/system/cilium/charts/cilium/templates/cilium-configmap.yaml(3 hunks)packages/system/cilium/charts/cilium/templates/clustermesh-apiserver/deployment.yaml(1 hunks)packages/system/cilium/charts/cilium/values.schema.json(2 hunks)packages/system/cilium/charts/cilium/values.yaml(13 hunks)packages/system/cilium/charts/cilium/values.yaml.tmpl(3 hunks)packages/system/cilium/images/cilium/Dockerfile(1 hunks)packages/system/cilium/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-14T16:54:50.368Z
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile:3-4
Timestamp: 2025-05-14T16:54:50.368Z
Learning: In Dockerfiles for the cozystack project, ARG ordering where a variable like ${TARGETARCH} is referenced before being declared can still work correctly in practice because Docker's BuildKit may evaluate the variable only when it's actually used in a FROM statement.
Applied to files:
packages/system/cilium/images/cilium/Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (15)
packages/system/cilium/images/cilium/Dockerfile (1)
1-1: Version bump looks good.No functional changes; just the default ARG. Nothing else to do here.
packages/system/cilium/charts/cilium/Chart.yaml (1)
82-82: Chart/app versions correctly bumped.Consistent with the image/version updates elsewhere.
Also applies to: 98-98
packages/system/cilium/charts/cilium/values.schema.json (2)
538-548: Type widening for events.default.{burstLimit,rateLimit} is correct.Allows null or integer; matches typical Helm values usage.
4655-4662: seccompProfile alignment confirmed schema, values, templates, and README updated consistently; no stale externalIPs in chart directory.packages/system/cilium/charts/cilium/templates/clustermesh-apiserver/deployment.yaml (1)
225-227: Approve extraEnv repositioning — single block confirmed
Only one.Values.clustermesh.apiserver.extraEnvoccurrence found; Helm rendering and Kubernetes semantics unaffected.packages/system/cilium/values.yaml (1)
17-18: Authenticate and verify Cilium image digest
Pull or inspect ghcr.io/cozystack/cozystack/cilium:1.17.8 with proper GHCR credentials (e.g.,docker pullorskopeo inspect) and confirm its digest matches sha256:81262986a41487bfa3d0465091d3a386def5bd1ab476350bd4af2fdee5846fe6.packages/system/cilium/charts/cilium/README.md (3)
3-3: Version badges updated to 1.17.8 — OK.
88-88: Docs reflect image/tag/digest updates — keep README in sync with values.yaml.Looks consistent with values.yaml. Ensure helm-docs (or your doc generator) was re-run so future diffs don’t drift.
Also applies to: 200-200, 381-381, 521-521, 588-588, 598-598, 628-628, 739-739, 766-766, 816-816
139-140: Values.yaml defaults to 0 via schema; README is correct.Likely an incorrect or invalid review comment.
packages/system/cilium/charts/cilium/values.yaml (1)
189-199: Confirm sha256 digests match upstream manifests
Run registry-inspect (e.g.docker manifest inspect) on each image tag in values.yaml—quay.io/cilium/{cilium,certgen,operator,clustermesh-apiserver}, quay.io/cilium/hubble-{relay,ui,ui-backend}, quay.io/cilium/cilium-envoy, quay.io/cilium/startup-script, docker.io/library/busybox, ghcr.io/spiffe/{spire-agent,spire-server}—to ensure eachdigest:aligns with the manifestconfig.digest.packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml (2)
402-403: Good extensibility: extra volumeMounts hookThe include enables downstream injection without forking the template. Looks correct and well-indented.
1065-1065: Good extensibility: extra volumes hookAllows clean downstream customization of volumes. Indentation and placement are correct.
packages/system/cilium/charts/cilium/templates/_extensions.tpl (3)
6-10: Extension point for extra volumes looks goodSimple, no-op by default, and easy to override. No concerns.
12-13: Extension point for extra volumeMounts looks goodMatches the usage site and expected YAML shape.
18-22: dnsPolicy extension renders conditionally — good default-preserving behaviorAvoids emitting invalid empty values and centralizes customization.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/system/cilium/charts/cilium/templates/cilium-configmap.yaml (1)
68-74: Blocking chart installs for legitimate kubeProxyReplacement valuesThe new
failnow rejects anything except the literal strings"true"or"false". Cilium still acceptsdisabled,probe,strict(and their boolean aliases), so existing deployments that set (for example)kubeProxyReplacement=probewill now fail to render entirely. We shouldn’t hard-stop valid configurations; please widen the allowlist (or drop the guard) so the chart remains compatible with supported Cilium flag values.-{{- if and (ne $kubeProxyReplacement "true") (ne $kubeProxyReplacement "false") }} - {{ fail "kubeProxyReplacement must be explicitly set to a valid value (true or false) to continue." }} +{{- if not (or + (eq $kubeProxyReplacement "true") + (eq $kubeProxyReplacement "false") + (eq $kubeProxyReplacement "strict") + (eq $kubeProxyReplacement "probe") + (eq $kubeProxyReplacement "disabled") + (eq $kubeProxyReplacement "partial")) }} + {{ fail "kubeProxyReplacement must be set to one of: true, false, strict, probe, disabled, or partial." }} {{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/system/cilium/charts/cilium/Chart.yaml(2 hunks)packages/system/cilium/charts/cilium/README.md(12 hunks)packages/system/cilium/charts/cilium/templates/_extensions.tpl(1 hunks)packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml(3 hunks)packages/system/cilium/charts/cilium/templates/cilium-configmap.yaml(3 hunks)packages/system/cilium/charts/cilium/templates/clustermesh-apiserver/deployment.yaml(1 hunks)packages/system/cilium/charts/cilium/values.schema.json(2 hunks)packages/system/cilium/charts/cilium/values.yaml(13 hunks)packages/system/cilium/charts/cilium/values.yaml.tmpl(3 hunks)packages/system/cilium/images/cilium/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/system/cilium/charts/cilium/Chart.yaml
- packages/system/cilium/charts/cilium/values.schema.json
- packages/system/cilium/images/cilium/Dockerfile
- packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml
- packages/system/cilium/charts/cilium/values.yaml.tmpl
- packages/system/cilium/charts/cilium/templates/_extensions.tpl
🔇 Additional comments (4)
packages/system/cilium/charts/cilium/templates/clustermesh-apiserver/deployment.yaml (1)
225-227:extraEnvplacement remains correctIndenting with
nindent 8keeps the generated entries inside theenvlist, so relocating the block ahead of the readiness probe preserves behavior while improving grouping with the other env vars. Nicely done.packages/system/cilium/charts/cilium/values.yaml (2)
513-515: LGTM: schema expansion for BPF events limitsAllowing [null, integer] for rateLimit/burstLimit aligns with optionality and avoids schema validation friction.
Also applies to: 525-527
194-198: Verify image digests against tags—agent and preflight both use v1.17.8 with identical sha256; ensure each sha256 digest matches its multi-arch image tag for all updated components.packages/system/cilium/charts/cilium/README.md (1)
3-3: Version badge update looks good.Version and AppVersion badges now correctly reflect 1.17.8.
| seccompProfile: | ||
| type: "Unconfined" |
There was a problem hiding this comment.
Use seccompProfile RuntimeDefault instead of Unconfined
Unconfined weakens the pod’s sandboxing. Prefer RuntimeDefault unless you have a specific, documented need.
Apply this diff:
podSecurityContext:
# -- AppArmorProfile options for the `cilium-agent` and init containers
appArmorProfile:
type: "Unconfined"
- seccompProfile:
- type: "Unconfined"
+ seccompProfile:
+ type: "RuntimeDefault"🤖 Prompt for AI Agents
In packages/system/cilium/charts/cilium/values.yaml around lines 273 to 274,
seccompProfile is set to "Unconfined"; change it to "RuntimeDefault" to
strengthen pod sandboxing. Update the seccompProfile.type value to
RuntimeDefault (preserve YAML formatting/indentation and quotes consistent with
surrounding file) and commit the change with a short message like "set
seccompProfile.type to RuntimeDefault".
What this PR does
Release note
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores