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

API: References and Specifications #5138

Open
jerop opened this issue Jul 14, 2022 · 6 comments
Open

API: References and Specifications #5138

jerop opened this issue Jul 14, 2022 · 6 comments
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@jerop
Copy link
Member

jerop commented Jul 14, 2022

Tekton Pipelines has the following fields:

  • TaskRef - references to Tasks and Custom Tasks
  • TaskSpec - embedded specifications of Tasks and CustomTasks
  • PipelineRef - references to Pipelines
  • PipelineSpec - embedded specifications of Pipelines

We have discussed replacing them with ref and spec, especially when we discussed supporting Pipelines in Pipelines. We descoped this work from the TEPs where it came up, so opening this issue to track this discussion and gather more feedback.

Below is the most promising API for using ref and spec - thanks to @afrittoli!

This example below demonstrates the API change to Pipelines, but the same can be applied to other types.

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: clone-scan-notify
spec:
  tasks:

    # task using reference
    - name: pt-0
      ref:
         apiVersion: tekton.dev/v1beta1
         kind: Task
         name: git-clone

    # task using specification
    - name: pt-1
      spec:
         apiVersion: tekton.dev/v1beta1
         kind: Task
         spec:
           params: (...)
           steps:
             - (...)

    # pipeline using reference
    - name: pt-2
      ref:
         apiVersion: tekton.dev/v1beta1
         kind: Pipeline
         name: golang-pipeline

    # pipeline using specification
    - name: pt-3
      spec:
         apiVersion: tekton.dev/v1beta1
         kind: Pipeline
         spec:
           params: (...)
           tasks:
             - (...)

    # custom task using reference
    - name: pt-4
      ref:
         apiVersion: example.dev/v1
         kind: Example
         name: example-pipeline

    # custom task using specification
    - name: pt-5
      spec:
         apiVersion: example.dev/v1
         kind: Example
         spec:
           field1: (...)
           field2: (...)
             
    # anything using remote resolution (kind is specified in resource field)
    - name: pt-6
      ref:
        resolver: git
        resource: (...)

Notes

  • We could default apiVersion such that it matches the top level value - this way apiVersion is required for Custom Tasks only
  • ChildReferences already takes this shape - where it lists the references with apiVersion and kind - so this change will align the specifications with the status
  • We can support both the existing API and this API while we migrate to this new version, but we should go into V1 with this API if we decide this is the direction we want to go

/kind feature
/area api

@tekton-robot tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. area/api Indicates an issue or PR that deals with the API. labels Jul 14, 2022
@jerop
Copy link
Member Author

jerop commented Jul 14, 2022

@tektoncd/core-maintainers please take a look

@abayer
Copy link
Contributor

abayer commented Jul 14, 2022

My biggest concern with this is that tasks[0].spec will have to be a runtime.RawExtension, since it could be a spec for a Task, a custom task, or (eventually) a Pipeline. We're stuck with runtime.RawExtension for custom tasks, but I'm not a fan of having to do that same kind of conversion for every PipelineTask.

I definitely lean more towards having distinct .taskSpec, .customTaskSpec (instead of overloading .taskSpec.spec as we do now for custom tasks), and (eventually) .pipelineSpec fields on PipelineTask. That said, switching to .ref from .taskRef etc makes sense to me, with defaulting as mentioned for apiVersion, and I'd say defaulting kind to Task in PipelineTask.Ref as well.

@pritidesai
Copy link
Member

pritidesai commented Jul 15, 2022

My biggest concern with this is that tasks[0].spec will have to be a runtime.RawExtension, since it could be a spec for a Task, a custom task, or (eventually) a Pipeline. We're stuck with runtime.RawExtension for custom tasks, but I'm not a fan of having to do that same kind of conversion for every PipelineTask.

💯

It's an implementation detail but I prefer avoiding runtime.RawExtension for other types of resources if possible.

I definitely lean more towards having distinct .taskSpec, .customTaskSpec (instead of overloading .taskSpec.spec as we do now for custom tasks), and (eventually) .pipelineSpec fields on PipelineTask. That said, switching to .ref from .taskRef etc makes sense to me, with defaulting as mentioned for apiVersion, and I'd say defaulting kind to Task in PipelineTask.Ref as well.

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: clone-scan-notify
spec:
  tasks:
    # task using reference
    - name: pt-0
      ref:
         apiVersion: tekton.dev/v1beta1
         kind: Task
         name: git-clone

I like using kind this way but can we avoid introducing apiVersion here for task and pipeline? The problem with apiVersion here is it could mean that pipeline supports mixed versions, pipeline could be v1 and task could be v1beta1. We have not supported this kind of combination AFAIR. If we avoid apiVersion, it leads us to just two fields:

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: clone-scan-notify
spec:
  tasks:
    # task using reference
    - name: pt-0
      ref:
         kind: Task
         name: git-clone

isn't ref is too short here? 😞

And, for the spec, we are again going back to spec.tasks[0].spec.spec 🙃

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: clone-scan-notify
spec:
  tasks:

    # task using specification
    - name: pt-1
      spec:
         apiVersion: tekton.dev/v1beta1
         kind: Task
         spec:
           params: (...)
           steps:
             - (...)

instead, its cleaner to be more descriptive.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 13, 2022
@jerop
Copy link
Member Author

jerop commented Oct 25, 2022

/lifecycle frozen

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 25, 2022
@Yongxuanzhang
Copy link
Member

comment for tracking 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Todo
Development

No branches or pull requests

5 participants