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
Make it possible to shell into any container, even if it has no shell #10834
Comments
yes I've hit this several times. We added oc rsh that provides a helper for -itp -- bash but a more generic static linking solution would be good that works on any container It can't totally depend on busybox (has to be replaceable/configurable) because we can't ship/support busybox. Probably bonus points for allowing this to be more sophisticated down the road and inject other things (statically linked ssh) |
Is this something we can prioritize? cc @timstclair Anyone up for this? |
I don't think we'd want to override the image's shell, it would have to be a fallback iff the shell is missing. |
@luxas I have this problem as well, see #35584 for how I'd like to address it. I'm looking for additional use cases, so your input would be most welcome. This isn't a priority for SIG Node (at least as defined in the tentative 2017Q1 road map), but it's the only thing I'm attempting to contribute so it's a priority for me. |
I think I disagree with Clayton - if we offer this it should be consistent
functionality. This doesn't have to replace exec, but it should work the
same in all cases. Having a static busybox seems like a good start, but it
does become one more thing that nodes need to have installed or that we
ship with kubernetes.
…On Wed, Jan 4, 2017 at 1:43 PM, Lee Verberne ***@***.***> wrote:
@luxas <https://github.com/luxas> I have this problem as well, see #35584
<#35584> for how I'd like to
address it. I'm looking for additional use cases, so your input would be
most welcome.
This isn't a priority for SIG Node (at least as defined in the tentative
2017Q1 road map), but it's the only thing I'm attempting to contribute so
it's a priority for me.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVDAldH-SrWZegijl9qK_V_pa6NkRks5rPBKLgaJpZM4FTpIs>
.
|
Let me clarify - it is unacceptable for exec to change to magically override the built in shell, because that is not backwards compatible and breaks large classes of apps on Kube. It's acceptable to make static injection a net new mechanism, and might be acceptable to fallback. I also don't want a generic shell command that is tied too closely with one particular shell impl - we would not be able to ship that for lots of reasons, most of which include support. So I'm -1 to a "special" exec command that uses a shell binary we ship always. |
I didn't mean to imply overriding. IMO `exec` should stay literal - run
this command in the user's container. If that command doesn't exist, fail.
This proposal was always about adding a net new shell API that was
guaranteed to work on any container without the container needing to have
any particular files in its image.
So if you're -1 on that, can you explain why? The main reason many people
do `FROM busybox` or `FROM alpine` is to have a shell and basic tools, but
as soon as they do that they are beholden to every CVE in those not-my-app
layers. It seems best to me to absolve users of the need to do that, if
possible.
…On Thu, Jan 5, 2017 at 8:00 AM, Clayton Coleman ***@***.***> wrote:
Let me clarify - it is unacceptable for exec to change to magically
override the built in shell, because that is not backwards compatible and
breaks large classes of apps on Kube. It's acceptable to make static
injection a net new mechanism, and *might* be acceptable to fallback.
I also don't want a generic shell command that is tied too closely with
one particular shell impl - we would not be able to ship that for lots of
reasons, most of which include support. So I'm -1 to a "special" exec
command that uses a shell binary we ship always.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVMQn3MvMbIRjl2ZNle7DO5qUOeNmks5rPROxgaJpZM4FTpIs>
.
|
It's a requirement for the runtimes rather than the kubelet, and the CRI will need to add support. It's the run times that would have to ship a binary. Implementation would be pretty straightforward for the current container-based runtimes and slightly more difficult for hypervisor/cloud runtimes. This solution is constrained to a single static binary specified by the Kubernetes developers, limiting its usefulness. I'd much prefer to be able to use an arbitrary container image for debugging, but the two aren't mutually exclusive. |
I'm ok with a flag that says "inject shell" that makes it opt in. But we
(red hat) probably won't support busybox for that, so my personal ask is
that we try to define what API injected shell is expected to follow before
we introduce it - has to be really carefully thought through.
Agree with Lee's points that this won't work with all runtimes, and that
the real problem isn't just a shell binary, it's the other tools (gdb,
crash dump analysis, tar) that really become important. I'd probably lean
towards a solution like what Lee is proposing than this.
…On Thu, Jan 5, 2017 at 12:23 PM, Lee Verberne ***@***.***> wrote:
It's a requirement for the runtimes rather than the kubelet, and the CRI
will need to add support. It's the run times that would have to ship a
binary. Implementation would be pretty straightforward for the current
container-based runtimes and slightly more difficult for hypervisor/cloud
runtimes.
This solution is constrained to a single static binary specified by the
Kubernetes developers, limiting its usefulness. I'd much prefer to be able
to use an arbitrary container image for debugging, but the two aren't
mutually exclusive.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p-0ixjpQVSQ9ekBmGTsBtaPgvhrvks5rPScGgaJpZM4FTpIs>
.
|
I'm fine with more robust solutions - this was filed as an idea to solve
the most immediate piece of the problem, but it's just an idea :)
@smarterclayton should know by now that 96.3% of my ideas are outright
terrible or at the very least are least off-target.
On Thu, Jan 5, 2017 at 10:16 AM, Clayton Coleman <notifications@github.com>
wrote:
… I'm ok with a flag that says "inject shell" that makes it opt in. But we
(red hat) probably won't support busybox for that, so my personal ask is
that we try to define what API injected shell is expected to follow before
we introduce it - has to be really carefully thought through.
Agree with Lee's points that this won't work with all runtimes, and that
the real problem isn't just a shell binary, it's the other tools (gdb,
crash dump analysis, tar) that really become important. I'd probably lean
towards a solution like what Lee is proposing than this.
On Thu, Jan 5, 2017 at 12:23 PM, Lee Verberne ***@***.***>
wrote:
> It's a requirement for the runtimes rather than the kubelet, and the CRI
> will need to add support. It's the run times that would have to ship a
> binary. Implementation would be pretty straightforward for the current
> container-based runtimes and slightly more difficult for hypervisor/cloud
> runtimes.
>
> This solution is constrained to a single static binary specified by the
> Kubernetes developers, limiting its usefulness. I'd much prefer to be
able
> to use an arbitrary container image for debugging, but the two aren't
> mutually exclusive.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/kubernetes/issues/
10834#issuecomment-270702682>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABG_p-
0ixjpQVSQ9ekBmGTsBtaPgvhrvks5rPScGgaJpZM4FTpIs>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVC28WNIKs6o7QomMBiRkFeg1aF95ks5rPTN_gaJpZM4FTpIs>
.
|
Well, since I was very skeptical of Lee's changes in terms of scope, I
can't disagree that this is potentially a much simpler path. I think the
unfortunate choice is between a limited experience that enables you to at
least look at filesystems and potentially download other things into the
container for further debugging, or a much more complete story that is also
very complex (requires mutable pods, lots of design impact, etc). Could we
somehow bridge the two or find an in between spot?
…On Thu, Jan 5, 2017 at 1:21 PM, Tim Hockin ***@***.***> wrote:
I'm fine with more robust solutions - this was filed as an idea to solve
the most immediate piece of the problem, but it's just an idea :)
@smarterclayton should know by now that 96.3% of my ideas are outright
terrible or at the very least are least off-target.
On Thu, Jan 5, 2017 at 10:16 AM, Clayton Coleman ***@***.***
>
wrote:
> I'm ok with a flag that says "inject shell" that makes it opt in. But we
> (red hat) probably won't support busybox for that, so my personal ask is
> that we try to define what API injected shell is expected to follow
before
> we introduce it - has to be really carefully thought through.
>
> Agree with Lee's points that this won't work with all runtimes, and that
> the real problem isn't just a shell binary, it's the other tools (gdb,
> crash dump analysis, tar) that really become important. I'd probably lean
> towards a solution like what Lee is proposing than this.
>
> On Thu, Jan 5, 2017 at 12:23 PM, Lee Verberne ***@***.***>
> wrote:
>
> > It's a requirement for the runtimes rather than the kubelet, and the
CRI
> > will need to add support. It's the run times that would have to ship a
> > binary. Implementation would be pretty straightforward for the current
> > container-based runtimes and slightly more difficult for
hypervisor/cloud
> > runtimes.
> >
> > This solution is constrained to a single static binary specified by the
> > Kubernetes developers, limiting its usefulness. I'd much prefer to be
> able
> > to use an arbitrary container image for debugging, but the two aren't
> > mutually exclusive.
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/kubernetes/kubernetes/issues/
> 10834#issuecomment-270702682>,
> > or mute the thread
> > <https://github.com/notifications/unsubscribe-auth/ABG_p-
> 0ixjpQVSQ9ekBmGTsBtaPgvhrvks5rPScGgaJpZM4FTpIs>
>
> > .
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/kubernetes/issues/
10834#issuecomment-270715713>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/
AFVgVC28WNIKs6o7QomMBiRkFeg1aF95ks5rPTN_gaJpZM4FTpIs>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p8JtD82p4NfNm6dc1WThoUHAItp5ks5rPTSZgaJpZM4FTpIs>
.
|
Agreed. In the very best case my proposal will take a long time to fully implement, but the complete troubleshooting story is important for Kubernetes overall. Ideally we could find a limited solution that makes progress towards a complete solution, like The most contentious part of my proposal is mutating the pod spec, but that's not a requirement. We could instead create a new resource to model this sort of ephemeral troubleshooting container, but there where concerns in SIG node about the proliferation of container types and backwards compatibility with tools that introspect the pod (e.g. for security/auditing). I can't think of a way to inject a binary in service of a more complete solution. Binary injection seems brittle if done simply and a larger, diverging problem to do robustly. |
What if we *always* injected a toolbox volume into every container, and you
could make `kubectl toolbox -ti podname -c container` be the equivalent of
`kubectl exec -ti podname -c container $(kubectl get pod podname -o
go-template='.status.toolboxPath')/bin/bash`
…On Thu, Jan 5, 2017 at 1:20 PM, Lee Verberne ***@***.***> wrote:
Agreed. In the very best case my proposal will take a long time to fully
implement, but the complete troubleshooting story is important for
Kubernetes overall. Ideally we could find a limited solution that makes
progress towards a complete solution, like kubectl debug to troubleshoot
a duplicate pod before being able to troubleshoot a running one.
The most contentious part of my proposal is mutating the pod spec, but
that's not a requirement. We could instead create a new resource to model
this sort of ephemeral troubleshooting container, but there where concerns
in SIG node about the proliferation of container types and backwards
compatibility with tools that introspect the pod (e.g. for
security/auditing).
I can't think of a way to inject a binary in service of a more complete
solution. Binary injection seems brittle if done simply and a larger,
diverging problem to do robustly.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVLSpOU_Nh8_DDVsNlScxnv0wD-9eks5rPV6fgaJpZM4FTpIs>
.
|
Assuming this is implemented using something like container image volumes (i.e. reusing existing image build system, distribution channels, resource efficiency via caching & layer sharing, etc) it has only a few disadvantages:
That being said, this suggestion simplifies many things. # 2 and # 3 can be solved in another fashion. # 4 detracts from user experience but isn't the end of the world and gcr.io/google_containers could host a canonical version. We could relax the requirement for binary version decoupling in # 1 if everyone really likes this solution, especially if we could figure out a way to asynchronously update the container image volumes in the future. I am having a little trouble getting past the awkwardness of squatting on a filesystem path like '/k8s' for every container. It feels like meddling in the container's affairs, but I could get over it if it's a path forward. |
On Thu, Jan 5, 2017 at 3:08 PM, Lee Verberne ***@***.***> wrote:
Assuming this is implemented using something like container image volumes (i.e. reusing existing image build system, distribution channels, resource efficiency via caching & layer sharing, etc) it has only a few disadvantages:
It couples troubleshooting tools to pod creation, and one of our goals was to get the freshest utilities at debug time and free us from updates due to CVEs unrelated to our code.
I'm not sure CVEs in the debug tools matter much?
It constrains troubleshooting to a single, previously configured toolbox. We (Google) care about this because we'd like to implement a security policy where audit events depend on what image is run. (e.g. a shell triggers an alert but the automated troubleshooting script doesn't)
It doesn't help troubleshoot a container that's crashlooping as exec is unavailable
How would you like to debug such a container?
Once we have shared PID namespaces, we could do this as a taglong
container, rather than a volume. It could do everything but access
the filesystem of the container under debug.
Another alternative might be to have a `debugCommand[]` and run that
instead of the main command if it has crashed X times in Y seconds.
e.g. gdbstub...
It means we have construct an awkward container image where bash (et. al.) uses a path of /k8s/bin, /k8s/etc, /k8s/lib, etc (for .status.toolboxPath='/k8s')
That being said, this suggestion simplifies many things. # 2 and # 3 can be solved in another fashion. # 4 detracts from user experience but isn't the end of the world and gcr.io/google_containers could host a canonical version.
We could relax the requirement for binary version decoupling in # 1 if everyone really likes this solution, especially if we could figure out a way to asynchronously update the container image volumes in the future.
I am having a little trouble getting past the awkwardness of squatting on a filesystem path like '/k8s' for every container. It feels like meddling in the container's affairs, but I could get over it if it's a path forward.
Sorry, I would make it random, like a UUID, which is why I suggested
it be exposed as an API field rather than a const.
|
Sorry, I would make it random, like a UUID, which is why I suggested
it be exposed as an API field rather than a const.
Oh, ok. I assumed a configurable default. A random path is more compelling.
It makes things like $PATH, shared libraries & configuration files more
difficult. We could probably use some sort of exec shim to update $PATH and
what not, but getting bash to find /<UUID>/etc/bashrc probably isn't
possible. hmmm.
|
One concern I have with auto mounting debug tools into every container is it makes the tools available to potential attackers as well. For example, if an arbitrary command execution vulnerability gets you to a shell, a lot of attack paths become much easier. I thought this was the original motivation for the tool being discussed, so that we didn't need to build a shell into every container? |
Tim: I think we're still at the brainstorming stage. :)
…On Fri, Jan 6, 2017 at 10:34 AM, Tim St. Clair ***@***.***> wrote:
One concern I have with auto mounting debug tools into every container is
it makes the tools available to potential attackers as well. For example,
if an arbitrary command execution vulnerability gets you to a shell, a lot
of attack paths become much easier. I thought this was the original
motivation for the tool being discussed, so that we didn't need to build a
shell into every container?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10834 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFVgVMn_nI5dbab1nhadEyu6bUFmCcmKks5rPolTgaJpZM4FTpIs>
.
|
FYI @EricMountain-1A |
I've updated the proposal in #35584 with some of the ideas discussed here (and added a link). The latest revision of that proposal favors an approach which I think will give a better user experience. It's not exactly what's being requested in this issue, but it may serve the same purpose. You'd have the choice between creating a copy of a pod with arbitrary changes (such as image or entrypoint) or attaching a new container to the running pod (initially with access to pod volumes but perhaps eventually with access to container filesystems). I'd be interested to know whether this meets your needs. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Does this get subsumed by debug containers, or is this a slightly different issue? |
@kfox1111 I think this will be solved by debug containers, but if you disagree let me know and I'll unassign. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
/lifecycle frozen |
With #59484 merged, this has become "possible". We still have a way to go to make it useful out of the box and to graduate the feature. I'm going to close this issue because we have a few others tracking this. If you're interested in following along, keep an eye on issue #27140 and kubernetes/enhancements#277. /close |
@verb: Closing this issue. 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. |
Today
kubectl exec sh
depends on a shell being present in each container image. I think we can do better.What if we had something like
kubectl sh
which would run a statically linked busybox (or something like that) that was provided by the host node and made that enter the cgroups and namespaces and chroot of the container? We already have a short list of features that only work if the host node has some piece of software installed, this would be the same.The text was updated successfully, but these errors were encountered: