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

Configure image registry credentials in containerd #1227

Closed
Niksko opened this issue Nov 26, 2020 · 12 comments · Fixed by #1955
Closed

Configure image registry credentials in containerd #1227

Niksko opened this issue Nov 26, 2020 · 12 comments · Fixed by #1955
Assignees
Labels
type/enhancement New feature or request

Comments

@Niksko
Copy link

Niksko commented Nov 26, 2020

What I'd like:

A way to configure image registry credentials in containerd, for pulling images from private registries (or in the case of Dockerhub, with authentication to avoid rate limiting).

This is a common alternative to defining image pull secrets on a per pod basis, and is recommended by the Kubernetes docs if you want particular credentials to apply to all pods on a node.

Having this exposed as a setting would be ideal. Containerd seems to support configuration of registry authentication via toml file as per this pr. As a first pass, could we expose a setting that passes through to this section of the containerd configuration?

Any alternatives you've considered:

  1. Image pull secrets per pod. Inherently difficult and fiddly without automation on the side of creating manifests.
  2. A daemonset that mounts host paths to create the required configuration file. Not sure if this will work, it seems as if we might need to restart containerd.
  3. Adding configuration via the admin container. Seems like it should be possible, but would be difficult to orchestrate.
  4. Pre-pulling images. If the image is already present on the node and the ImagePullPolicy is set correctly, the node will just use the pre-pulled image. We could likely orchestrate pulling all essential images shortly after startup, but again, it would be difficult to do.
@jahkeup
Copy link
Member

jahkeup commented Dec 1, 2020

Hi @Niksko, thanks for the feature request and outlining your alternative approaches in contrast. We'll look into how to model and support registry credentials through our settings API.

@jahkeup jahkeup added the type/enhancement New feature or request label Dec 1, 2020
@Niksko
Copy link
Author

Niksko commented Dec 7, 2020

I'm happy to submit a PR for this, it doesn't seem like a huge change. A few questions:

  1. Is there a way to easily test my changes to the settings model locally, without baking an AMI?
  2. Should this be a generic setting, or Kubernetes specific? Perhaps just a generic setting - I'm not really sure if there's as strong a use case for ECS, but it certainly wouldn't hurt to have it as an option.
  3. Can the user facing setting be as simple as:
[settings.containerd.registry-authentication]
url = "https://my-registry.org"
username = "niksko"
password = "supersecret"
auth = "somebase64"
identitytoken = "sometoken"

[settings.containerd.registry-authentication]
url = "https://dockerhub.io"
username = "other-user"
password = "supersecret2"
auth = "somemorebase64"
identitytoken = "someothertoken"

A flat array of objects is easier for mustache to iterate over. Using the url as the key and the auth creds as the value is more difficult. Or is this a non-starter because it would mean including secrets in userdata?

@bcressey
Copy link
Contributor

bcressey commented Dec 8, 2020

I like your approach, but you nailed the big concern: I'm not comfortable with handling secrets with the mechanisms we have today. Userdata is not encrypted; settings are in plaintext when they're stored on disk, returned by the API, and rendered into templates.

I expect we'd want containerd to support per-registry credential helpers or plugins first, as discussed in containerd/containerd#6637 - then it would just be a matter of shipping those helpers in the base image, or offering a way to load them at runtime.

@gregdek gregdek added this to the backlog milestone Apr 1, 2021
@gabegorelick
Copy link

A daemonset that mounts host paths to create the required configuration file. Not sure if this will work, it seems as if we might need to restart containerd.

Would bootstrap containers work?

@Niksko
Copy link
Author

Niksko commented May 13, 2021

@gabegorelick yes, they probably would, good suggestion!

@bcressey
Copy link
Contributor

bcressey commented May 18, 2021

Bootstrap containers would be blocked by the SELinux policy from modifying configuration files directly, but they would help work around the security problems of passing secrets in user data.

For EC2, the instance could have role credentials that allowed pulling a bootstrap container and reading secrets from SSM or Secrets Manager, then apply them to the API using apiclient.

Settings are no longer readable by unprivileged containers after ea35f1b, so the remaining hurdle is to secure the configuration files after they're generated from templates.

That could be done by either:

  • restricting reads to all of /etc for unprivileged containers
  • writing the configuration somewhere else so we can apply a different label

@gabegorelick
Copy link

For EC2, the instance could have role credentials that allowed pulling a bootstrap container

Yeah I kind of assumed that when mentioning bootstrap containers. But this leads to another issue: how would it work if you wanted to pull these containers from a registry that can't be accessed via instance role? E.g. if your bootstrap container is in Docker Hub you can't expect it to set your Docker Hub credentials.

@stevehipwell
Copy link

This is a blocker to the adoption of Bottlerocket as this can currently be easily done via userdata. An alternative view on this is that the credentials being provided here should only be R/O so the MVP would be to be able to specify these as plaintext with warnings and then to iterate on a "better" solution.

@Niksko
Copy link
Author

Niksko commented Jul 22, 2021

Seems like the alpha Kubelet credential provider plugins feature may help us solve this.

The kubelet arguments and configuration seem like they would be straightforward to implement, but how would we facilitate providing the binaries? Perhaps via a bootstrap container?

@zmrow
Copy link
Contributor

zmrow commented Jul 22, 2021

@Niksko I hadn't seen that feature - it looks pretty neat and seems like it could help, at least at first glance.

You hit the nail on the head though, making sure the correct credential provider binaries are provided is indeed part of the problem with it.

We'll look into this feature more as it matures - hopefully it can help!

@stevehipwell
Copy link

Has there been any progress on this?

With an AL2 node we write the credentials out to /var/lib/kubelet/config.json from the userdata, would this pattern still work correctly with a bootstrap container?

Could the API not be updated to support this as @Niksko suggested which would allow the end user to decide if they want to add credentials to userdata or to use a bootstrap container to fetch them dynamically and set them with the API?

@samjo-nyang
Copy link
Contributor

For your interest, I implemented this feature through the kubelet credential provider. I developed a token server and a simple kubelet credential provider, and inject the provider using the bootstrap container. Here is a company tech blog article - https://hyperconnect.github.io/2022/02/21/no-more-image-pull-secrets.html (unfortunately, it is Korean, so you may need a translator...)

@kdaula kdaula modified the milestones: 1.7.0, 1.8.0 Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.