Skip to content

✨ Quality: shutil.which() return value not checked before os.path.dirname() - #5413

Open
Manhhoangvp95 wants to merge 2 commits into
canonical:masterfrom
Manhhoangvp95:improve/quality/shutil-which-return-value-not-checked-be
Open

✨ Quality: shutil.which() return value not checked before os.path.dirname()#5413
Manhhoangvp95 wants to merge 2 commits into
canonical:masterfrom
Manhhoangvp95:improve/quality/shutil-which-return-value-not-checked-be

Conversation

@Manhhoangvp95

@Manhhoangvp95 Manhhoangvp95 commented Mar 27, 2026

Copy link
Copy Markdown

✨ Code Quality

Problem

On non-Windows platforms, shutil.which("kubectl") returns None if the
executable is not found. The code then calls os.path.dirname(full_path)
which will raise TypeError: expected str, bytes or os.PathLike object, not NoneType.
This will crash at runtime when kubectl is not installed.

Severity: high
File: installer/common/file_utils.py

Solution

Add a check for None: if full_path is None: raise RuntimeError("kubectl not found")
or return a sensible default.

Changes

  • installer/common/file_utils.py (modified)

Summary

Changes

Testing

Possible Regressions

Checklist

  • Read the contributions page.
  • Submitted the CLA form, if you are a first time contributor.
  • The introduced changes are covered by unit and/or integration tests.

Notes

Closes #5412

…ame()

On non-Windows platforms, `shutil.which("kubectl")` returns `None` if the 
executable is not found. The code then calls `os.path.dirname(full_path)` 
which will raise `TypeError: expected str, bytes or os.PathLike object, not NoneType`.
This will crash at runtime when kubectl is not installed.


Affected files: file_utils.py

Signed-off-by: Manhhoangvp95 <87350762+Manhhoangvp95@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Improves robustness of the installer’s kubectl path discovery by handling the case where shutil.which("kubectl") returns None on non-Windows platforms, preventing an immediate TypeError from os.path.dirname(None).

Changes:

  • Add a None guard around shutil.which("kubectl") in get_kubectl_directory() with a fallback directory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread installer/common/file_utils.py Outdated
Comment thread installer/common/file_utils.py Outdated
Comment on lines +76 to +78
full_path = shutil.which("kubectl")
if full_path is None:
return "/usr/local/bin"

Copilot AI Mar 31, 2026

Copy link

Choose a reason for hiding this comment

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

Please add a unit test covering the shutil.which("kubectl") is None branch (and the expected behavior you choose—exception vs fallback). This logic is easy to regress and the repo already has unit test coverage for other installer/common helpers.

Copilot uses AI. Check for mistakes.
louiseschmidtgen

This comment was marked as low quality.

@louiseschmidtgen louiseschmidtgen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Manhhoangvp95,
Thank you for contributing to MicroK8s.

  • 1 on copilots comment: Let's not try to guess where the kubectl installation lives ("/usr/local/bin") and instead fail with a sensible error.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

fix: shutil.which() return value not checked before os.path.dirname()

3 participants