Export required Conversions for Reconciling the Deprecated CustomRun

[JeromeJu]

Changes

This commit exports the required conversions for reconciling v1beta1
CustomRun in the PipelineRun reconciler. It involves the following
ConvertFrom funcs:

• Param
• WorkspaceBinding
• TaskRef

ie. we would want to convert the fields back in v1beta1 at https://github.com/tektoncd/pipeline/blob/main/pkg/reconciler/pipelinerun/pipelinerun.go#L970-L975

part of #5541
/kind misc

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• [n/a] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
• Has Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including functionality, content, code)
• Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• [n/a] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
• [n/a] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

[Yongxuanzhang]

/assign

[chitrangpatel]

/assign

[EmmaMunley]

lgtm

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: EmmaMunley
To complete the pull request process, please ask for approval from jerop after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

• pkg/apis/OWNERS

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

[lbernick]

I'm curious why we need to export the individual conversion functions instead of just using the existing exported functions for converting Tasks, TaskRuns, etc?

[JeromeJu]

I'm curious why we need to export the individual conversion functions instead of just using the existing exported functions for converting Tasks, TaskRuns, etc?

Yeah I also tried to use the existing exported functions but for the createRunObject func, it is accessing the taskRef, workspaces and params from rpt *resources.ResolvedPipelineTask (and pipelineRunFacts.GetFinalTasks() ), and there were no upstream functions that I could use directly fetching from the PipelineRun itself. I was not sure if there were any previous functions that I could convert 🤔

[lbernick]

I'm curious why we need to export the individual conversion functions instead of just using the existing exported functions for converting Tasks, TaskRuns, etc?

Yeah I also tried to use the existing exported functions but for the createRunObject func, it is accessing the taskRef, workspaces and params from rpt *resources.ResolvedPipelineTask (and pipelineRunFacts.GetFinalTasks() ), and there were no upstream functions that I could use directly fetching from the PipelineRun itself. I was not sure if there were any previous functions that I could convert 🤔

Just a heads up, permalinks can help with linking to sections of code-- they ensure that if the code changes, you're still linking to the lines intended.

It's hard for me to see why this is necessary without the context of #6444; I'm not sure whether this PR makes sense on its own. Is the problem specifically that v1beta1 customruns might have v1beta1 params and workspace bindings, and those need to be converted somewhere? It might be easier to wait until #6498 is addressed.

I'd actually prefer to not have a PR split here, since the reason for this PR depends heavily on what is needed by the storage swap PR, and doesn't necessarily make sense as a change on its own. Other reviewers may have different opinions.

[lbernick]

v1beta1 customrun is not deprecated

[JeromeJu]

Thanks @lbernick . I think that makes a lot of sense and I will take the guidance since we can always reopen this if this could help break #6444 down.

I'm curious why we need to export the individual conversion functions instead of just using the existing exported functions for converting Tasks, TaskRuns, etc?

Yeah I also tried to use the existing exported functions but for the createRunObject func, it is accessing the taskRef, workspaces and params from rpt *resources.ResolvedPipelineTask (and pipelineRunFacts.GetFinalTasks() ), and there were no upstream functions that I could use directly fetching from the PipelineRun itself. I was not sure if there were any previous functions that I could convert 🤔

Just a heads up, permalinks can help with linking to sections of code-- they ensure that if the code changes, you're still linking to the lines intended.

It's hard for me to see why this is necessary without the context of #6444; I'm not sure whether this PR makes sense on its own. Is the problem specifically that v1beta1 customruns might have v1beta1 params and workspace bindings, and those need to be converted somewhere? It might be easier to wait until #6498 is addressed.

I'd actually prefer to not have a PR split here, since the reason for this PR depends heavily on what is needed by the storage swap PR, and doesn't necessarily make sense as a change on its own. Other reviewers may have different opinions.