[WIP] Improve kubectl.GetPrinter #39095
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #38779
Addresses an item from: #38768
Release note:
This patch prevents potential panics by callers of
kubectl.GetPrinter
by avoiding returning both anil
error andprinter
when the specified output format iswide
.I am hoping this can at least serve to start a discussion for how to handle this case in
kubectl.GetPrinter
. Both PrinterForMapping and PrinterForCommand make a call tokubectl.GetPrinter
(PrinterForMapping
calls this indirectly through a call to PrinterForCommand) and only avoid dealing with both anil
printer andnil
error (when output format ==wide
) due toPrinterForCommand
simply returning the results it receives fromkubectl.GetPrinter
, andPrinterForMapping
creating an entirely new printer when this is the case.An alternative solution to this could be to still return a
nil
printer in kubectl.GetPrinter, but return an error (such asunsupported output format
) and havePrinterForCommand
handle this error by either creating aNewHumanReadablePrinter
at this point, or delegating the error toPrinterForMapping
which already creates aNewHumanReadable
printer whengeneric == false
forkubectl.GetPrinter
.@fabianofranz