shipwright-io / build

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

As a shipwright developer, I want to stop relying on service accounts for mounting my secrets at the pod level #679

Open qu1queee opened 3 years ago

qu1queee commented 3 years ago

Idea:

This is coming as an outcome of https://github.com/shipwright-io/build/issues/662. We want this issue as a way to understand the implication of stopping using serviceAccount and referencing that in Tekton TaskRun, in favor of dealing with our secrets directly, without the need of a service account, as seen in:

qu1queee commented 3 years ago

Adding more info on this issue.

Today we use the creds-init feature in Tekton, to delegate to Tekton the responsability of generating a ~/.docker/config.json for docker registry authentication. This is nice while it allow users to reference a secret with basic auth, and from there Tekton will generate the required file and place it in at the pod level.

Its my understanding that the Tekton community will make the creds-init feature disable by default. While it will keep a flag to enable it, long term and the recommendation is to stop using it.

The feature can be removed, by setting the disable-creds-init key to true under the feature-flags confimap in the tekton deployment namespace. When this is removed, the strategies using kaniko or buildpacks will not longer work.

An option to fulfill what we lost when disabling the feature, is to use workspaces to mount the required secret, from where the ~/.docker/config.json will be created. An example is available in this commit.

Things to keep in mind from the above example:

imjasonh commented 3 years ago

The issue that @SaschaSchwarze0 mentioned in the grooming meeting today seems like a documentation issue.

https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/ mentions a couple ways to generate a Docker config Secret:

  1. docker-registry and --docker-username, etc:
kubectl create secret docker-registry regcred \
    --docker-server=server.io \
    --docker-username=my-user \
    --docker-password=my-password \
    --docker-email=my@email.com

...generates a Secret like:

data:
  .dockerconfigjson: ...
  1. generic and --from-file with --type=dockerconfigjson
kubectl create secret generic regcred2 \
    --from-file=.dockerconfigjson=$HOME/.docker/config.json \
    --type=kubernetes.io/dockerconfigjson

...generates a Secret like:

data:
  .dockerconfigjson: ...

Unfortunately these don't make it very easy to consume for our use case, since we ultimately want to be able to mount this as a file at ~/.docker/config.json, and if we mount it at ~/.docker/, where the file will be named ~/.docker/.dockerconfigjson 😢

Aside: We can actually mount the file in any directory, e.g., /shipwright/dockerconfig/ as long as we set the DOCKER_CONFIG=/shipwright/dockerconfig/ env var, as described here. Most/all Docker-adjacent tooling should be able to handle this env var (I know kaniko, ko, buildpacks, etc., respect this; I don't know about s2i, etc.)

Anyway, the problem is the name of the file is .dockerconfigjson, but that's only the case because that's what docker secret create docker-registry and --type=kubernetes.io/dockerconfigjson expect to do.

This variation on (2) above:

kubectl create secret generic regcred3 \
  --from-file=config.json=$HOME/.docker/config.json

...will generate a secret like:

data:
  config.json: ...

...which we can more easily mount and use in our Pods.

qu1queee commented 3 years ago

@imjasonh the variation on (2) will work, but we still have (1) not cover, right?

As (1) is heavily used when generating the registry secrets, I will say that we will eventually need glue-code in the controller, to:

a) Generate a volume and volumemount with the desire config.json file, by reading the secret provided in spec.output.credentials.name and making this available to the build-and-push step.

b) Ensure the config file under the above volumemount is of the form config.json, instead of .dockerconfigjson , in order for the tooling to properly consume it.

imjasonh commented 3 years ago

Some glue code to help out users who did (1) or the original (2) could be useful, with a warning to users that they should migrate to the variation of (2).

As for how that glue could work, we could simply prepend a step in the TaskRun that looks for ~/.config/.dockerconfigjson and renames it to config.json (failing if that file already exists?) before running the rest of the steps. That solution might be minimal and reliable enough we don't even need to consider removing it, though we should probably log a warning somewhere if we do it, since people might be confused about the expected filename.

SaschaSchwarze0 commented 3 years ago

I stumbled over a Kubernetes features that resolves all our problems:

spec:
  volumes:
    - name: container-registry-secret
      secret:
        secretName: user-defined-secret-of-type-dockerconfigjson
        items:
          - key: .dockerconfigjson
            path: config.json

This allows us to specify a different file name. It actually is a relative path, relative to the path defined on the volumeMount in the container.

adambkaplan commented 3 years ago

Note that Tekton's feature that merges registry credentials is mainly used for the spec.builder image in our API. We have an issue to deprecate this field - #935. As a potential alternative, we would provide mechanisms/patterns for pull secrets to be provided directly to the build strategy.