cc @bgrant0607 @dchen1107 @yujuhong

Labelling this PR as size/S

GCE e2e test build/test passed for commit 012ec0f42875df068e29cc6267543c34ffa863b4.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Labelling this PR as size/M

GCE e2e build/test failed for commit d69d1152b81e1feb2c9e8801bcde0ad179a8d15f.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot test this github issue: #21138

GCE e2e test build/test passed for commit d69d1152b81e1feb2c9e8801bcde0ad179a8d15f.

• Build Log
• Test Artifacts
• Internal Jenkins Results

LGTM

PR needs rebase

PR changed after LGTM, removing LGTM.

@bgrant0607: Need another LGTM due to a rebase.

GCE e2e test build/test passed for commit 2623fdd.

• Build Log
• Test Artifacts
• Internal Jenkins Results

LGTM

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

GCE e2e test build/test passed for commit 2623fdd.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@liggitt @deads2k

Do you have any concerns about putting image names in NodeStatus? It seems like they might contain sensitive information, and it would be unfortunate to have to restrict access to nodes.

is this the name that matches the container image name? if we expect someone to have access to the node API object, but not access to the bound pods for the node, then yes... this exposes a lot more information.

I'd also be concerned about the size of the node records listing every image they contained, especially as frequently as they are updated

Do you have any concerns about putting image names in NodeStatus? It seems like they might contain sensitive information, and it would be unfortunate to have to restrict access to nodes.

We already restrict access to node resources pretty tightly (cluster-admin sort of roles), so I'm not too concerned about adding this information since its already available on the node itself.

What is the utility is having this information? I'd be more worried about someone trying to use this for scheduling decisions, which is a big security hole if you aren't verifying pull access to local images.

This would allow someone to try to get access to a restricted image, a "smart" scheduler would look for the node that already has that image, schedule it there and bypass the authorization check, thus making a really bad choice. Hopefully that's either a) not why you're doing this, b) you've considered an solved thsi problem, c) you never have the concept of a private image (unlikely).

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

GCE e2e test build/test passed for commit 2623fdd.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue

@deads2k Yes, the information was intended to be used for scheduling.

ImageLocalityPriority:
https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/scheduler/algorithm/priorities/priorities.go#L156

Filed #21560 to discuss this type of policy/convention.

@deads2k Security-wise, I don't see the difference between starting a set of pods spread across all the nodes (and there are many ways to achieve that) vs. choosing the specific node(s) that contain a particular image.

@deads2k Security-wise, I don't see the difference between starting a set of pods spread across all the nodes (and there are many ways to achieve that) vs. choosing the specific node(s) that contain a particular image.

I don't see a reason to help user do it, but I will concede that I can get a similar effect by scaling an RC up and down over and over again. Other than an "always pull policy" (in which case this change does nothing), is anyone looking at a way to secure access to images already on the node?

For the scheduling use case, I think a one-way hash of the image names would actually be good enough.

@deads2k Re. access to images on nodes: not AFAIK. I'll file an issue.

@davidopp @resouer Would obfuscating image names (e.g., using a one-way hash) cause any problems for ImageLocalityPriority?

FYI. Image security issue is currently solved using an admission controller that overrides the image pull policy to be PullAlways.

One way hash of image names is actually going to result in inefficient scheduler behavior in a lot of common cases where the same image is present as multiple image repository tags.

Someone who pulls "mysql:latest" and "mysql:5.8" will in all likelihood have the same underlying image digest (mysql@sha256:....). The hash of the name obscures the commonality of that image. For Docker based images, if the name on the node is to a digest, we should return the digest as the hash rather than further hashing it. Anyone using digests directly (like OpenShift, or some of the other automated systems that use digest) is likely going to not see much benefit from the scheduler work if we do only a hash of the naive name.

Docker 1.10 is much more strongly organized around using digest internally - fairly soon we should have that as the actual image ID on disk.

One way hash of image names is actually going to result in inefficient scheduler behavior in a lot of common cases where the same image is present as multiple image repository tags

+1 for this.

I like the digest idea.

One challenge with that is that the scheduler doesn't have the digest.

The scheduler may have the digest - certainly we do in almost all pods
(openshift transforms tags into digests when the image starts being
tracked). It just depends on having a lot of good integration with a
registry and the changing tags (to trigger deterministic deployments).

The scheduler would at minimum have to parse the docker image to make that
determination - but it's a fairly low cost opportunistic check.

On Mon, Feb 22, 2016 at 12:57 PM, Brian Grant notifications@github.com
wrote:

One challenge with that is that the scheduler doesn't have the digest.

—
Reply to this email directly or view it on GitHub
#21182 (comment)
.