Skip to content

fix: add missing return err after failed ResolveTCPAddr in checkIfIpIsLive - #1008

Closed
Jah-yee wants to merge 1 commit into
konflux-ci:mainfrom
Jah-yee:fix/nil-pointer-panic-checkIfIpIsLive
Closed

fix: add missing return err after failed ResolveTCPAddr in checkIfIpIsLive#1008
Jah-yee wants to merge 1 commit into
konflux-ci:mainfrom
Jah-yee:fix/nil-pointer-panic-checkIfIpIsLive

Conversation

@Jah-yee

@Jah-yee Jah-yee commented Aug 6, 2026

Copy link
Copy Markdown

Bug Description

In pkg/ibm/ibmz_helpers.go, the function checkIfIpIsLive has a missing return statement after a failed net.ResolveTCPAddr call. When DNS resolution fails, the error is logged but execution continues to line 86, where server.Network() and server.String() are called on a nil net.TCPAddr, causing a nil pointer panic.

Fix

Add return err after the error log on line 84, matching the error-handling pattern used elsewhere in the same function (see lines 89 and 93).

Impact

Any DNS resolution failure for an IBM Z or IBM Power host IP address will crash the controller with a nil pointer dereference panic rather than returning a clean error to the caller.


Fixes #985

…sLive

Fixes nil pointer panic when DNS resolution fails for IBM Z/Power host IPs.

Before: when net.ResolveTCPAddr returned an error, the function logged it but
continued execution with a nil server pointer, causing panic on server.Network()
and server.String() calls on line 86.

After: returns the error immediately after logging, matching the error-handling
pattern used elsewhere in the same function (lines 89 and 93).

Signed-off-by: Jah-yee <166608075+Jah-yee@users.noreply.github.com>
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Summary by Qodo

Fix nil pointer panic in checkIfIpIsLive on DNS resolution failure

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Return immediately when ResolveTCPAddr fails to prevent nil TCPAddr dereference
• Preserve existing error-handling pattern for connection and close failures
Diagram

graph TD
  A["checkIfIpIsLive"] --> B["ResolveTCPAddr :22"] --> C{"Resolve OK?"}
  C -->|"No"| D["Log + return err"]
  C -->|"Yes"| E["DialTimeout (5s)"] --> F{"Dial OK?"}
  F -->|"No"| G["Return err"]
  F -->|"Yes"| H["Close conn + return nil"]
Loading
High-Level Assessment

The early-return on ResolveTCPAddr failure is the correct minimal fix: it matches the function’s existing error-handling style and prevents nil dereference without changing control flow for successful cases.

Files changed (1) +1 / -0

Bug fix (1) +1 / -0
ibmz_helpers.goReturn error on ResolveTCPAddr failure to avoid nil TCPAddr panic +1/-0

Return error on ResolveTCPAddr failure to avoid nil TCPAddr panic

• Adds a missing 'return err' after 'net.ResolveTCPAddr' fails in 'checkIfIpIsLive'. This prevents subsequent calls to 'server.Network()' / 'server.String()' on a nil '*net.TCPAddr' and returns a clean error to the caller instead of panicking.

pkg/ibm/ibmz_helpers.go

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. checkIfIpIsLive return untested 📘 Rule violation ▣ Testability
Description
The new return err line in checkIfIpIsLive is not exercised by existing automated tests, and the
IBM unit tests stub out pingFunc, bypassing the real checkIfIpIsLive implementation. This
violates the requirement that every modified executable line in the patch is executed by at least
one automated test (or has explicit justification).
Code

pkg/ibm/ibmz_helpers.go[85]

+		return err
Relevance

●●● Strong

Team has accepted adding branch/line coverage in IBM code to exercise new error paths.

PR-#897

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2363 requires tests that execute every modified executable line. The patch adds
return err in checkIfIpIsLive, but existing IBM unit tests set pingFunc to stub
implementations (bypassing checkIfIpIsLive), and the helper test file focuses on
createInstanceName only, leaving the modified line without a covering test.

Rule 2363: Require tests exercising every new or modified executable line in a patch
pkg/ibm/ibmz_helpers.go[82-90]
pkg/ibm/ibmz_test.go[121-135]
pkg/ibm/ibmp_test.go[139-144]
pkg/ibm/ibmz_helpers_test.go[19-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new executable line was added (`return err`) in `checkIfIpIsLive`, but there is no automated test that executes this line.

## Issue Context
Current IBM unit tests commonly set `pingFunc` to a stub function, which bypasses the real `checkIfIpIsLive` and therefore cannot cover the new error-return behavior.

## Fix Focus Areas
- pkg/ibm/ibmz_helpers.go[78-96]
- pkg/ibm/ibmz_helpers_test.go[1-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 2 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread pkg/ibm/ibmz_helpers.go
server, err := net.ResolveTCPAddr("tcp", ip+":22")
if err != nil {
log.Error(err, "failed to resolve ip address")
return err

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. checkifipislive return untested 📘 Rule violation ▣ Testability

The new return err line in checkIfIpIsLive is not exercised by existing automated tests, and the
IBM unit tests stub out pingFunc, bypassing the real checkIfIpIsLive implementation. This
violates the requirement that every modified executable line in the patch is executed by at least
one automated test (or has explicit justification).
Agent Prompt
## Issue description
A new executable line was added (`return err`) in `checkIfIpIsLive`, but there is no automated test that executes this line.

## Issue Context
Current IBM unit tests commonly set `pingFunc` to a stub function, which bypasses the real `checkIfIpIsLive` and therefore cannot cover the new error-return behavior.

## Fix Focus Areas
- pkg/ibm/ibmz_helpers.go[78-96]
- pkg/ibm/ibmz_helpers_test.go[1-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@Jah-yee

Jah-yee commented Aug 7, 2026

Copy link
Copy Markdown
Author

Closing as duplicate — PR #990 by 123456wda addresses the same issue #985 with the same fix. Thanks to 123456wda for the faster submission!

@Jah-yee Jah-yee closed this Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Missing error return in checkIfIpIsLive causes nil pointer panic on DNS resolution failure

1 participant