🐛 (CLI): Fix panic from unchecked type assertion in printDeprecationWarnings#5689
🐛 (CLI): Fix panic from unchecked type assertion in printDeprecationWarnings#5689SebTardif wants to merge 1 commit into
Conversation
…cationWarnings Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SebTardif 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 |
|
Hi @SebTardif. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
/retest |
1 similar comment
|
/retest |
|
/retest |
|
@SebTardif: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Problem
In
pkg/cli/cli.goline 586,printDeprecationWarningsuses bare type assertions:p.(plugin.Deprecated)without the comma-ok pattern panics at runtime if any resolved plugin does not implement theplugin.Deprecatedinterface. This can be triggered by external plugins (user-installed binaries) that do not implementDeprecated.The safe comma-ok pattern is already used elsewhere in the same codebase:
pkg/cli/root.go:180:if deprecated, ok := p.(plugin.Deprecated); ok {pkg/cli/init.go:102:if _, isDeprecated := p.(plugin.Deprecated); !isDeprecated {The unsafe assertion on line 586 was introduced on 2023-03-20.
Fix
Use the comma-ok pattern (
deprecated, ok := p.(plugin.Deprecated)), matching the style inroot.go:180. This also eliminates three redundant type assertions per iteration by reusing thedeprecatedvariable.Verification
make lint-fix: 0 issuesgo test ./pkg/cli/...: all pass