shipwright-io / build

Shipwright - a framework for building container images on Kubernetes
https://shipwright.io
Apache License 2.0
657 stars 112 forks source link

Build is missing support to specify a secret for FROM images in Dockerfiles #531

Open SaschaSchwarze0 opened 3 years ago

SaschaSchwarze0 commented 3 years ago

In a Build, we currently have "3.5" places to define a reference to a Kubernetes Secret:

  1. spec.source.credentials.name for the Git repository
  2. spec.builder.credentials.name for the pull operation of the builder image (atm only used by the s2i build strategy)
  3. spec.output.credentials.name for the push operation of the output image
  4. spec.runtime.base.credentials.name can be specified but is not used anywhere in our code

In general, although, we have a purpose for each of the Secrets, we "only" place them in the ServiceAccount associated with the TaskRun. Tekton will then take care of them to make them available to all its containers. This means that theoretically, a Secret specified under the output, can be used by any container registry related operation in any build step.

This includes the pulling of FROM image(s) in a Dockerfile-based build. I only checked Kaniko, but expect buildah to also use a Secret to pull the FROM image(s). Using a Secret to pull the FROM is required or useful for these two scenarios:

  1. The FROM image comes from a private registry. In this case, authentication is required.
  2. The FROM image comes from a public repository at DockerHub. The rate limits there limits anonymous access. To get a guarantee that the image pull works requires to use authentication. Otherwise too many BuildRuns by potentially multiple users can cause 429 responses from DockerHub.

As of today, we only support authentication for the image pull of the FROM image(s) if it comes from the same container registry that is also used for the output or builder image. This blocks those scenarios where this is not the case.

We should provide means to specify other Secrets somewhere in the Build to support authentication for FROM image(s).

Side note: there is a bug/limitation currently in Tekton so that only one Secret of type dockerconfigjson is supported. PR Support multiple secrets of type dockercfg and dockerconfigjson #3659 is supposed to lift this.

zhangtbj commented 3 years ago

Thanks for raise this issue up.

A quick question, can we share the same account or use one secret for both spec.builder.credentials.name and FROM ?

SaschaSchwarze0 commented 3 years ago

A quick question, can we share the same account or use one secret for both spec.builder.credentials.name and FROM ?

We at the moment allow the user to define the Secret in the Build to be for a specific purpose. Either as source secret or for the builder image or for the output image. But, implementation-wise, we do nothing to ensure that the secret is only used for the desired purpose. Instead, we simply assign all secrets to the TaskRuns ServiceAccount which causes Tekton to make it globally available. This is fine and nothing that I suggest to be changed.

But, it effectively means that account sharing and Secret reusing is not only possible, it is a MUST. You can define two Secrets for the same container registry using different credentials. You can also assign them at different places in a Build. But after it has been assigned to the ServiceAccount and that one to the TaskRun, Tekton in its creds.Write() code will merge all the credentials into a single config.json file which can only contain one credential for one container registry. Means, only one of the Secrets will win. That's something we have to document somewhere.

gabemontero commented 3 years ago

Yeah I agree this seems like a good catch.

FWIW, there is some openshift build v1 perspective on this

So yeah, in theory,

SaschaSchwarze0 commented 3 years ago

Good feedback @gabemontero.

One thing I consider difficult from a user's perspective is having one Kubernetes Secret with multiple credentials. Although this is possible with Secrets of type dockerconfigjson, I see there a usability issue:

The easiest way to create such a Secret with kubectl imo is kubectl create secret docker-registry. This does not allow having multiple credentials in a single Secret, for this a user imo needs to manually build the config.json file manually and then use kubectl create secret generic <NAME> --type=kubernetes.io/dockerconfigjson --from-file=config.json - which is complicated. Though, a CLI can simplify it.

Downstream, IBM Cloud Code Engine even goes one step further and somehow expects secrets to contain a single credential (because we list them an UI and CLI and also show the registry host and user name there).

gabemontero commented 3 years ago

Good feedback @gabemontero.

One thing I consider difficult from a user's perspective is having one Kubernetes Secret with multiple credentials. Although this is possible with Secrets of type dockerconfigjson, I see there a usability issue:

I wonder how much docker related background comes into play here. Those who were deep into using docker before coming to k8s are probably not bothered by this at all. I see that for example with the devops crew who managed say openshift online or our internal CI system, which is comprised of many k8s/openshift clusters. They are definitely using Secrets which contain credentials to multiple registries via import docker config files.

The easiest way to create such a Secret with kubectl imo is kubectl create secret docker-registry. This does not allow having multiple credentials in a single Secret, for this a user imo needs to manually build the config.json file manually and then use kubectl create secret generic <NAME> --type=kubernetes.io/dockerconfigjson --from-file=config.json - which is complicated. Though, a CLI can simplify it.

Yep, in that doc link https://docs.openshift.com/container-platform/4.6/builds/creating-build-inputs.html#builds-docker-credentials-private-registries_creating-build-inputs I posted earlier, you'll see that exact same ... create secret generic... invocation (with the only change replacing kubectl with oc) as well as some of the CLI simplificationsoc` provides in associating secrets with openshift v1 builds. I would envision similar "helpers" in shipwright's upcoming CI.

Downstream, IBM Cloud Code Engine even goes one step further and somehow expects secrets to contain a single credential (because we list them an UI and CLI and also show the registry host and user name there).

So if I were to recap: a) to reiterate I would agree the current shipwright API needs some curation around the original issue you've opened here @SaschaSchwarze0 .... ideas like the simple one in my last comment or others are what we have to settle on b) the API ideally optimally facilitates both the single cred per secret and multilpe cred per secret scenarios. Both are valid, especially in the context of how the cluster(s) are managed, and we have examples of usage for each with existing and potential consumers of shipwright