-
Notifications
You must be signed in to change notification settings - Fork 38.9k
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
exec credential provider: InteractiveMode support #99310
exec credential provider: InteractiveMode support #99310
Conversation
Hi @ankeesler. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
staging/src/k8s.io/client-go/pkg/apis/clientauthentication/v1beta1/types.go
Show resolved
Hide resolved
staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go
Outdated
Show resolved
Hide resolved
/ok-to-test |
/remove-sig api-machinery |
/kind feature |
e5b3e80
to
441ab17
Compare
856a1c6
to
4b349da
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ankeesler, liggitt 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 |
/retest Review the full test history for this PR. Silence the bot with an |
4 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
Looking into test failure. It might be flake #102842 (comment) /hold |
The value here is that the exec plugin author can use the kubeconfig to assert how standard input is treated with respect to the exec plugin, e.g., - an exec plugin author can ensure that kubectl fails if it cannot provide standard input to an exec plugin that needs it (Always) - an exec plugin author can ensure that an client-go process will still call an exec plugin that prefers standard input even if standard input is not available (IfAvailable) Signed-off-by: Andrew Keesler <akeesler@vmware.com>
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
4b349da
to
e427d2f
Compare
pull-kubernetes-integration was an actual test failure, fixed by https://github.com/ankeesler/kubernetes/blob/cd83d89ac94c5b61fdd38840098e7223e5af0d34/test/integration/client/exec_test.go#L693 pull-kubernetes-unit failure looks like https://storage.googleapis.com/k8s-gubernator/triage/index.html?test=TestMain#bf11e79583121ff73ec4 |
/lgtm |
What type of PR is this?
/kind bug
(My guilty conscious is saying that this is really actually a feature, but I am submitting it as a bug because we would like to fix #98451 ASAP)
What this PR does / why we need it:
users.user.exec.interactiveMode
to kubeconfig which is used to specify how standard input should be handled in relation to the exec pluginExecCredential.Spec.Interactive
to communicate to exec plugin whether standard input was passed to itexec.GetAuthenticator()
fails if the exec plugin requires standard input, but it is not availableWhich issue(s) this PR fixes:
Fixes #98451
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
/sig auth
/cc @liggitt @enj
FYI - I have been using https://github.com/ankeesler/sample-exec-plugin to test this manually.