TEP-0018 Allow a Run to Specify a Default Bundle #210

coryrc · 2020-09-16T18:55:43Z

Allow a *Run to specify a bundle to lookup where to find references by
name instead of in the local cluster database.

Allow a *Run to specify a bundle to lookup where to find references by name instead of in the local cluster database.

tekton-robot · 2020-09-16T18:55:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign chetan-rns
You can assign the PR to them by writing /assign @chetan-rns in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

teps/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

coryrc · 2020-09-16T18:58:11Z

As specified in the proposal, I'm not sure how to make this observable. I believe Tasks/Pipelines from bundles will have annotations stating such? That could be enough?

vdemeester · 2020-09-17T07:46:20Z

teps/0018-default-bundle.md

+  - "@coryrc"
+creation-date: 2020-09-15
+last-updated: 2020-09-15
+status: in pr review


you could put proposed here 😝

vdemeester · 2020-09-17T07:52:38Z

teps/0018-default-bundle.md

+In a Pipeline, a Task can be referenced by name. This name is currently looked
+up in the current namespace where the Pipeline and PipelineRun exist. It should
+be possible for a PipelineRun or TaskRun to specify a bundle to be searched for
+a match instead.


This builds upon TEP-0005, as commented on #205, should we have the syntax in an update TEP-0005 or a new one specifically for OCI bundle usage in tektoncd/pipeline API ?

vdemeester · 2020-09-17T07:56:45Z

teps/0018-default-bundle.md

+## Motivation
+
+When CI is defined in-repo, some method must be provided to test Tekton objects
+during presubmit without altering Tasks being used by others. Additionally


presubmit should be defined here 👼

by here, I mean in the TEP 🙃

my guess, is that presubmit here, is local execution, before the commit really happen — git hooks jargon (https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks and https://pre-commit.com/ for ex), right ?

It's not a local pre-commit; it's like the "Review requested" section of this Github PR

I swear I was going to describe it later in the doc, sorry.

vdemeester · 2020-09-17T07:58:13Z

teps/0018-default-bundle.md

+
+### Goals
+
+Provide a different place to lookup Tekton objects when referenced solely by name.


I think there is a missing TEP (or information in a TEP, most likely TEP-0005) : how do we refer bundle in tektoncd/pipeline API ? right now, it is not defined in any TEP, there is only examples.

vdemeester · 2020-09-17T08:24:44Z

teps/0018-default-bundle.md

+I have a repository where I have the entire CI/CD system using Tekton
+resources defined in the repository. I wish to test commits to the repository
+(presubmits) before allowing them to merge. I wish to support using the exact
+resources when running locally.
+
+My Pipelines refer to Tasks defined in-repo by name only and same for
+PipelineRun->Pipeline and TaskRun->Task. I can still use `_Ref.bundle` if I
+am referring to resources which are not created by this branch of the repository.
+
+My presubmit test takes non-Run resources and creates a bundle. All PipelineRuns
+and TaskRuns are modified to use the bundle as the default lookup environment. I
+can now execute in the production namespace without interfering with the
+environment in the existing namespace.


I think, right now — without tekton oci bundle —, most "tekton as code" (aka CI/CD defined in the repository) approach are doing isolation by namespace (cc @afrittoli @dibyom @chmouel).

Assuming we have a way to reference bundle in the API and tooling to build bundle from a folder (or a set of files, or …), how would we solve this (without that TEP) ?

For pre-commit (local git hooks I assume), we would need to have a tool that (1) package the task/pipeline files into a bundle and (2) create a *Run object that refers those (resolves the pushed bundle, update on-the-fly the bundle reference). A similar workflow that comes to mind when describing this is ko — which does this for images and k8s yaml files. Something similar could be done for a tool like this, it could even be in tkn to be honest (or as a tkn plugin to start with) — tkn bundle resolve, tkn bundle run, …

Some open-questions then:

How the tools knows what bundle reference to replace on-the-fly ? ko uses the package path, and ko:// path, the tool could infer those from some conventions in the task/pipeline definition (some annotation, …)

Should it only work when bundle are specified ? (aka if I only have a task name, assuming it's on the cluster, the tool doesn't do anything)

In the light of this (if we agree, such a tool could exists), what is the benefit of having something server-side ? (default)

ko-type approach would mostly be equivalent, but not entirely. The machine you are launching Tekton from must have access and pathing to where the bundles are located; you could not control its permissions tightly. If server-side, cron jobs could be unable to access the Internet and only be able to create *Run custom resources in the cluster (though admittedly cron jobs are not usually security holes). Bundle locations must also be readable to users. Additionally, I think it's less "declarative" and more "imperative"? I am no k8s expert.

OTOH it means your bundle does not need to be readable by the cluster with Tekton.

I am also about to do the hack where I generate a unique namespace per each, but that's fragile.

vdemeester · 2020-09-17T08:25:55Z

teps/0018-default-bundle.md

+I wish to support multiple repositories with CI/CD defined in-repo in the same
+namespace without the possibility of name conflicts. The system creating
+the PipelineRun must always specify a default bundle.


The tool (described above) could be used in such scenario, a bit like what is currently done in our tekton as code experiment — where we create a new namespace, here we would run that tool to pack and push the bundle and execute the *Run with it.

vdemeester · 2020-09-17T08:26:22Z

teps/0018-default-bundle.md

+}
+```
+
+If `spec.lookup.bundle` is specified, the local database is never queried to


wdym by local database ?

etcd (sorry, I am not k8s expert)

The place where the Task ends up when you run kubectl apply -f my-task.yaml

vdemeester · 2020-09-17T08:27:34Z

teps/0018-default-bundle.md

+If any Tekton resources specify `bundle:` referring to bundles generated from
+resources specified in the repository being modified, a presubmit test must be
+aware of it and must modify the bundle reference to point to a version generated
+from the current commit being tested.


The tool described above could do that. Now that I think of it, tektoncd/triggers could, in the long-term future, integrate that tools/behavior directly (cc @dibyom)

vdemeester · 2020-09-17T08:28:11Z

teps/0018-default-bundle.md

+1. All references could be required to have a bundle key. When you want to use
+a custom version locally, you will need to modify the bundle to point to your
+local copy. Tooling could make this trivial to do.


Ah, I should have read the alternatives, my comments are describing this alternative I guess.

pierretasci · 2020-09-17T15:43:07Z

teps/0018-default-bundle.md

+
+## Proposal
+
+Create a new key for PipelineRun and TaskRun called `spec.lookup.bundle`; use it


This is an implementation detail but since you mention the API, something about a lookup field feels like the wrong tense?

One thing that other fields in Tekton have is a "default" field. Eg, params have a value and a default. Perhaps a PipelineRun, and TaskRun object need a default block for things like this? Something like spec.defaults.bundle?

Sorry, I am not sufficiently knowledgeable about k8s to judge names! You all decide and I'll write it down!

coryrc · 2020-10-26T17:18:51Z

People prefer a ko-like client-side tool.

TEP-0018 Default bundle

174ad37

Allow a *Run to specify a bundle to lookup where to find references by name instead of in the local cluster database.

tekton-robot requested review from AlanGreene and hrishin September 16, 2020 18:55

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 16, 2020

vdemeester reviewed Sep 17, 2020

View reviewed changes

vdemeester added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Sep 17, 2020

pierretasci mentioned this pull request Sep 17, 2020

TEP-0005: Make Tekton Bundles implementable #205

Merged

pierretasci reviewed Sep 17, 2020

View reviewed changes

coryrc closed this Oct 26, 2020

bobcatfish mentioned this pull request Nov 10, 2020

Add support for referencing Tasks in git tektoncd/pipeline#2298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0018 Allow a Run to Specify a Default Bundle #210

TEP-0018 Allow a Run to Specify a Default Bundle #210

coryrc commented Sep 16, 2020

tekton-robot commented Sep 16, 2020

coryrc commented Sep 16, 2020

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020

coryrc Sep 17, 2020 •

edited

Loading

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020 •

edited

Loading

coryrc Sep 17, 2020

coryrc Sep 17, 2020 •

edited

Loading

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020

coryrc Sep 17, 2020

coryrc Sep 17, 2020

vdemeester Sep 17, 2020

vdemeester Sep 17, 2020

coryrc Sep 17, 2020

pierretasci Sep 17, 2020

coryrc Sep 17, 2020

coryrc commented Oct 26, 2020


		### Goals

		Provide a different place to lookup Tekton objects when referenced solely by name.


		## Proposal

		Create a new key for PipelineRun and TaskRun called `spec.lookup.bundle`; use it

TEP-0018 Allow a Run to Specify a Default Bundle #210

TEP-0018 Allow a Run to Specify a Default Bundle #210

Conversation

coryrc commented Sep 16, 2020

tekton-robot commented Sep 16, 2020

coryrc commented Sep 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coryrc Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coryrc Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coryrc commented Oct 26, 2020

coryrc Sep 17, 2020 •

edited

Loading

vdemeester Sep 17, 2020 •

edited

Loading

coryrc Sep 17, 2020 •

edited

Loading