Hephy users can access each others private-registry images, via k8s image cache

[rwos]

This is the hephy version of kubernetes/kubernetes#18787. Obviously only relevant if you (want to) run hephy as a strict (adversarial users) multi-tenant setup.

It goes something like this:

User A creates an app and pulls from his private registry:

• deis create --no-remote a-app
• deis config:set PORT=8080 -a a-app
• deis registry:set -a a-app username=foo password=bar
• deis pull -a a-app privateregistry.example.com/a-app

User B can now do this:

• deis create --no-remote b-app
• deis config:set PORT=8080 -a b-app
• deis pull -a b-app privateregistry.example.com/a-app

That should fail (User B can't access privateregistry.example.com), but it works because the imagePullPolicy on the apps is set to "IfNotPresent". Setting it to "Always" would help (as would using the AlwaysPullImages admission controller), but that obviously makes things slower.

---

With hephy, we could actually solve this better/easier than k8s itself - we could just always attempt a pull in the controller (only on deis pull, and we don't have to download the whole image), and check if that works. And then leave the PullPolicy as "IfNotPresent" and still get the speed increase from that, on the k8s side of things.

[Cryptophobia]

Hi @rwos , yes this is a known limitation for multi-tenant Hephy but whatever we implement we need to make sure it is backwards compatible with how ECR and other registries currently work.

[Cryptophobia]

@rwos can you take a look at this issue again. Seems to be failing to pull the credentials for users who are using GCR.

[rwos]

@Cryptophobia Can't really assist here, sorry - I don't have a GCR/ECR setup to test this, and I don't really see why this doesn't work on GCR/ECR (haven't used either of them at all). But I guess the revert is okay.

[Cryptophobia]

@rwos, I really like the PR and this is just a tradeoff between old behavior and new security. The problem is that users who use ECR/GCR have the registry credentials set in the values.yaml file and they expect that once set there they do not have to set them per app. This PR adds extra security which is good and preferable.

Is it possible to add better Exception output here https://github.com/pngmbh/controller/blob/1ce3bc5632b2f5855ba775cc173100d1fa34fda2/rootfs/api/models/release.py#L139-L144 so that users know that they must set the docker registry credentials per each application?

This is a non-backwards change and I am okay with it since it improves security. Should we just add your code and then skip image_access_check if using ECR or GCR like an user attempted here: teamhephy/controller#101 ?