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

Allow image-builder to consume and publish to different image registries with different docker credentials #12060

Open
Pothulapati opened this issue Aug 11, 2022 · 3 comments
Labels
meta: never-stale This issue can never become stale team: workspace Issue belongs to the Workspace team type: feature request New feature or request

Comments

@Pothulapati
Copy link
Contributor

Pothulapati commented Aug 11, 2022

Problem

The image builder can only reference one secret object, and though that object can list 20 repositories in it and their corresponding credentials, we have no facility to manage or merge / combine multiple user provided registry credentials into a secret object.

This means that the image-builder cannot pull an image from one registry that requires credentials and then push to another different registry that requires credentials (It can if one of these is public). This is an issue for those cases where a customer is trying to get the workspace base image from one registry and them push the built image for a workspace (that adds customisations on top of the base image) to another image registry ready to be used to start workspaces. In practice, this means you cannot use an in-cluster registry to store workspace images and a private (i.e. one that needs credentials) registry to pull base images from. This is an issue especially in airgapped environments that use the in-cluster registry (we recommend AWS users to use the in-cluster registry backed by s3 for storage), which usually always use a private registry for base images.

❓ Why do we recommend using the in-cluster repository and not the other existing one? We don’t support all registries for pushing to them (we really only test against GCR and Docker Registry, i’ve not got workspace-image push working with artifactory or harbor), they don’t want workspace-images (multi gigabyte images) stored in their registry space optimized for serving small containers very quickly, or where they have to pay per image push for scanning / auditing, etc
❓ Why don't we support all registries? It wouldn't also solve the issue, but to your question for example, we lack ECR support because:
  1. we need to do an AWS api call to create workspace-images manually before we can push an image to it
  2. ECR credentials need to be refreshed every 12 hours, there is a beta kubelet feature to make this easier to do, but that won’t directly help registry facade. @mrzarquon did POC using a batch job to refresh the credential in a dockerconfigjson object

Technical details — Self-hosted team

With #10792, We started investing on ways to allow docker credentials to be passed by the user even when incluster registry is enabled, to allow for workspace base images to be pulled from different (& usually private) registries.

As the above issue describes, The fix initially seemed simple i.e to move credentials out of external so that a registry credentials secret can be passed even when an external registry is not being used.

Roadblock

Internally, The current way this works is in a either or approach i.e pass the secret if external registry is used, or pass builtin-registry-credentials if inclusterRegistry is enabled.

Now, We need a way to pass multiple secrets and thats where it gets tricky, as the current components (that need the registry communication) configs seem to take only a single dockerConfig file.

The installer's conventions doesn't let us merge these things for the following reasons:

  • We can't take credentials directly in the config, as we have been using secrets as a way of any kind of credentials store (storage, certificates, etc)
  • We can't also take a list of secrets, and merge them as the installer by design cannot read something from the cluster.

Potential solution

Components should instead have the ability to use multiple files, merging them before using them as a registry. This feels right as it confirms to the installer conventions, where the instalelr would just mount these secrets correctly.

Describe alternatives you've considered

As discussed above, the alternatives would break conventions but were considered.

Additional context

@corneliusludmann
Copy link
Contributor

Please correct me when I'm wrong, @Pothulapati. That is what I understood:

It seems the following components using the registry auth credentials:

  • blobserve
    if pointer.BoolDeref(ctx.Config.ContainerRegistry.InCluster, false) {
    secretName = dockerregistry.BuiltInRegistryAuth
    } else if ctx.Config.ContainerRegistry.External != nil {
    secretName = ctx.Config.ContainerRegistry.External.Certificate.Name
    } else {
    return nil, fmt.Errorf("%s: invalid container registry config", Component)
    }
  • image-builder-mk3
    if pointer.BoolDeref(ctx.Config.ContainerRegistry.InCluster, false) {
    secretName = dockerregistry.BuiltInRegistryAuth
    } else if ctx.Config.ContainerRegistry.External != nil {
    secretName = ctx.Config.ContainerRegistry.External.Certificate.Name
    } else {
    return "", fmt.Errorf("%s: invalid container registry config", Component)
    }
  • registry-facade
    if pointer.BoolDeref(ctx.Config.ContainerRegistry.InCluster, false) {
    secretName = dockerregistry.BuiltInRegistryAuth
    } else if ctx.Config.ContainerRegistry.External != nil {
    secretName = ctx.Config.ContainerRegistry.External.Certificate.Name
    } else {
    return nil, fmt.Errorf("%s: invalid container registry config", Component)
    }

The problem is that the components allow using the built-in registry auth OR the external registry auth but not both. But customers that want to pull workspace images from a registry that needs auths (instead of Docker Hub) need to add an additional registry secret. That happens regularly when using air-gap because the workspace images will be mirrored in a local registry that is accessible in the air-gapped network.

We cannot merge the built-in registry auth and the user-given registry auth in the installer because the installer does not have access to the user-given registry auth (which is a K8s secret) during the rendering phase (render does not have access to the cluster).

Question (for Team Workspace?): Which of the components listed above need to pull the workspace images? Only image builder? Components that need to pull workspace images need to accept two secrets (for the built-in registry as well as the user-given secret).

@corneliusludmann corneliusludmann added priority: highest (user impact) Directly user impacting type: feature request New feature or request labels Aug 11, 2022
@atduarte atduarte removed the priority: highest (user impact) Directly user impacting label Aug 17, 2022
@atduarte atduarte changed the title Allow multiple docker credentials to be passed Allow image-builder to consume and publish to different image registries Aug 19, 2022
@atduarte atduarte changed the title Allow image-builder to consume and publish to different image registries Allow image-builder to consume and publish to different image registries with different docker credentials Aug 19, 2022
@stale
Copy link

stale bot commented Nov 23, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Nov 23, 2022
@stale stale bot closed this as completed Dec 3, 2022
@stale stale bot moved this to Awaiting Deployment in 🌌 Workspace Team Dec 3, 2022
@kylos101 kylos101 reopened this Dec 6, 2022
@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Dec 6, 2022
@kylos101 kylos101 added meta: stale This issue/PR is stale and will be closed soon meta: never-stale This issue can never become stale labels Dec 6, 2022
@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Dec 6, 2022
@ChevronTango
Copy link

ChevronTango commented Aug 18, 2023

I want to bump this because it's still an issue. Right now you can't use ECR as a backing store for Gitpod, despite the new ECR flag. The reason for this is, whilst the imagebuilder-mk3 is able to auth and check the image, it does not pass the ECR auth through to the image-builder-bob. It only passes through the docker config. This means that the bob can authenticate and pull images from a private-registry or ECR, but cannot push to ECR.

{
Name: "WORKSPACEKIT_BOBPROXY_AUTH",
Secret: &wsmanapi.EnvironmentVariable_SecretKeyRef{
SecretName: o.Config.PullSecret,
Key: ".dockerconfigjson",
},
},

It would be a quick win to switch this out for an if statement that asks "is the workspace registry ECR?" if yes pass through the ECR auth, and if no pass through the dockerconfig like normal.

The inclusion of this would allow deployments to use ECR natively rather than relying on a cluster local docker-registry or some other managed docker registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: never-stale This issue can never become stale team: workspace Issue belongs to the Workspace team type: feature request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants