-
Notifications
You must be signed in to change notification settings - Fork 415
Implement wrappers in CLI module to allow working with older API versions #3539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c8c90d4
to
18ad332
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
LGTM label has been added. Git tree hash: 9b1aa47ea323dd92b9310b7547ea8c5d56617ce9
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: embik The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-0.28 |
@embik: once the present PR merges, I will cherry-pick it on top of release-0.28 in a new PR and assign it to you. In response to this:
Instructions 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/test-infra repository. |
/retest |
@embik: new pull request created: #3541 In response to this:
Instructions 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/test-infra repository. |
Summary
This PR makes the
kubectl kcp bind
andkubectl kcp claims
commands comatible with kcp 0.27+ at least, probably also older releases.Currently our API does not have a shard, common type for APIBindings, and I did not want to embark in refactoring half the conversion logic just to fix this bug, plus it would have made backporting this into 0.28 impossible.
So instead I created a very lightweight wrapped in the CLI helpers. These turn the Kubernetes objects they wrap in kind of ActiveRecord-like objects that know how to refresh and update themselves, plus they know how to print or check their own status.
The goal was to try to keep the actual plugin logic as linear and typed as possible.
FAQ: Why are we not using the existing conversion funcs to handle permission claims? – because our conversion funcs do not do error handling, as they are meant to always succeed. More specifically, the conversion funcs for PermissionClaims assume that you have already pre-converted them in the APIBinding object to, if necessary, set annotations. This is simply not applicable here and so calling them would not help in detecting unsupported configurations.
What Type of PR Is This?
/kind regression
Related Issue(s)
Fixes #3529
Release Notes