Update Cilium v1.19.0 - #2054
Conversation
📝 WalkthroughWalkthroughCilium Helm chart version bump from 1.18.6 to 1.19.0 with extensive template enhancements, new extensibility hooks, standalone DNS proxy feature, CoreDNS MCS API auto-configuration, cloud provider enhancements (ENI, Azure, Alibaba), encryption mode refinements, and expanded RBAC permissions across multiple components. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Helm
participant Agent as Cilium Agent
participant ConfigMgr as Config Manager
participant CloudInit as Cloud-Init
participant Kernel as Kernel/eBPF
User->>Agent: Deploy with buildDaemonConfig
activate Agent
Note over Agent: Check daemon.configSources validity
alt buildDaemonConfig == true
Agent->>ConfigMgr: Initialize config via tmpVolume
ConfigMgr->>Agent: Provide built config
else buildDaemonConfig == false
Agent->>ConfigMgr: Mount config from ConfigMap
ConfigMgr->>Agent: Provide mounted config
end
Agent->>CloudInit: Check waitForCloudInit
alt Cloud-Init Enabled
Agent->>CloudInit: Wait for cloud-init status
CloudInit->>Agent: cloud-init completed
end
Agent->>Kernel: Apply eBPF programs
Kernel->>Agent: Health check on "health" port
activate Kernel
Agent-->>Kernel: Readiness/Liveness probes
deactivate Kernel
deactivate Agent
sequenceDiagram
participant Helm as Helm Templates
participant ClusterMesh as ClusterMesh API Server
participant CoreDNS as CoreDNS
participant MCSJob as MCS API Auto-Config Job
participant KubeAPI as Kubernetes API
Helm->>Helm: Check clustermesh.mcsapi.enabled
activate Helm
alt MCS API Enabled
Helm->>KubeAPI: Create RBAC (ClusterRole, ClusterRoleBinding)
Helm->>KubeAPI: Create Job (coredns auto-config)
Helm->>KubeAPI: Create ServiceAccount
KubeAPI->>MCSJob: Schedule Job
activate MCSJob
MCSJob->>ClusterMesh: Fetch MCS API configuration
MCSJob->>CoreDNS: Update CoreDNS ConfigMap
CoreDNS->>KubeAPI: Watch ServiceImports
MCSJob->>KubeAPI: Mark Job completed
deactivate MCSJob
else
Note over Helm: Skip MCS API setup
end
deactivate Helm
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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 |
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
b19889d to
181356c
Compare
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 upgrading the Cilium CNI to version 1.19.0, incorporating a multitude of new features, configuration options, and deprecations from the upstream project. The update includes refreshed container images for all core components, significant enhancements to ClusterMesh functionality with Multi-Cluster Services API support and CoreDNS auto-configuration, and expanded encryption capabilities with more granular strict mode settings. Additionally, it introduces a standalone DNS proxy, refines BPF datapath controls, and adjusts various operational parameters for improved stability and observability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request updates Cilium to v1.19.0. The changes mostly consist of version updates in various configuration files, Makefiles, and Dockerfiles, along with updates to Helm chart values and templates to reflect new features and deprecations in the new Cilium version.
My review has identified a potential issue with the SPIRE image versions being downgraded, which could introduce risks. I've also made a suggestion regarding the use of the latest image tag for better reproducibility.
Overall, the changes are consistent with a version upgrade, with many improvements in the Helm templates for flexibility and robustness.
| repository: ghcr.io/cozystack/cozystack/cilium | ||
| tag: 1.18.6 | ||
| digest: "sha256:4f4585f8adc3b8becd15d3999f3900a4d3d650f2ab7f85ca8c661f3807113d01" | ||
| tag: latest |
There was a problem hiding this comment.
Using the latest tag for an image is generally discouraged as it can lead to non-reproducible builds and unexpected behavior in deployments. It would be better to use a specific version tag, even if it's updated by the build process. This makes the source code clearer and deployments more predictable.
There was a problem hiding this comment.
Actionable comments posted: 3
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/clustermesh-config/clustermesh-secret.yaml (1)
21-31:⚠️ Potential issue | 🔴 CriticalMove these changes out of the vendored upstream Helm chart.
This file is part of the vendored Cilium chart (pulled via
helm pull cilium/cilium). Direct edits to vendored charts violate the coding guidelines and will be overwritten on the next chart update via the Makefileupdatetarget.The
clustermesh-clustershelper correctly handles both map and list cluster formats and returns proper JSON for consumption byfromJson, but these changes must be applied through proper chart vendoring mechanisms (e.g., Kustomize patches, values overlays, or contributing upstream) rather than direct file edits.packages/system/cilium/charts/cilium/templates/hubble/tls-cronjob/_job-spec.tpl (1)
96-98:⚠️ Potential issue | 🔴 CriticalFix Helm template syntax error in hubble-metrics certificate configuration.
Lines 96 and 98 have mismatched braces causing
| quote }}to be emitted as literal text in rendered YAML. Thequotefilter is outside the template expression.This bug exists in Cilium 1.19.0 upstream. The template expressions should move
| quoteinside the{{ }}block:Proposed fix
- commonName: {{ list (.Values.cluster.name | replace "." "-") "hubble-metrics.cilium.io" | join "." }} | quote }} + commonName: {{ list (.Values.cluster.name | replace "." "-") "hubble-metrics.cilium.io" | join "." | quote }} hosts: - - {{ list (.Values.cluster.name | replace "." "-") "hubble-metrics.cilium.io" | join "." }} | quote }} + - {{ list (.Values.cluster.name | replace "." "-") "hubble-metrics.cilium.io" | join "." | quote }}
🤖 Fix all issues with AI agents
In `@packages/system/cilium/charts/cilium/README.md`:
- Line 69: The description for the Helm value key "authentication.enabled"
contains a typo; update the sentence "But the resulting authentication requests
for these requests will not be processed, therefore the requests not be
allowed." to read "But the resulting authentication requests for these requests
will not be processed; therefore the requests will not be allowed." so it is
grammatically correct and clearer.
In `@packages/system/cilium/charts/cilium/values.yaml.tmpl`:
- Around line 3496-3498: Fix the minor spacing typo in the comment that reads
``clustermesh.config.enabled=true``documentation by inserting a space between
the closing backticks and "documentation" so it becomes
``clustermesh.config.enabled=true`` documentation; locate the comment containing
the backtick-quoted config name (the snippet
``clustermesh.config.enabled=true``documentation) and update it accordingly.
- Around line 4340-4377: The values.yaml.tmpl references undefined template
variables STANDALONE_DNS_PROXY_REPO, STANDALONE_DNS_PROXY_VERSION, and
STANDALONE_DNS_PROXY_DIGEST which are not handled by the Makefile image target;
either (A) extend the Makefile's image target to set and substitute these
variables the same way it does for cilium/envoy/certgen (add
STANDALONE_DNS_PROXY_* vars to the image target, update the yq substitution
steps and any image build/push logic to produce repo/tag/digest values) or (B)
remove the standalone DNS proxy image fields (repository/tag/digest/useDigest)
from values.yaml.tmpl if the feature is not supported; reference the
values.yaml.tmpl template variables and the Makefile image target/yq
substitution logic when implementing the change.
🧹 Nitpick comments (3)
packages/system/cilium/values.yaml (1)
17-19:tag: latestis a build-time placeholder — confirm if intentional hereThe Makefile
imagetarget overwrites this with the actual version:yq -i '.cilium.image.tag = strenv(TAG)' values.yamlsets it to the Chart version. The digest on line 19 is extracted from the built image metadata. Havingtag: latestcommitted alongside a specific digest is confusing — while the digest pins it safely, a reader of values.yaml would expect them to align. This pattern is unique to cilium among all system packages; consider either:
- Document this as the expected placeholder state before the
imagetarget runs, or- Use the actual version tag in the committed values.yaml for clarity.
packages/system/cilium/charts/cilium/README.md (1)
62-65: Add descriptions for newly documented cloud nodeSpec fields.The Description column is empty for these new keys, which makes the values table less usable. Please add short, user-facing descriptions (even 1–2 sentences) for
alibabacloud.nodeSpec.*andazure.nodeSpec.azureInterfaceName.Also applies to: 121-121
packages/system/cilium/charts/cilium/values.yaml.tmpl (1)
1273-1277:ttlSecondsAfterFinishedchanged tonull— completed certgen Job pods will persist.Previously this was set to an integer value, which would auto-clean completed Job pods. Setting it to
nullmeans the Kubernetes default applies (no automatic cleanup). This could lead to accumulation of completed Job pods over time if not managed externally. Verify this matches the upstream 1.19.0 default and is intentional.
| | annotations | object | `{}` | Annotations to be added to all top-level cilium-agent objects (resources under templates/cilium-agent) | | ||
| | apiRateLimit | string | `nil` | The api-rate-limit option can be used to overwrite individual settings of the default configuration for rate limiting calls to the Cilium Agent API | | ||
| | authentication.enabled | bool | `true` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests not be allowed. | | ||
| | authentication.enabled | bool | `false` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests not be allowed. | |
There was a problem hiding this comment.
Fix grammar in authentication.enabled description.
Current text says “therefore the requests not be allowed,” which reads as a typo.
✏️ Proposed fix
-| authentication.enabled | bool | `false` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests not be allowed. |
+| authentication.enabled | bool | `false` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests will not be allowed. |📝 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.
| | authentication.enabled | bool | `false` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests not be allowed. | | |
| | authentication.enabled | bool | `false` | Enable authentication processing and garbage collection. Note that if disabled, policy enforcement will still block requests that require authentication. But the resulting authentication requests for these requests will not be processed, therefore the requests will not be allowed. | |
🤖 Prompt for AI Agents
In `@packages/system/cilium/charts/cilium/README.md` at line 69, The description
for the Helm value key "authentication.enabled" contains a typo; update the
sentence "But the resulting authentication requests for these requests will not
be processed, therefore the requests not be allowed." to read "But the resulting
authentication requests for these requests will not be processed; therefore the
requests will not be allowed." so it is grammatically correct and clearer.
| # -- Deploy clustermesh-apiserver for clustermesh. This option is typically | ||
| # used with ``clustermesh.config.enabled=true``. Refer to the | ||
| # ``clustermesh.config.enabled=true``documentation for more information. |
There was a problem hiding this comment.
Minor typo: missing space before "documentation".
Line 3498 has clustermesh.config.enabled=true``documentation — there's a missing space between the backtick-quoted value and the word "documentation".
- # used with ``clustermesh.config.enabled=true``. Refer to the
- # ``clustermesh.config.enabled=true``documentation for more information.
+ # used with ``clustermesh.config.enabled=true``. Refer to the
+ # ``clustermesh.config.enabled=true`` documentation for more information.🤖 Prompt for AI Agents
In `@packages/system/cilium/charts/cilium/values.yaml.tmpl` around lines 3496 -
3498, Fix the minor spacing typo in the comment that reads
``clustermesh.config.enabled=true``documentation by inserting a space between
the closing backticks and "documentation" so it becomes
``clustermesh.config.enabled=true`` documentation; locate the comment containing
the backtick-quoted config name (the snippet
``clustermesh.config.enabled=true``documentation) and update it accordingly.
| # -- Standalone DNS Proxy Configuration | ||
| # Note: The standalone DNS proxy uses the agent's dnsProxy.* configuration | ||
| # for DNS settings (proxyPort, enableDnsCompression) to ensure consistency. | ||
| standaloneDnsProxy: | ||
| # -- Enable standalone DNS proxy (alpha feature) | ||
| enabled: false | ||
| # -- Roll out Standalone DNS proxy automatically when configmap is updated. | ||
| rollOutPods: false | ||
| # -- Standalone DNS proxy annotations | ||
| annotations: {} | ||
| # -- Standalone DNS proxy debug mode | ||
| debug: false | ||
| # -- Standalone DNS proxy server port | ||
| serverPort: 10095 | ||
| # -- Standalone DNS proxy Node Selector | ||
| nodeSelector: | ||
| kubernetes.io/os: linux | ||
| # -- Standalone DNS proxy tolerations | ||
| tolerations: [] | ||
| # -- Standalone DNS proxy auto mount service account token | ||
| automountServiceAccountToken: false | ||
| # -- Standalone DNS proxy update strategy | ||
| updateStrategy: | ||
| type: RollingUpdate | ||
| rollingUpdate: | ||
| maxSurge: 2 | ||
| maxUnavailable: 0 | ||
| # -- Standalone DNS proxy image | ||
| image: | ||
| # @schema | ||
| # type: [null, string] | ||
| # @schema | ||
| override: ~ | ||
| repository: "${STANDALONE_DNS_PROXY_REPO}" | ||
| tag: "${STANDALONE_DNS_PROXY_VERSION}" | ||
| digest: "${STANDALONE_DNS_PROXY_DIGEST}" | ||
| useDigest: ${USE_DIGESTS} | ||
| pullPolicy: "${PULL_POLICY}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for definitions of STANDALONE_DNS_PROXY variables in the repository
echo "=== Searching for STANDALONE_DNS_PROXY variable definitions ==="
rg -rn "STANDALONE_DNS_PROXY" --type-not yaml -g '!**/charts/**' | head -40
echo ""
echo "=== Searching in Makefile or script files ==="
fd -e mk -e Makefile -e sh -e bash --exec grep -l "STANDALONE_DNS_PROXY" {} \;
echo ""
echo "=== Searching in any file outside charts ==="
rg -rn "STANDALONE_DNS_PROXY" -g '!**/values.yaml.tmpl' | head -40Repository: cozystack/cozystack
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Find Makefile in packages/system/cilium
echo "=== Looking for Makefile in cilium package ==="
find packages/system/cilium -name "Makefile" -o -name "*.mk" | head -20
echo ""
echo "=== Looking at root Makefile and build scripts ==="
ls -la | grep -i make
echo ""
echo "=== Search for how template variables are typically handled ==="
rg -rn "values\.yaml\.tmpl" packages/system/ | head -20
echo ""
echo "=== Check other image variable patterns in cilium values.yaml.tmpl ==="
rg -n "\$\{.*_REPO\}" packages/system/cilium/charts/cilium/values.yaml.tmpl | head -20Repository: cozystack/cozystack
Length of output: 604
🏁 Script executed:
#!/bin/bash
# Read the cilium Makefile
echo "=== Content of packages/system/cilium/Makefile ==="
cat -n packages/system/cilium/Makefile
echo ""
echo "=== Search for template variable definitions ==="
rg -n "STANDALONE_DNS_PROXY|USE_DIGESTS|PULL_POLICY" packages/system/cilium/MakefileRepository: cozystack/cozystack
Length of output: 1668
🏁 Script executed:
#!/bin/bash
# Check the included makefiles
echo "=== Content of hack/common-envs.mk ==="
cat -n hack/common-envs.mk | head -50
echo ""
echo "=== Content of hack/package.mk ==="
cat -n hack/package.mk | head -100
echo ""
echo "=== Search for how values.yaml.tmpl files are rendered ==="
rg -n "values\.yaml\.tmpl" hack/ -A 3 -B 3Repository: cozystack/cozystack
Length of output: 2956
🏁 Script executed:
#!/bin/bash
# Check root Makefile
echo "=== Root Makefile content ==="
cat -n Makefile
echo ""
echo "=== Search for any script that processes .tmpl files ==="
fd -type f -name "*.sh" -o -name "*.py" -o -name "*.mk" | xargs grep -l "\.tmpl" 2>/dev/null | head -20
echo ""
echo "=== Check if values.yaml.tmpl exists and view it with template variables ==="
wc -l packages/system/cilium/charts/cilium/values.yaml.tmpl
echo ""
echo "=== Look for template variable pattern in the .tmpl file ==="
rg -n '\$\{' packages/system/cilium/charts/cilium/values.yaml.tmpl | head -30Repository: cozystack/cozystack
Length of output: 6196
🏁 Script executed:
#!/bin/bash
# Search for how template variables are substituted
echo "=== Search for scripts that substitute template variables ==="
find . -name "*.sh" -type f | xargs grep -l "values.yaml.tmpl\|CILIUM_REPO\|PULL_POLICY" 2>/dev/null
echo ""
echo "=== Check if there's an envsubst or similar template processing ==="
rg -rn "envsubst|sed.*values\.yaml\.tmpl|yq.*values\.yaml\.tmpl" --type-not yaml
echo ""
echo "=== Search for where CILIUM_REPO type variables are defined ==="
rg -rn "CILIUM_REPO|PULL_POLICY" --type-not yaml -g '!**/charts/**' | head -30
echo ""
echo "=== Check cilium package specifically for image building ==="
ls -la packages/system/cilium/Repository: cozystack/cozystack
Length of output: 938
🏁 Script executed:
#!/bin/bash
# Check Chart.yaml
echo "=== Chart.yaml content ==="
cat -n packages/system/cilium/Chart.yaml
echo ""
echo "=== Check if values.yaml is generated or handwritten ==="
head -20 packages/system/cilium/values.yaml
echo ""
echo "=== Check if values.yaml.tmpl is the source ==="
head -20 packages/system/cilium/charts/cilium/values.yaml.tmpl
echo ""
echo "=== Check templates directory ==="
ls -la packages/system/cilium/templates/
echo ""
echo "=== Compare line counts ==="
wc -l packages/system/cilium/values.yaml packages/system/cilium/charts/cilium/values.yaml.tmpl
echo ""
echo "=== Search for STANDALONE_DNS_PROXY in values.yaml (not .tmpl) ==="
grep -n "STANDALONE_DNS_PROXY" packages/system/cilium/values.yamlRepository: cozystack/cozystack
Length of output: 1796
🏁 Script executed:
#!/bin/bash
# Search for .tmpl processing in cilium Makefile more carefully
echo "=== Full cilium Makefile ==="
cat packages/system/cilium/Makefile
echo ""
echo "=== Search for any script that might process .tmpl files ==="
find . -type f \( -name "*.sh" -o -name "*.py" \) | xargs grep -l "tmpl\|STANDALONE_DNS_PROXY" 2>/dev/null
echo ""
echo "=== Check if values.yaml.tmpl is perhaps not meant to be used yet ==="
git log --oneline packages/system/cilium/charts/cilium/values.yaml.tmpl 2>/dev/null | head -5
echo ""
echo "=== Search for STANDALONE_DNS_PROXY in entire repo ==="
rg -rn "STANDALONE_DNS_PROXY" 2>/dev/nullRepository: cozystack/cozystack
Length of output: 1832
Add STANDALONE_DNS_PROXY image variables to the Makefile image target or remove them from values.yaml.tmpl.
The template variables ${STANDALONE_DNS_PROXY_REPO}, ${STANDALONE_DNS_PROXY_VERSION}, and ${STANDALONE_DNS_PROXY_DIGEST} are referenced in values.yaml.tmpl but are not defined or substituted anywhere in the build pipeline. The cilium Makefile's image target only handles cilium, envoy, and certgen image variables via yq, while the .tmpl file (a vendor file from upstream cilium) contains the undefined STANDALONE_DNS_PROXY references. Either extend the Makefile to handle STANDALONE_DNS_PROXY image building and substitution, or remove these references from the template if the feature is not yet supported.
🤖 Prompt for AI Agents
In `@packages/system/cilium/charts/cilium/values.yaml.tmpl` around lines 4340 -
4377, The values.yaml.tmpl references undefined template variables
STANDALONE_DNS_PROXY_REPO, STANDALONE_DNS_PROXY_VERSION, and
STANDALONE_DNS_PROXY_DIGEST which are not handled by the Makefile image target;
either (A) extend the Makefile's image target to set and substitute these
variables the same way it does for cilium/envoy/certgen (add
STANDALONE_DNS_PROXY_* vars to the image target, update the yq substitution
steps and any image build/push logic to produce repo/tag/digest values) or (B)
remove the standalone DNS proxy image fields (repository/tag/digest/useDigest)
from values.yaml.tmpl if the feature is not supported; reference the
values.yaml.tmpl template variables and the Makefile image target/yq
substitution logic when implementing the change.
|
Let's update it after v1.0 released |
|
closed in favor #2173 |
What this PR does
Release note
Summary by CodeRabbit
New Features
Improvements