fix: Return error from GetDefaultKubeConfigPath instead of calling os.Exit#5509
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the backend config.GetDefaultKubeConfigPath helper to return an error instead of terminating the process, and updates callers/tests to handle the new (string, error) signature. This aligns the config utility layer with Go library best practices and avoids unrecoverable exits during tests or reuse.
Changes:
- Changed
GetDefaultKubeConfigPath()to return(string, error)and removed the directos.Exit(1)call. - Updated
setKubeConfigPathandParseto propagate kubeconfig path resolution errors. - Updated backend tests that call
GetDefaultKubeConfigPathto handle the returned error.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/pkg/config/config.go | Convert default kubeconfig path resolution to error-returning flow and propagate it through config parsing. |
| backend/pkg/kubeconfig/kubeconfig_test.go | Update integration test to handle new (string, error) signature. |
| backend/cmd/headlamp_test.go | Update tests to resolve kubeconfig path once and pass it through, handling errors. |
d0c92ef to
828512d
Compare
828512d to
ae46855
Compare
ae46855 to
e7f0ce4
Compare
88a8055 to
15e714b
Compare
15e714b to
607bc56
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for working on this.
Looks like the backend tests are failing in CI. Can you check with cd backend && go test ./... and fix the errors?
How to run the backend tests
Run cd backend && go test ./... to see all failures. Fix the failing tests and commit the result.
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
There are some open Copilot review comments — could you take a look at them? Please mark each one as resolved once you've addressed it.
a3c744d to
ffd24d1
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
Could you take a look at the commit messages in this PR? We follow a Linux kernel style for git commits — see the contributing guide and git log for examples.
Commits that need attention
fix: Return error from GetDefaultKubeConfigPath instead of calling os.Exit— Missingarea: descriptionprefix — e.g.frontend: HomeButton: Fix so it navigates to homeorbackend: config: Add enable-dynamic-clusters flag.
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
ffd24d1 to
41f1ce1
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for working on this.
Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.
41f1ce1 to
9ab48b4
Compare
9ab48b4 to
cb92334
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.
cb92334 to
1a2e914
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: illume, YadavAkhileshh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
GetDefaultKubeConfigPath()inbackend/pkg/config/config.gowas callingos.Exit(1)directly when it failed to resolve the current OS user. This is a Go anti-pattern for library code utility functions should return errors and let the caller decide how to handle failure.This PR refactors the function to return
(string, error)and updates all callers accordingly.Related Issue
Fixes #5493
Changes
GetDefaultKubeConfigPath()signature fromstringto(string, error).os.Exit(1)with a proper error return.config.goand test files to handle the returned error.Testing
cd backend && go test ./pkg/config/... ./pkg/kubeconfig/... ./cmd/...