Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

design clean support for "image pull inside kubernetes" #121

Closed
cgwalters opened this issue Oct 20, 2021 · 11 comments
Closed

design clean support for "image pull inside kubernetes" #121

cgwalters opened this issue Oct 20, 2021 · 11 comments

Comments

@cgwalters
Copy link
Member

cgwalters commented Oct 20, 2021

Breaking this issue out from discussion in openshift/os#657

We need to make it easy for a Kubernetes node to pull container images for host OS updates in the same way kubelet is doing so. In particular, I want to support pulling from e.g. an in-cluster registry.

There are three major aspects to this:

  • Configuring pull secrets; this can be done today by injecting REGISTRY_AUTH_FILE in the environment. We should also add CLI flags that just wrap the global skopeo --authfile argument.
  • Ensuring that we trust the service CA
  • Joining the pod network - we need to use cluster DNS at least. Digging into things, it seems at least crio has special support for forking a copy of its binary into a designated cgroup. Presumably this aids applying separate cpu/network/io limits to image pulls.
@cgwalters
Copy link
Member Author

cgwalters commented Oct 20, 2021

My strawman would be that we expose in this library something like:

pub struct ImagePullConfiguration {
  pub auth_file: Option<String>, // maps to --authfile
  pub certificate_authority: Option<?>,  // maps to `--src-cert-dir`
  pub network_config: ???,
  pub systemd_config: Option<Vec<String>>, // direct options passed to `systemd-run`, see also https://github.com/coreos/rpm-ostree/blob/main/rust/src/isolation.rs
}

or so, and then implement it by extending how we fork skopeo.

While we're here, I actually also really want to support systemd-run skopeo when run as root so we can use e.g. -p DynamicUser=yes to completely drop privileges.

@cgwalters
Copy link
Member Author

containers-image-proxy now has a config struct.

It's still missing the certificate bits though, and that only allows us to configure for pulls from inside a cluster, it doesn't actually demonstrate it working.

@mkenigs
Copy link
Contributor

mkenigs commented Dec 7, 2021

Trying to understand the conclusion about networking from openshift/os#657 - will we need to do anything special for networking? After debugging a node and oc logging in I can run skopeo inspect docker://image-registry.openshift-image-registry.svc:5000/openshift/cli --username kubeadmin --password $(oc whoami -t) So I'm assuming it'd also work with just --authfile?

Or do we need to fork into a cgroup for resource limits?

@mkenigs
Copy link
Contributor

mkenigs commented Dec 7, 2021

Would we want to copy or factor out somewhere the systemd-run code from https://github.com/coreos/rpm-ostree/blob/main/rust/src/isolation.rs?

@cgwalters
Copy link
Member Author

Sorry, information on this is split across several places. It turns out that I was confused by DNS - openshift/os#657 (comment)

There is no need to switch to a separate network namespace, we can contact the kube service IP addresses from the host. It's just DNS that is special cased.

Or to state this another way, I think the only remaining blocker for the "pull host updates from internal registry" is correctly setting up the certificates.

@mkenigs
Copy link
Contributor

mkenigs commented Dec 8, 2021

Do you still want to add the systemd-run support here or do that separately?

@cgwalters
Copy link
Member Author

The basic support for that landed in containers/containers-image-proxy-rs#15 and I tried to use it in coreos/rpm-ostree#3239 but it's sadly blocking on selinux right now 😢

mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 8, 2021
Will help allow ostree pull images from internal registries
Closes ostreedev/ostree-rs-ext#121
mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 8, 2021
Will help allow ostree pull images from internal registries
Helps ostreedev/ostree-rs-ext#121
mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 8, 2021
Will help allow ostree pull images from internal registries
Helps ostreedev/ostree-rs-ext#121
mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 8, 2021
Will help allow ostree pull images from internal registries
Helps ostreedev/ostree-rs-ext#121
mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 8, 2021
Will help allow ostree pull images from internal registries
Helps ostreedev/ostree-rs-ext#121

Also change authfile type from String to PathBuf
mkenigs added a commit to mkenigs/containers-image-proxy-rs that referenced this issue Dec 9, 2021
Will help allow ostree pull images from internal registries
Helps ostreedev/ostree-rs-ext#121

Also change authfile type from String to PathBuf
mkenigs added a commit to mkenigs/ostree-rs-ext that referenced this issue Dec 9, 2021
Exposes the corresponding options from containers-image-proxy and
skopeo

Also changes authfile type from String to PathBuf for consistency

Helps ostreedev#121
mkenigs added a commit to mkenigs/ostree-rs-ext that referenced this issue Dec 9, 2021
Exposes the corresponding options from containers-image-proxy and
skopeo

Also changes authfile type from String to PathBuf for consistency

Helps ostreedev#121
Depends containers/containers-image-proxy-rs#22
@cgwalters
Copy link
Member Author

cgwalters commented Jan 4, 2022

One thing that came out of a discussion on this is that kubelet doesn't use its own service account to pull containers or anything like that - the credentials are per-pod. The pod can have its own credentials to pull. See https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/

Hence, it likely makes sense to use the MCD's service account.

See also https://docs.openshift.com/container-platform/4.9/registry/accessing-the-registry.html

In some recovery scenarios, it'd be nice to streamline using the kube:admin's credentials when logged into a node via ssh or so. Think e.g. hacking on a node to install a debug kernel via rpm-ostree rebase directly.

--

https://docs.openshift.com/container-platform/4.9/authentication/bound-service-account-tokens.html

For internal registry: https://github.com/openshift/cluster-image-registry-operator/blob/2b82360000454e7e02b6f3b46d1354546dffccbc/bindata/nodecadaemon.yaml
(Yet another load-bearing bash script run as root!)

@mkenigs
Copy link
Contributor

mkenigs commented Jan 4, 2022

Is this correct then?

@cgwalters
Copy link
Member Author

I think but need to verify that we should already have a pull secret associated with the pod.

cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Jan 20, 2022
This is related to ostreedev#121
as well as containers/containers-image-proxy-rs#8
etc.

The CLI code here supports `--authfile`.  However, passing it on
the CLI each time for production use cases pushes complexity to
users.

Add support for global persistent and runtime config files in
`/etc/ostree/auth.json` and `/run/ostree/auth.json`.

Change the default constructor for image pulls to use that by default.

Note that the CLI options override the config defaults.

While we're here, also add `--auth-anonymous` to the CLI, which is
needed to ensure we don't use a config file if present.
cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Jan 20, 2022
This is related to ostreedev#121
as well as containers/containers-image-proxy-rs#8
etc.

The CLI code here supports `--authfile`.  However, passing it on
the CLI each time for production use cases pushes complexity to
users.

Add support for global persistent and runtime config files in
`/etc/ostree/auth.json` and `/run/ostree/auth.json`.

Change the default constructor for image pulls to use that by default.

Note that the CLI options override the config defaults.

While we're here, also add `--auth-anonymous` to the CLI, which is
needed to ensure we don't use a config file if present.
cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Jan 20, 2022
This is related to ostreedev#121
as well as containers/containers-image-proxy-rs#8
etc.

The CLI code here supports `--authfile`.  However, passing it on
the CLI each time for production use cases pushes complexity to
users.

Add support for global persistent and runtime config files in
`/etc/ostree/auth.json` and `/run/ostree/auth.json`.

Change the default constructor for image pulls to use that by default.

Note that the CLI options override the config defaults.

While we're here, also add `--auth-anonymous` to the CLI, which is
needed to ensure we don't use a config file if present.
cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Jan 20, 2022
This is related to ostreedev#121
as well as containers/containers-image-proxy-rs#8
etc.

The CLI code here supports `--authfile`.  However, passing it on
the CLI each time for production use cases pushes complexity to
users.

Add support for global persistent and runtime config files in
`/etc/ostree/auth.json` and `/run/ostree/auth.json`.

Change the default constructor for image pulls to use that by default.

Note that the CLI options override the config defaults.

While we're here, also add `--auth-anonymous` to the CLI, which is
needed to ensure we don't use a config file if present.
@cgwalters
Copy link
Member Author

Calling this one fixed! We had the relevant test pass in openshift/machine-config-operator#2886

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

No branches or pull requests

2 participants