Skip to content
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

Validate Image when creating Notebook Server #56

Open
wg102 opened this issue Feb 10, 2021 · 9 comments
Open

Validate Image when creating Notebook Server #56

wg102 opened this issue Feb 10, 2021 · 9 comments

Comments

@wg102
Copy link
Contributor

wg102 commented Feb 10, 2021

The UI does not block the use of a custom image that does not exist. A user can write anything as a custom ID and still be able to click 'Create'. This leads to a forever pending Notebook Server.

With a separation of user available container, having that separation provides a pretty straightforward context.

Possible idea: Implement an API layer. server side check for existence of that image. limit to pullable images.

@blairdrummond
Copy link

Another approach, get the Allow-List from the gatekeeper CRD, and then use that

https://github.com/StatCan/gatekeeper-policies/blob/master/general/container-allowed-images/constraint.yaml

As @justbert mentions, we might need a role/role-binding for JupyterAPIs to access this resource, but I think this is probably the most elegant solution.

NOTE: This does not prevent people from requesting "valid" but non-existent images. But I think it does prevent people from trying to access things that are not allowed.

I might be inclined to go the route of

if not gatekeeper_allows(image_name):
    raise Exception("This image is not allowed by AAW policy. Please refer to the Gatekeeper Allow-list")
elif image_name.startswith('$OUR_REGISTRY') and not in_acr(image_name):
    raise Exception("This image is not available. It may be a typo or maybe the image was removed.")
else:
    return True # All good!

This would check both the gatekeeper policies and the local registry, so that we know that the image is valid and exists.

(I am not sure if there's a nice way to see if an image exists in a remote registry)

@justbert
Copy link

justbert commented Apr 6, 2021

I'm not super familiar with the Docker registry protocol, but I believe there's a way to pull down metadata for an image.

@ca-scribner
Copy link
Contributor

I'm 💯 for giving useful feedback to the users ("this image points to an allowed repository but we cannot find an image with that name/tag", "this image points to a repository that is outside the allowed list", etc...). That way it is easier for people to know what's what.

@justbert agreed I think you can pull metadata from docker registries via some docker commands or simple gets?

If we have to, programming this vetting script to parse the allow list is ok, but ideally the allow/deny logic would be in a single entity so that we cannot have a mismatch (website allows some, gatekeeper actually allows others). I'd order the solution preference as:

  1. Solution that lets both k8s and portal ask "yes/no" about an image the exact same way. eg: (not an actual suggestion, just an analogy) both k8s and portal post image_name to the same gatekeeper endpoint. This way there's no possibility of disagreement
  2. k8s and portal invoke the code separately, but they both use the same library (eg: if gatekeeper's logic is in go somewhere in our github, we import the same library into whatever decides yes/no on the webpage validation)
  3. k8s and portal logic are separately implemented, where both interpret the same allow-list format file (either the format we use now, or some more generic format that both k8s and portal codes read)

Last thought is whether there's a way to make this upstreamable. At a minimum I see upstream benefiting from validation logic on whether an image exists (maybe with a setting of whether a missing image is just a warning vs a blocking error on the form). Maybe we can make that part upstreamable, with an optional allow-list that let's someone configure further restrictions in an allow list? trying to keep our upstream delta small if we can.

@justbert
Copy link

justbert commented Apr 6, 2021

The policy is IN the cluster. Just pull the resource and parse it. If you have a watcher on it via the k8s SDK, the master will notify you of a change to the underlying policy and you can update your cache.

@ca-scribner
Copy link
Contributor

ok so rather than have code that parses the allow list from the text in github, parse it from k8s' current resource instance (eg: what it currently thinks the allow list should be)? That makes sense. And since it is a simple allow sort of list, the potential for logic difference is pretty small anyway

@justbert
Copy link

justbert commented Apr 6, 2021

Yeah, that's what I would do. Though 1. may be best...There MAY be a way to expose the endpoint from the OPA instance under Gatekeeper. Hmmm... Though it would be quite hacky.

@justbert
Copy link

justbert commented Apr 6, 2021

@wg102 wg102 self-assigned this Apr 6, 2021
@wg102 wg102 added the size/M 2-3 days label Apr 7, 2021
@sylus
Copy link
Member

sylus commented Feb 18, 2022

Is this issue being looked at anymore?

@jumana-s jumana-s removed their assignment Apr 29, 2022
@blairdrummond
Copy link

CC @Collinbrown95 , in theory we could strip whitespace here while validating the image, AND the docker registry api allows pulling metadata on the image (as Justin mentioned) to confirm the image exists

https://docs.docker.com/registry/spec/api/#pulling-an-image-manifest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants