-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
client exec auth: updates for 1.11 #8932
client exec auth: updates for 1.11 #8932
Conversation
Deploy preview for kubernetes-io-vnext-staging processing. Built with commit 19dbe40 https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5b171ba5b13fb15e151d1d9d |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing 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 |
Deploy preview for kubernetes-io-vnext-staging processing. Built with commit 19dbe40 https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5b171ba58df8940831d5e731 |
This somewhat overlaps with #8826 |
Since yours already has a lgtm let's just wait on that then I'll rebase
once it's merged.
…On Tue, Jun 5, 2018, 4:33 PM Andrew Lytvynov ***@***.***> wrote:
This somewhat overlaps with #8826
<#8826>
I can drop that PR if you'd like to update all in one go.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8932 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACO_XRpweRDLwfO3haGjQWm23Zl6WMgaks5t5xU_gaJpZM4UbwJW>
.
|
/assign @zacharysarah |
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.
@ericchiang 👋 I left some feedback but there are some merge conflicts that may affect this review. Please resolve the merge conflicts then @mention
me for a follow-up. 👍
} | ||
}' | ||
``` | ||
The executed command is expected to print an `ExceCredential` object to `stdout`. `k8s.io/client-go` |
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.
Replace lines 753-754 with:
The executed command prints an `ExecCredential` object to `stdout`. `k8s.io/client-go`
authenticates against the Kubernetes API using the returned credentials in the `status`.
|
||
When plugins are executed from an interactive session, `stdin` and `stderr` are directly | ||
exposed to the plugin so it can prompt the user for input for interactive logins. | ||
When run from an interactive session `stdin` is exposed directly to the plugin. Plugins should use a |
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.
Replaces lines 756-758 with:
When run from an interactive session, `stdin` is exposed directly to the plugin. Plugins should use a
[TTY check](https://godoc.org/golang.org/x/crypto/ssh/terminal#IsTerminal) to determine if it's
appropriate to prompt a user interactively.
@ericchiang Bump |
/lgtm |
closing in favor of rebased version in #9154 /close |
/cc @awly
ref kubernetes/kubernetes#61796