-
Notifications
You must be signed in to change notification settings - Fork 533
Using node pull credentials during builds, image stream imports and pull-through #136
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,252 @@ | ||
| --- | ||
| title: node-artifacts-during-build-and-image-stream-import | ||
| authors: | ||
| - "@rmarasch" | ||
| reviewers: | ||
| - "@dmage" | ||
| - "@bparees" | ||
| - "@adambkaplan" | ||
| approvers: | ||
| - "@dmage" | ||
| - "@bparees" | ||
| - "@adambkaplan" | ||
| creation-date: 2019-12-02 | ||
| last-updated: 2020-01-20 | ||
| status: implementable | ||
| --- | ||
|
|
||
| # Using node pull credentials during build and image stream import | ||
|
|
||
| ## Release Signoff Checklist | ||
|
|
||
| - [x] Enhancement is `implementable` | ||
| - [ ] Design details are appropriately documented from clear requirements | ||
| - [ ] Test plan is defined | ||
| - [ ] Graduation criteria for dev preview, tech preview, GA | ||
| - [ ] User-facing documentation is created in [openshift-docs](https://github.com/openshift/openshift-docs/) | ||
|
|
||
| ## Summary | ||
|
|
||
| Allow OpenShift users to import and use images from any registry configured | ||
| during or after the cluster installation by sharing node's pull credentials | ||
| with `openshift-api`, `builder` and `image-registry` pods. | ||
|
|
||
| ## Motivation | ||
|
|
||
| To have image stream, image registry and builds working closely to node avoids | ||
| complexity and redundancy as each component won't need to mount different | ||
| Secrets to have access to information that is already present on the node's | ||
| filesystem. | ||
|
|
||
| OpenShift should seamlessly use the cluster-wide pull secret provided during | ||
| installation in builds, imagestream imports and pull-through operations. This | ||
| is particularly important for images pulled from `registry.redhat.io`, which | ||
| requires a pull secret. | ||
|
|
||
| Today pull secrets provided during cluster install are available on the | ||
| node's filesystem. If user attempts to import an image stream or pull-through | ||
| from these locations, OpenShift fails as none of `openshift-api`, `builder` or | ||
| `image-registry` use the credentials provided during the installation. | ||
|
|
||
| ### Goals | ||
|
|
||
| - Allow users to use container images from any registry provided during or | ||
| after cluster installation. | ||
| - Allow users to run `builds` based on images hosted in any registry provided | ||
| during or after cluster installation without providing any extra credentials. | ||
| - Allow `image-registry` to execute pull-through using the node's pull | ||
| credentials. | ||
|
|
||
| ### Non-Goals | ||
|
|
||
| - Change the way users manage cluster pull credentials. | ||
| * Additional credentials may still be provided through secrets on namespace. | ||
| - Use Node mirroring config. | ||
| * To use config files files such as `/etc/containers/registries.conf` is not | ||
| part of this proposal. | ||
| - Use Node CAs. | ||
| * This should be done in the future through another enhancement proposal as | ||
| we currently have different sources of truth when it comes to CAs (different | ||
| methods of providing them to different parts of the codebase). | ||
|
|
||
| ## Proposal | ||
|
|
||
| ### User Stories | ||
|
|
||
| As an OpenShift user | ||
| I want to be able to import imagestreams from any image registry that the | ||
| cluster nodes can pull images from | ||
| so that I can use them without manually creating any extra credentials. | ||
|
|
||
| As an OpenShift user | ||
| I want to be able to use, during builds, images from any image registry that | ||
| the cluster nodes can pull images from | ||
| so that I can use these images as base for my own images. | ||
|
|
||
| As an OpenShift user | ||
| I want to be able to pull-through images from any registry that the cluster | ||
| nodes can pull images from | ||
| so that I can use image registry cache to speed-up my builds. | ||
|
|
||
| ### Implementation Details/Notes/Constraints | ||
|
|
||
| This is the node's filesystem path we are taking into account: | ||
|
|
||
| - `/var/lib/kubelet/config.json` contains the node's pull secret. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this file change? E.g. a user can ssh into the machine and append secrets there.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has influence on openshift/cluster-openshift-apiserver-operator#284, i.e. we need to reread that file. Is that the case?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it can be changed via the MCO which causes node reboots so rereading is moot. if the MCO is ever updated to allow node filesystem changes w/o rebooting the nodes, we'd have to setup a watch on the mounted file in our pod. I believe these steps are what lead to the /var/lib/kubelet/config.json being updated:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pull secrets are read in a "per image stream import request" basis, we don't keep it in memory. |
||
|
|
||
| #### Image Stream Import | ||
|
|
||
| - Mount the pull secret as `readOnly`, `hostPath` in | ||
| `openshift-apiserver` deployment under `/var/lib/kubelet/config.json`. | ||
|
|
||
| ```yaml | ||
| volumeMounts: | ||
| - mountPath: /var/lib/kubelet/config.json | ||
| name: node-pull-credentials | ||
| readOnly: true | ||
| volumes: | ||
| - name: node-pull-credentials | ||
| hostPath: | ||
| path: /var/lib/kubelet/config.json | ||
| type: File | ||
| ``` | ||
|
|
||
| - When a user imports an imagestream, `openshift-apiserver` will merge all pull | ||
| credentials found in `/var/lib/kubelet/config.json` with other credentials | ||
| that may exist in the namespace. | ||
|
|
||
|
|
||
| #### Builds | ||
|
|
||
| - `controller-manager` would mount pull credentials inside `build` pod as | ||
| `hostPath`(similar to what is done for Image Stream Import). | ||
| - Builder image parses node's pull credentials and uses them during build, | ||
| merging with other pull credentials that are linked to the `builder` service | ||
| account. If the `BuildConfig` specifies a pull secret, we will continue the | ||
| current behavior of using the provided pull secret as an override. | ||
|
|
||
| #### Registry pull-through | ||
|
|
||
| - As done for Image Stream Import, mount pull credentials inside the image | ||
| registry pod. | ||
| - Pull credentials will then be consumed by the registry. | ||
|
|
||
| ### Risks and Mitigations | ||
ricardomaraschini marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #### Pull credentials for registry may exist on namespace and on node | ||
|
|
||
| User may have created a secret containing credentials for a registry that node | ||
| already has credentials to. | ||
|
|
||
| Mitigations: | ||
|
|
||
| Always prioritize namespace secrets over node's credentials. If a credential | ||
| exists on the namespace, do not use node's credentials. | ||
|
|
||
| #### Pull credentials inside build pod may be visible to the user | ||
|
|
||
| Cluster wide credentials mounted inside the builder pod may be a security risk | ||
| as the user may be able to shell into the pod and copy them. Another possible | ||
| risk is that the user may copy the credentials into a resulting image. | ||
|
|
||
| Mitigations: | ||
|
|
||
| As far as I verified(and this needs to be once more tested) it is impossible | ||
| to spawn a shell inside the builder pods. I also tried to copy the credentials | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ephemeral pods will allow sharing process namespace and thus provide you with access to it. That's a major problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This issue also applies to "openshift-apiserver" during image stream import or is something only for builds?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ricardomaraschini what @soltysh is saying is that even though we block oc rsh into a build-pod, someone (may) be able to create an ephemeral container in the build-pod's process namespace and use that mechanism to get into it instead. @deads2k are there security mechanisms that would prevent someone from abusing this? much like we don't allow you to exec/rsh into a pod you can't "create" yourself (so normal users can't exec/rsh into a privileged container, as i understand it), does the ephemeral container api have similar restrictions?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bparees there is no limitation at this time to my knowledge. its not clear to me that we could use rbac appropriately in a namespace to block the subresource to create the ephemeral container as build pods would be in the same namespace as other pods we would want to allow ephemeral access.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that subresource effectively updates the pod resource. If i can't create privileged pods, i would not expect to be able to edit prvileged pods and i'd expect that restriction to extend to subresources that modify the pod. No?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. spoke with @derekwaynecarr offline. SCC will need to handle this (ensure that a user who can't create a particular pod also cannot invoke this subresource which edits the pod). between @openshift/sig-developer-experience and @openshift/sig-master we need to ensure this is covered by the time the ephemeral container api goes beta (at which point it will be on by default).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have enabled EphemeralContainers on kube-apiserver and simulated the feature, as ec.json: It might be that this is already supported by the current codebase. |
||
| from builder's filesystem into a resulting image but it failed. | ||
|
|
||
| #### Pushing to registries using node's credentials | ||
|
|
||
| If pull credentials are used during builds we may allow users to push images to | ||
| any registry configured on the node. This is not an ideal scenario as cluster | ||
| administrators may not to be aware or wanting this. | ||
|
|
||
| Mitigations | ||
|
|
||
| Pull credentials must be used **only** for pulling images. On pushing we must | ||
| use only user provided(not node) credentials'. | ||
|
|
||
| #### Absence of mounted path on node filesystem | ||
|
|
||
| If the path we are trying to mount through `hostPath` directive does not exist | ||
| on the node where the pod is running the pod won't come up. | ||
|
|
||
| Mitigations | ||
|
|
||
| Mounted path always exist on worker nodes. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about master nodes?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. The confirmation we have is that this path does not exist only on bootstrap nodes. @adambkaplan could we tag someone from node's team to get a final answer on this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cgwalters can you please confirm that the cluster pull secret always exists at
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to make that permanent "ABI" though; see also openshift/machine-config-operator#1190 which proposes ensuring all MCO configuration (including pull secret) lives in I think probably the best bet is for whatever wants it to parse the kubelet config or so perhaps? Or, I guess we could make
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is clear that we must make the file path easily configurable.
@cgwalters Do you mean to parse
I guess this would take some time to get done and we already have some PRs in place to implement this enhancement. I am inclined to stick with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cgwalters are you going to change root-dir for kubelet? |
||
|
|
||
| #### Image stream secrets endpoint | ||
|
|
||
| Currently `builder` pod obtains a set of credentials by a request to | ||
| `openshift-apiserver` done by the `openshift-controller-manager`, this approach | ||
| could create a data leak if we decide to use the same endpoint to also return | ||
| Node's credentials as well. | ||
|
|
||
| Mitigations | ||
|
|
||
| We should not change the endpoint behavior. By mounting the Node's pull | ||
| credentials inside the `builder` pod we don't need to change the endpoint as | ||
| the `builder` pod can merge credentials internally. Node's pull secret should | ||
| never be exposed through any API endpoint. | ||
|
|
||
| #### Pull secrets may be exposed through an ephemeral container | ||
|
|
||
| Kubernetes implements a feature that allows users to temporary create | ||
| [ephemeral](https://kubernetes.io/docs/concepts/workloads/pods/ephemeral-containers/) | ||
| containers into a running pod. This could potentially allow users to copy | ||
| mounted pull secrets from a build pod as the ephemeral pod may allow `rsh`. | ||
|
|
||
| Mitigations | ||
|
|
||
| Build pods run using special permissions and regular users are not be able to | ||
| spawn them due to it. As an user is not allowed to spawn a build pod it should | ||
| also not be allowed to change/patch a running build pod definition by inserting | ||
| ephemeral containers to it. We need to ensure that this is implemented once the | ||
| ephemeral container API goes beta and ephemeral containers are enabled by | ||
| default. | ||
|
|
||
| ## Design Details | ||
|
|
||
| ### Test Plan | ||
|
|
||
| - Create a project and attempt to import images streams from | ||
| `registry.redhat.io` without provide any other pull credential. | ||
| - Pull OpenShift's `base image` during a build. | ||
| - Attempt to use an image from `registry.redhat.io` as input/source during a | ||
| (build)[https://docs.openshift.com/container-platform/4.2/builds/creating-build-inputs.html#image-source_creating-build-inputs] | ||
| - Attempt, on build, to **push** images to a registry for which credentials | ||
| only exist on node. This should fail as node's pull credentials are only used | ||
| when pulling images. | ||
| - Try to pull-through OpenShift's `base image`. | ||
|
|
||
| ### Graduation Criteria | ||
|
|
||
| #### Tech Preview | ||
|
|
||
| Not applicable. | ||
|
|
||
| #### Generally Available | ||
|
|
||
| 1. QE has test cases for all scenarios defined on Test Plan. | ||
| 2. Regressions tests are passing. | ||
| 3. Documentation in place regarding how node's credentials are used during | ||
| builds. | ||
|
|
||
| ### Upgrade / Downgrade Strategy | ||
|
|
||
| Does not apply. | ||
|
|
||
| ## Implementation History | ||
|
|
||
| 2019-12-02: Initial draft | ||
| 2019-12-03: Added node's docker certificates to the enhancement proposal. | ||
| 2019-12-04: Added extra test cases to our Test Plan. | ||
| 2019-12-05: Using node credentials only for pull, never for push. | ||
| 2019-12-10: Removing node's certificates from this proposal scope. | ||
| 2019-12-12: Added pull-through support. | ||
| 2019-12-13: Added note on `readOnly` and image registry pull-through. | ||
| 2020-01-20: Added note on `ephemeral` containers under Risks and Mitigations. | ||
|
|
||
| ## Infrastructure Needed | ||
|
|
||
| None. | ||
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.
What I'm missing here is an explicit opt-in for that functionality, not every cluster-wide secret can and should be exposed. I see the requirement, but the risks are much higher, so this should not be a default, rather an opt-in.
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.
@soltysh this enhancement is narrowly focused on the
pull-secretadded to theopenshift-confignamespace at install time. This pull secret is implicitly shared cluster-wide on all nodes - I am not aware of any opt-out other than deleting this secret.I agree that a general "share any secret across a cluster" feature should absolutely be opt-in. DevEx has discussed this a bit as a future enhancement proposal [1]
[1] https://issues.redhat.com/browse/DEVEXP-426