feat(HACBS-2461): add pipelineRef struct to reference a pipeline #18
Conversation
tekton/pipeline_ref.go
Outdated
| tektonParams = append(tektonParams, tektonv1.Param{ | ||
| Name: p.Name, | ||
| Value: tektonv1.ParamValue{ | ||
| Type: "string", |
| } | ||
|
|
||
| type ResolverRef struct { | ||
| Resolver string `json:"resolver"` |
There was a problem hiding this comment.
I said that we should limit the possible values, but let's avoid this to support custom resolvers.
There was a problem hiding this comment.
ok. we do intend to restrict what pipelines can be used, right? but I guess that validation will go to the controller.
There was a problem hiding this comment.
I didn't quite get this comment, sorry. Could you rephrase it?
tekton/pipeline_ref.go
Outdated
|
|
||
| type PipelineRef struct { | ||
| Name string `json:"name,omitempty"` | ||
| ResolverRef ResolverRef `json:"resolverRef"` |
There was a problem hiding this comment.
This should be optional as well if we support the same definition Tekton supports. Another option and maybe I like it would be to remove this struct and rename ResolverRef to PipelineRef.
In fact, the RP/RPA wil have probably something like:
Spec:
Applications
PipelineRef
Resolver
Params
There was a problem hiding this comment.
Yep, that's what PipelineRun.spec has as well and I thought that's how I thought the types are set up, but you correctly pointed out that the types don't match what's in the PipelineRun yaml. Just spelling it out here.
I did as you suggested. Please take a look.
There was a problem hiding this comment.
If you look at PipelineRun docs, you have something like this:
spec:
pipelineRef:
resolver: bundles
params:
- name: bundle
value: docker.io/myrepo/mycatalog:v1.0But in the types definition, there is one extra level:
type ResolverRef struct {
Resolver ResolverName `json:"resolver,omitempty"`
Params Params `json:"params,omitempty"`
}
type PipelineRef struct {
Name string `json:"name,omitempty"`
APIVersion string `json:"apiVersion,omitempty"`
ResolverRef `json:",omitempty"`
}I was just saying that originally I didn't realize this and assumed that the types are defined the way I would expect from looking at the docs (no middle layer). And that's the way we want to define it in our copy of the structure. But in any case, I did as you suggested.
tekton/pipeline_ref.go
Outdated
|
|
||
| func (pr PipelineRef) ToTektonPipelineRef() tektonv1.PipelineRef { | ||
| tektonParams := tektonv1.Params{} | ||
| for _, p := range pr.ResolverRef.Params { |
There was a problem hiding this comment.
| for _, p := range pr.ResolverRef.Params { | |
| for _, param := range pr.ResolverRef.Params { |
Also, empty line before this one please.
tekton/pipeline_ref.go
Outdated
| } | ||
|
|
||
| func (pr PipelineRef) ToTektonPipelineRef() tektonv1.PipelineRef { | ||
| tektonParams := tektonv1.Params{} |
There was a problem hiding this comment.
You can call it just params. Not used in this context and doesn't seem confusing to me.
mmalina
force-pushed
the
git-resolver
branch
from
941afe9
to
e826bef
Compare
This will be used in release-service (and possibly others in the future) to reference a pipeline in a ReleaseStrategy CR. This definition will then be converted and used in PipelineRuns created by the release service. Signed-off-by: Martin Malina <mmalina@redhat.com>
mmalina
force-pushed
the
git-resolver
branch
from
e826bef
to
c22bb9d
Compare
|
Since this will really only be used in the release-service, we decided not to add this here, but directly to the release-service repo: konflux-ci/release-service#252 |
This will be used in release-service (and possibly others in the future) to reference a pipeline in a ReleaseStrategy CR. This definition will then be converted and used in PipelineRuns created by the release service.