PMK-6388: fix bash exit status reporting

[mridulgain]

ISSUE(S):

https://platform9.atlassian.net/browse/PMK-6388

SUMMARY

Root cause: Error was not reported back correctly.
2023-09-06T17:49:09.0543Z DEBUG Running command https_proxy=http://qcwebproxylb.juniper.net:3128 bash "-c" "grep host_id /etc/pf9/host_id.conf | cut -d '=' -f2"stdout:stderr:grep: /etc/pf9/host_id.conf: No such file or directory
In scenarios when "file is not found", it's expected to receive a non nil error.
When piping commands, the status code reported by grep is overwritten by that of cut

mridulgain@HXJN3GQY1WFV pf9ctl % grep host_id /etc/pf9/host_id.conf| cut -d '=' -f2
grep: /etc/pf9/host_id.conf: No such file or directory
mridulgain@HXJN3GQY1WFV pf9ctl % echo $?
0

Which leads to a nil err & despite of having error handling it fails to gracefully handle it and later panics with nil pointer dereference due to empty hostID inside AuthorizeHost function.

Suggested fix:

use && instead of | operator to achieve same behavior with correct exit status.

mridulgain@HXJN3GQY1WFV pf9ctl % grep host_id /etc/pf9/host_id.conf
grep: /etc/pf9/host_id.conf: No such file or directory
mridulgain@HXJN3GQY1WFV pf9ctl % echo $?
2

ISSUE TYPE

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that may cause existing functionality to not work as expected)
• This change requires a documentation update

IMPACTED FEATURES/COMPONENTS:

prepNode

RELATED ISSUE(S):

DEPENDS ON:

TESTING DONE

Automated

Manual

Reviewers

Summary by Bito

Fixed a bug in command pipeline error handling by replacing the pipe operator (|) with logical AND (&&) when reading host_id from /etc/pf9/host_id.conf. This change ensures proper error reporting from grep commands and prevents potential nil pointer dereference panics.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

[bito-code-review]

Code Review Agent Run #a50a7b

Actionable Suggestions - 1

• pkg/pmk/node.go - 1

  • Consider pipe instead of AND operator · Line 147-147

Review Details

• Files reviewed - 1 · Commit Range: c0edddd..c0edddd

  • pkg/pmk/node.go
• Files skipped - 0
• Tools

  • Golangci-lint (Linter) - ✖︎ Failed
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

AI Code Review powered by

[bito-code-review]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
Bug Fix - Fix Command Pipeline Error Handling	- node.go - Modified grep command to use && instead of pipe for proper error status reporting

[bito-code-review]

Consider pipe instead of AND operator

The command pipe operation using && may not provide the expected behavior. When using &&, the second command will only execute if the first command succeeds. This means if grep fails, cut won't execute at all. Consider using pipe operator | instead to ensure cut processes the output of grep regardless of exit status.

Code suggestion

Check the AI-generated fix before applying

Suggested change

	cmd := `grep host_id /etc/pf9/host_id.conf && cut -d '=' -f2`
	cmd := `grep host_id /etc/pf9/host_id.conf | cut -d '=' -f2`

Code Review Run #a50a7b

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them