Skip to content

Comments

fix: Adding ipv6 check on the node init function#10

Open
MitchLewis930 wants to merge 1 commit intopr_050_beforefrom
pr_050_after
Open

fix: Adding ipv6 check on the node init function#10
MitchLewis930 wants to merge 1 commit intopr_050_beforefrom
pr_050_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

User description

PR_050


PR Type

Bug fix


Description

  • Add IPv6 validation check in node initialization

  • Return error if IPv6 enabled but node lacks IPv6 address

  • Prevent Cilium pods from failing on single-stack clusters

  • Ensure consistent behavior regardless of KPR configuration


Diagram Walkthrough

flowchart LR
  A["Node Information Received"] --> B{"IPv6 Enabled AND<br/>No IPv6 Address?"}
  B -->|Yes| C["Log Error & Return Error"]
  B -->|No| D["Continue Node Setup"]
  C --> E["Fail Fast"]
  D --> F["Use Node CIDR"]
Loading

File Walkthrough

Relevant files
Bug fix
init.go
Add IPv6 address validation in node initialization             

daemon/k8s/init.go

  • Added IPv6 validation check in WaitForNodeInformation function
  • Returns error if IPv6 is enabled but node lacks IPv6 address
  • Logs detailed error information with node name and IP addresses
  • Prevents silent failures on single-stack clusters with IPv6 enabled
+10/-0   

Cilium pods on a single stack cluster were failing when ipv6 was enabled.
The change would make sure that if the underlying node does not have ipv6
configuration enabled it would fail even if KPR is disabled.Earlier, if KPR
is enabled, the node would not see a fatal error.

Signed-off-by: Vipul Singh <singhvipul@microsoft.com>
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Logs node IPs: The new error log includes nodeIP4 and nodeIP6 values, which may be considered sensitive
infrastructure data depending on the deployment's logging policy.

Referred Code
log.WithFields(logrus.Fields{
	logfields.NodeName:         n.Name,
	logfields.IPAddr + ".ipv4": nodeIP4,
	logfields.IPAddr + ".ipv6": nodeIP6,
}).Error("No IPv6 support on node as ipv6 address is nil")
return fmt.Errorf("node %s does not have an IPv6 address", n.Name)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Move IPv6 check before success log

Reorder the logic to perform the IPv6 address check before logging the "Received
own node information" message.

daemon/k8s/init.go [121-139]

+// If the host does not have an IPv6 address, return an error
+if option.Config.EnableIPv6 && nodeIP6 == nil {
+    log.WithFields(logrus.Fields{
+        logfields.NodeName:         n.Name,
+        logfields.IPAddr + ".ipv4": nodeIP4,
+        logfields.IPAddr + ".ipv6": nodeIP6,
+    }).Error("No IPv6 support on node as ipv6 address is nil")
+    return fmt.Errorf("node %s does not have an IPv6 address", n.Name)
+}
+
 log.WithFields(logrus.Fields{
     logfields.NodeName:         n.Name,
     logfields.Labels:           logfields.Repr(n.Labels),
     logfields.IPAddr + ".ipv4": nodeIP4,
     logfields.IPAddr + ".ipv6": nodeIP6,
     logfields.V4Prefix:         n.IPv4AllocCIDR,
     logfields.V6Prefix:         n.IPv6AllocCIDR,
     logfields.K8sNodeIP:        k8sNodeIP,
 }).Info("Received own node information from API server")
 
-// If the host does not have an IPv6 address, return an error
-if option.Config.EnableIPv6 && nodeIP6 == nil {
-    log.WithFields(logrus.Fields{
-        logfields.NodeName:         n.Name,
-        logfields.IPAddr + ".ipv4": nodeIP4,
-        logfields.IPAddr + ".ipv6": nodeIP6,
-    }).Error("No IPv6 support on node as ipv6 address is nil")
-    return fmt.Errorf("node %s does not have an IPv6 address", n.Name)
-}
-

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: This is a good suggestion to improve the logical flow by performing validation before logging a success message, which makes the logs less confusing in failure scenarios.

Medium
Improve error message and remove redundant logging

Remove the redundant nodeIP6 field from the error log and clarify the error
message to explain the configuration conflict.

daemon/k8s/init.go [133-138]

 log.WithFields(logrus.Fields{
     logfields.NodeName:         n.Name,
     logfields.IPAddr + ".ipv4": nodeIP4,
-    logfields.IPAddr + ".ipv6": nodeIP6,
-}).Error("No IPv6 support on node as ipv6 address is nil")
-return fmt.Errorf("node %s does not have an IPv6 address", n.Name)
+}).Error("Node does not have an IPv6 address, but IPv6 is enabled in Cilium config")
+return fmt.Errorf("node %s does not have an IPv6 address, but IPv6 is enabled in Cilium config", n.Name)
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that logging nodeIP6 is redundant since it's always nil in this context, and proposes a more descriptive error message that improves debuggability.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants