tektoncd / community

Community documentation for the Tekton project
https://tekton.dev
Apache License 2.0
363 stars 221 forks source link

TEP-0154 Concise Remote Resolver Syntax #1138

Closed chitrangpatel closed 7 months ago

chitrangpatel commented 8 months ago

This PR proposes to include a concise remote resolver syntax to reference remote resources. It addresses the feature request https://github.com/tektoncd/pipeline/issues/7508.

/kind tep

chitrangpatel commented 8 months ago

/assign @wlynch

chitrangpatel commented 8 months ago

cc @chmouel PTAL (specifically for http-resolver) 🙂

dibyom commented 8 months ago

/approve /hold

Added a hold in case other reviewers want to take a look before the TEP is merged. Feel free to remove after

chitrangpatel commented 8 months ago

If we want to be consice, couldn't we use name for this instead adding url and duplicating some information (the name) ? I think it would be backward compatible : it's {name}://… we infer the name and we don't read the rest, it's {name} only it's the current syntax that applies. In ResolutionRequest we would pass the url or the parameters and let the resolver handle that (and/or the helper code we provide).

Just the name alone won't help. That's because it then requires that the value starts with the resolver name. And that does not work with something like the http resolver where the scheme could be http or https. e.g. https://github.com/tektoncd/pipeline is probably meant to go to the git resolver. https://raw.githubusercontent.com/tektoncd/pipeline/main/examples/v1/taskruns/array-default.yaml is meant to go to the http resolver.

So in the above case, the name alone does not work. But name with the resolver works because we know where the resolution request is supposed to go:

taskRef:
  name: "https://github.com/tektoncd/pipeline"
  resolver: git

Can we write an example for a custom resolver ? They are part of the resolver "feature set" (to be able to write your own resolver) and thus we should make sure we take them into account in this proposal.

Can do!

vdemeester commented 8 months ago

@chitrangpatel, yeah, as you commented here, we would want "resolver" to be able to say "I handle those sheme", and some sort of priority.

Now, even with that, the git+https is a slight… tricky one to achieve. If the http resolver has a higher priority than git, would we always let the http resolver handle it, fail, and give it to the next ? In case of an 401 (unauthorized), would we still do the same (with a high level of chance it will be 401 with the git provider as well).

Some history note (that might not have any "impact"), the current syntax is also there because it was decided to be consistent with the rest of the API (on params). The initial proposal looked like

  name: …
  params:
    - foo: bar
    - {name}: {value}
chitrangpatel commented 8 months ago

If we want to be consice, couldn't we use name for this instead adding url and duplicating some information (the name) ? I think it would be backward compatible : it's {name}://… we infer the name and we don't read the rest, it's {name} only it's the current syntax that applies. In ResolutionRequest we would pass the url or the parameters and let the resolver handle that (and/or the helper code we provide).

Can we write an example for a custom resolver ? They are part of the resolver "feature set" (to be able to write your own resolver) and thus we should make sure we take them into account in this proposal.

Done! I added a section on Custom Resolver. Also, in the Design Details, I added a section on expanding the Resolver interface so that custom Resolvers can leverage it.

tekton-robot commented 7 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom, vdemeester

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[teps/OWNERS](https://github.com/tektoncd/community/blob/main/teps/OWNERS)~~ [dibyom,vdemeester] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
chitrangpatel commented 7 months ago

/hold cancel