backend: kubeconfig: Stop treating empty path and missing file as load errors#5731
Conversation
…d errors
LoadContextsFromFile previously returned
error loading kubeconfig files: error reading kubeconfig file:
open : no such file or directory
whenever the caller passed an empty path, and
error loading kubeconfig files: error reading kubeconfig file:
open .../kubeconfigs/config: no such file or directory
whenever the kubeconfig file simply did not exist on disk. The default
in-cluster mode passes "" for the static kubeconfig (the service
account token is the actual credential), and the dynamic-cluster
persistence file lives at ~/.config/Headlamp/kubeconfigs/config which
is absent on a fresh in-cluster pod until the user adds the first
dynamic cluster. Both situations are normal, yet createHeadlampHandler
logged them at ERROR level on every startup, polluting pod logs.
Treat both as no-ops in LoadContextsFromFile:
- Empty path: nothing to read, return (nil, nil, nil).
- Path resolves to a file that does not exist
(errors.Is(err, fs.ErrNotExist)): return (nil, nil, nil).
- Other read errors (permission denied, etc.): unchanged.
Also switch the existing fmt.Errorf from %v to %w so callers can use
errors.Is on the wrapped error in the future.
Update the existing invalid_file subtests in TestLoadAndStoreKubeConfigs
and TestLoadContextsFromKubeConfigFile to reflect the new contract,
and add empty_path regression subtests.
Issue: kubernetes-sigs#4724
Issue: kubernetes-sigs#4401
Signed-off-by: Govind Pandey <govindup63@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: govindup63 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Eliminates two spurious ERROR log lines emitted on every in-cluster Headlamp startup by treating an empty kubeconfig path and a non-existent kubeconfig file as a no-op rather than a load failure. The change is confined to LoadContextsFromFile, with existing callers (startup loaders in cmd/headlamp.go and the watcher) inheriting the new behavior unchanged.
Changes:
- In
LoadContextsFromFile, short-circuit when the path is empty and ignorefs.ErrNotExist, returning(nil, nil, nil). - Switch the read-error wrap from
%vto%wto allowerrors.Is/Ason the chain. - Rename the misnamed
invalid_filesubtests tomissing_file, flip them toNoError, and add newempty_pathsubtests inkubeconfig_test.go.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| backend/pkg/kubeconfig/kubeconfig.go | Skip empty paths and treat missing kubeconfig files as no-op; use %w wrap. |
| backend/pkg/kubeconfig/kubeconfig_test.go | Rename/flip invalid_file to missing_file and add empty_path regression tests. |
Summary
Two long-standing issues report the same root cause: a default in-cluster Headlamp deployment writes two
ERRORlines to its pod log on every startup, even when nothing is wrong.The first line comes from
LoadAndStoreKubeConfigs(store, "", ...)because in-cluster mode leavesKubeConfigPathempty (the service account token is the actual credential). The second comes fromLoadAndStoreKubeConfigs(store, ~/.config/Headlamp/kubeconfigs/config, ...): this file is owned by Headlamp itself and is absent on a fresh pod until the user adds the first dynamic cluster. Both situations are normal but were both treated asERROR.Related Issues
Issue: #4724
Issue: #4401
Changes
backend/pkg/kubeconfig/kubeconfig.go::LoadContextsFromFile(nil, nil, nil). There is nothing to read.os.ReadFilefails witherrors.Is(err, fs.ErrNotExist), return(nil, nil, nil). The file simply isn't there.fmt.Errorf("...: %v", err)to... %w, so callers can useerrors.Ison the chain.backend/pkg/kubeconfig/kubeconfig_test.goinvalid_filesubtests inTestLoadAndStoreKubeConfigsandTestLoadContextsFromKubeConfigFiletomissing_file, and flip their assertion toNoError. The old name was misleading because the test was passing a path to a file that does not exist on disk, not a malformed kubeconfig.empty_pathsubtest to each of the two test groups, covering the in-cluster scenario directly.The fix cascades through every caller:
createHeadlampHandlerat the two startup load sites (cmd/headlamp.go:522and:541), and the two reload paths inpkg/kubeconfig/watcher.go. None of those call sites needed to change.Steps to Test
cd backend && go test ./pkg/kubeconfig/ -run 'TestLoadAndStoreKubeConfigs|TestLoadContextsFromKubeConfigFile' -v. All subtests pass, including the newmissing_fileandempty_pathcases.kubeconfig.goand rerun: bothmissing_fileandempty_pathsubtests fail with exactly the error from the reporters, demonstrating the regression test catches the bug.cd backend && go test ./pkg/kubeconfig/... ./cmd/... -race. Clean.cd backend && go build ./... && go vet ./pkg/kubeconfig/ ./cmd/. Clean.I also ran the full backend suite. One race finding in
pkg/telemetry/TestResponseWriterHijack_SucceedsWithUnderlyingHijackerexists onmainindependent of this change (git stash && go test ./pkg/telemetry/...still fails) and is out of scope here.Notes for the Reviewer
--kubeconfig=/typo/pathwill no longer see anERRORline for that typo, only the (already existing) absence of contexts in the UI. If you'd prefer anINFO-level "skipping nonexistent kubeconfig path: ..." log at the call sites increateHeadlampHandler, I'm happy to add it.%v→%wswap is harmless on its own (the message string is identical) and gives callers anerrors.Is/errors.Asfoothold if they ever need to distinguish error kinds in the future.