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

Support v1 Task&Pipeline in remote resolution #6141

Closed
Yongxuanzhang opened this issue Feb 9, 2023 · 4 comments · Fixed by #6254
Closed

Support v1 Task&Pipeline in remote resolution #6141

Yongxuanzhang opened this issue Feb 9, 2023 · 4 comments · Fixed by #6254
Assignees
Labels
area/resolution Issues related to remote resolution kind/bug Categorizes issue or PR as related to a bug.

Comments

@Yongxuanzhang
Copy link
Member

Yongxuanzhang commented Feb 9, 2023

Expected Behavior

Remote resolution can resolve remote v1 tasks and pipelines

Actual Behavior

Remote resolution cannot resolve remote v1 tasks and pipelines

The pipelinerun is not created, and error log is:

"error":"1 error occurred:\n\t* failed to get pipeline: failed to convert obj [tekton.dev/v1](http://tekton.dev/v1), Kind=Pipeline into Pipeline\n\n",

Steps to Reproduce the Problem

apply the sample pipeline run:

apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  name: example-pipelinerun
  namespace: trusted-resources
spec:
  pipelineRef:
    resolver: git
    params:
      - name: url
        value: https://github.com/Yongxuanzhang/sample-tekton-pipeline
      - name: revision
        value: main
      - name: pathInRepo
        value: signed-pipeline.yaml

Additional Info

  • Kubernetes version:

    Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.1", GitCommit:"86ec240af8cbd1b60bcc4c03c20da9b98005b92e", GitTreeState:"clean", BuildDate:"2021-12-16T11:41:01Z", GoVersion:"go1.17.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-19T15:39:43Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"linux/amd64"}
  • Tekton Pipeline version:

    Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

Client version: 0.23.1
Pipeline version: devel (latest main)
@Yongxuanzhang Yongxuanzhang added the kind/bug Categorizes issue or PR as related to a bug. label Feb 9, 2023
@JeromeJu
Copy link
Member

JeromeJu commented Feb 9, 2023

Thanks for creating the issue @Yongxuanzhang

It feels to me 2 ways moving forward:

  • Separating the folder for v1/ v1beta1 remotely/resolved Task
  • Converting to V1beta1 if the accepted Task is v1 here, which could be changed until we swap to v1 as storage version

But I was not sure how we should specify this somewhere for the usage of resovlers?

cc @abayer this is the use case as discussed offline:)

@vdemeester
Copy link
Member

I think most likely we just need to make it so that the resolver can "marshal/unmarshal" v1 as well as v1beta1, isn't it ?

// resolvePipeline accepts an impl of remote.Resolver and attempts to
// fetch a pipeline with given name. An error is returned if the
// resolution doesn't work or the returned data isn't a valid
// v1beta1.PipelineObject.
func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) {
	obj, configSource, err := resolver.Get(ctx, "pipeline", name)
	if err != nil {
		return nil, nil, err
	}
	pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj)
	if err != nil {
		return nil, nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String())
	}
	return pipelineObj, configSource, nil
}

// readRuntimeObjectAsPipeline tries to convert a generic runtime.Object
// into a v1beta1.PipelineObject type so that its meta and spec fields
// can be read. An error is returned if the given object is not a
// PipelineObject or if there is an error validating or upgrading an
// older PipelineObject into its v1beta1 equivalent.
func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object) (v1beta1.PipelineObject, error) {
	if pipeline, ok := obj.(v1beta1.PipelineObject); ok {
		return pipeline, nil
	}

	return nil, errors.New("resource is not a pipeline")
}

We would want to support both, and call the conversion to whatever the storage version is at the time.

@JeromeJu
Copy link
Member

Yes:) I think conversion makes sense and to support both until the deprecation of v1beta1.

I think most likely we just need to make it so that the resolver can "marshal/unmarshal" v1 as well as v1beta1, isn't it ?

// resolvePipeline accepts an impl of remote.Resolver and attempts to
// fetch a pipeline with given name. An error is returned if the
// resolution doesn't work or the returned data isn't a valid
// v1beta1.PipelineObject.
func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) {
	obj, configSource, err := resolver.Get(ctx, "pipeline", name)
	if err != nil {
		return nil, nil, err
	}
	pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj)
	if err != nil {
		return nil, nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String())
	}
	return pipelineObj, configSource, nil
}

// readRuntimeObjectAsPipeline tries to convert a generic runtime.Object
// into a v1beta1.PipelineObject type so that its meta and spec fields
// can be read. An error is returned if the given object is not a
// PipelineObject or if there is an error validating or upgrading an
// older PipelineObject into its v1beta1 equivalent.
func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object) (v1beta1.PipelineObject, error) {
	if pipeline, ok := obj.(v1beta1.PipelineObject); ok {
		return pipeline, nil
	}

	return nil, errors.New("resource is not a pipeline")
}

We would want to support both, and call the conversion to whatever the storage version is at the time.

@lbernick lbernick added the area/resolution Issues related to remote resolution label Feb 15, 2023
@Yongxuanzhang
Copy link
Member Author

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resolution Issues related to remote resolution kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants