Add Image Output Resources

[bobcatfish]

Expected Behavior

If a Task declares that it produces an image output:

1. The TaskRun controller should verify that the promised image was actually created and pushed to where it was supposed to be pushed to. If the Task does not push the image, the TaskRun should fail.
2. The Image resource should be updated with the digest of the image created

This should leverage the design already fleshed out in knative build artifact design and the work being added to go-containerregistry in google/go-containerregistry#209 and google/go-containerregistry#261 (+ @jonjohnsonjr )

Actual Behavior

• Currently there is no handling of outputs (only used as part of templating)
• After Design Output handling #124 there should be hooks added to validate resources generically, and "file" resources will be validated, but no other types

Steps to Reproduce the Problem

Success case

1. Create a Pipeline that has 2 Tasks, one that builds an image and another that does something with that image using the digest of the image just built, e.g. (based on this example).

Tasks:

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: build-push
spec:
  inputs:
    resources:
    - name: workspace
      type: git
    params:
    - name: pathToDockerFile
      description: The path to the dockerfile to build
      default: /workspace/workspace/Dockerfile
    - name: pathToContext
      description: The build context used by Kaniko (https://github.com/GoogleContainerTools/kaniko#kaniko-build-contexts)
      default: /workspace/workspace
  outputs:
    resources:
    - name: builtImage
      type: image
  steps:
  - name: build-and-push
    image: gcr.io/kaniko-project/executor
    command:
    - /kaniko/executor
    args:
    - --dockerfile=${inputs.params.pathToDockerFile}
    - --destination=${outputs.resources.builtImage.url}
    - --context=${inputs.params.pathToContext}

---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: demo-deploy-kubectl
spec:
  inputs:
    resources:
    - name: workspace
      type: git
    - name: image
      type: image
    params:
    - name: path
      description: Path to the manifest to apply
    - name: yqArg
      description: Okay this is a hack, but I didn't feel right hard-codeing `-d1` down below
    - name: yamlPathToImage
      description: The path to the image to replace in the yaml manifest (arg to yq)
    clusters:
    - name: targetCluster
      description: Not yet used, kubectl command below should use this cluster
  steps:
  - name: replace-image
    image: mikefarah/yq
    command: ['yq']
    args:
    - "w"
    - "-i"
    - "${inputs.params.yqArg}"
    - "${inputs.params.path}"
    - "${inputs.params.yamlPathToImage}"
    - "${inputs.resources.image.url}@${inputs.resources.image.digest}" # This is the change relevant to this issue
  - name: run-kubectl
    image: lachlanevenson/k8s-kubectl
    command: ['kubectl']
    args:
    - 'apply'
    - '-f'
    - '${inputs.params.path}'

Pipeline:

apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: demo-pipeline
spec:
  resources:
  - name: source-repo
    type: git
  - name: web-image
    type: image
  tasks:
  - name: build-skaffold-web
    taskRef:
      name: build-push
    params:
    - name: pathToDockerFile
      value: Dockerfile
    - name: pathToContext
      value: /workspace/workspace/examples/microservices/leeroy-web
    resources:
      inputs:
      - name: workspace
        resource: source-repo
      outputs:
      - name: builtImage
        resource: web-image
  - name: deploy-web
    taskRef:
      name: demo-deploy-kubectl
    resources:
      inputs:
      - name: workspace
        resource: source-repo
      - name: image
        resource: web-image
        from:
        - build-skaffold-web
    params:
    - name: path
      value: /workspace/workspace/examples/microservices/leeroy-web/kubernetes/deployment.yaml
    - name: yqArg
      value: "-d1"
    - name: yamlPathToImage
      value: "spec.template.spec.containers[0].image"

2. Create a PipelineRun and PipelineResources to run it:

apiVersion: tekton.dev/v1alpha1
kind: PipelineResource
metadata:
  name: skaffold-image-leeroy-web
spec:
  type: image
  params:
  - name: url
    value: gcr.io/christiewilson-catfactory/leeroy-web
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineResource
metadata:
  name: skaffold-git
spec:
  type: git
  params:
  - name: revision
    value: master
  - name: url
    value: https://github.com/GoogleContainerTools/skaffold
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: demo-pipeline-run-1
spec:
  pipelineRef:
    name: demo-pipeline
  trigger:
    type: manual
  resources:
  - name: source-repo
    resourceRef:
      name: skaffold-git
  - name: web-image
    resourceRef:
      name: skaffold-image-leeroy-web

3. You won't be able to use the digest - the field will be blank! So this will fail b/c it will try to deploy an image called "${inputs.resources.image.url}@${inputs.resources.image.digest}" but the digest will be empty so it will be gcr.io/christiewilson-catfactory/leeroy-web@ (instead of gcr.io/christiewilson-catfactory/leeroy-web@123456somedigest789)

Error case

1. Create a Task that declares an image as an output, but don't actually create the image (e.g. it just runs echo), e.g. (based on a working example):

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: build-push-kaniko
spec:
  inputs:
    resources:
    - name: workspace
      type: git
  outputs:
    resources:
    - name: builtImage
      type: image
  steps:
  - name: build-and-push
    image: ubuntu
    command:
    - echo
    args:
    - "I said I would build an image but I did noooothing"

2. Create a TaskRun that uses that task
3. Notice there is no error!

This means if you were relying on your pipeline to produce the image, but you made a mistake and the image was never published, you may never notice!

Additional Info

• knative build artifact design
• This issue involves getting information from an invocation of a Task (on disk) back to the controller (and into a Run status) - this is the same functionality needed for Dynamic Input Parameters #552 so whatever design and implementation we have for that component should work for both

[solsson]

Maybe this belongs in #124, but it's specific to images. I'd like to offer two design ideas based on Source-to-URL use cases:

• It's unclear to me if ImageResource's URL property should include tag or not. I assume it can, but my suggestion is to add an optional Tag property and validate that URL does not contain a :. Otherwise the ambiguity will probably lead to variations among Tasks, with some having tag as param instead.

  • Also the Tag property would be useful until we have tag-to-digest resolution, allowing Resources to set git revision and pipeline tag to the same git ref. That would let us deploy withou the caveat.
  • Even with tag-to-digest such a trick can be used to support concurrent builds.

• For Tasks that consume the built image resource it can be useful to know the previous digest, maybe through an optional property PreviousDigest. The use case is to see, in multi-image builds like the current example, if an image has changed or not. If it hasn't maybe some resource intensive tasks like integration tests can be skipped, as can some redeployments. With build caching enabled and builds triggered on every push such checks could save lots of resources. I got the impression that for example Buildpack looks up the previous image for the URL+tag already.

[afrittoli]

If no-one else is looking at this I can give it a go and propose a design. Is there a special template to be used? Or any google doc should be fine?

[vdemeester]

@afrittoli I think any google doc should be fine at start, but we may want to have a special template in the near future (maybe we do cc @bobcatfish @imjasonh ?)

[afrittoli]

@shashwathi @bobcatfish In the description of this issue it talks about generic verification hooks to be introduced by #124; even though #124 is closed it doesn't seem to me that they have been implemented yet. Is that right? Is there a design for those yet or anyone working on them, or shall I include that in the design for this #216?

[vbatts]

Maybe this belongs in #124, but it's specific to images. I'd like to offer two design ideas based on Source-to-URL use cases:

* It's unclear to me if ImageResource's `URL` property should include tag or not. I assume it can, but my suggestion is to add an optional `Tag` property and validate that URL does not contain a `:`. Otherwise the ambiguity will probably lead to variations among Tasks, with some having tag as param instead.
  
  * Also the Tag property would be useful until we have tag-to-digest resolution, allowing Resources to set `git` `revision` and `pipeline` `tag` to the same git ref. That would let us deploy withou the [caveat](https://github.com/knative/build-pipeline/commit/54051bf06f22f15260b9d4dd87f6b6c82e7c03fe#diff-1c0f522a61adc59307209c8e0296db49R32).
  * Even with tag-to-digest such a trick can be used to support concurrent builds.

* For Tasks that consume the built image resource it can be useful to know the previous digest, maybe through an optional property `PreviousDigest`. The use case is to see, in multi-image builds like the current example, if an image has changed or not. If it hasn't maybe some resource intensive tasks like integration tests can be skipped, as can some redeployments. With build caching enabled and builds triggered on every push such checks could save lots of resources. I got the impression that for example Buildpack looks up the previous image for the URL+tag already.

Cool cool. As for "tags" (see also "labels" or "annotations" (I know annotations in k8s are even more heavily utilitized)), hopefully we can make reuse of keys like https://github.com/opencontainers/image-spec/blob/master/annotations.md#pre-defined-annotation-keys and the label-schema.org effort.
So as we could have build artifacts like an OCI image-layout these "tags" could be more shared and widely interpreted.

[bobcatfish]

Added examples to the description!

[nader-ziada]

/assign

[afrittoli]

Is there something still missing here from the current implementation?

[nlewo]

@afrittoli if I understand well, the digest is exposed in the taskrun but the image resource is not yet updated. So, this means we cannot use the digest as an image resource variable (since it is the empty string).

Are you still planning to update the image resource with the digest?

[vdemeester]

@afrittoli if I understand well, the digest is exposed in the taskrun but the image resource is not yet updated. So, this means we cannot use the digest as an image resource variable (since it is the empty string).

The digest should be available for the next Task/TaskRun that use this image as input resource (and use from, although I don't think it's required ?). If it doesn't work, then it's a bug 😅

[slowr]

@afrittoli if I understand well, the digest is exposed in the taskrun but the image resource is not yet updated. So, this means we cannot use the digest as an image resource variable (since it is the empty string).

The digest should be available for the next Task/TaskRun that use this image as input resource (and use from, although I don't think it's required ?). If it doesn't work, then it's a bug 😅

Is it supposed to pass the digest correctly? I have a pipeline with two tasks (one build and one deploy) and it seems that the digest is an empty string.

  Warning  InspectFailed  <invalid> (x5 over 31s)  kubelet, xxx  Failed to apply default image tag "xxx/dev/test@": couldn't parse image reference "xxx/dev/test@": invalid reference format
  Warning  Failed         <invalid> (x5 over 31s)  kubelet, xxx  Error: InvalidImageName

The image-digest-exporter successfully exports the digest.

[webapp-build : image-digest-exporter-blccm] 2019/10/12 22:38:13 Image digest exporter output: [{"name":"webapp-image","digest":"sha256:55f7dbd28eaaca9297527687ca27823664e38b2d0383135c20eb43c803c5087d"}]

[nlewo]

@slowr I don't think it's working atm. I had to deploy a intermediate task to do this. See here https://github.com/nlewo/poc-tekton#deploy-tasks for details and example.

[bobcatfish]

Closing this in favor of #1673 since we're redesigning all the PipelineResources (including this one) as part of that.