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.

Sign up for GitHub

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

Jump to bottom

Passes pipelines/tasks from bundles through defaulting before execution #4117

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Passes pipelines/tasks from bundles through defaulting before execution #4117

merged 2 commits into from
Jul 27, 2021

Conversation

sm43
Copy link
Member

@sm43 sm43 commented Jul 26, 2021 •
edited

for pipeline/tasks from yaml webhook does the defaulting but for pipeline/tasks
fetched from bundles, defaulting was not taking place due to
which validation error use to occur at execution.
This adds the defaulting for pipelines/tasks fetched from bundles.

Fixes #3874

Signed-off-by: Shivam Mukhade smukhade@redhat.com

Submitter Checklist

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

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 26, 2021
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 26, 2021
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/resources/pipelineref.go 70.4% 69.0% -1.4

@sm43
Copy link
Member Author

sm43 commented Jul 26, 2021

/test tekton-pipeline-unit-tests

@sm43
Copy link
Member Author

sm43 commented Jul 26, 2021

/kind bug

@tekton-robot tekton-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 26, 2021
@pritidesai
Copy link
Member

hey @sm43 please add more coverage if possible 🙏

@tekton-robot tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2021
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/resources/pipelineref.go 70.4% 69.0% -1.4

@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 27, 2021
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/resources/pipelineref.go 70.4% 86.2% 15.8

sm43 added 2 commits July 27, 2021 10:38
Passes pipelines from bundles through defaulting before execution
3ddba8b 
for pipeline from yaml webhook does the defaulting but for pipeline
fetched from bundles, defaulting was not taking place due to
which validation error use to occur at execution.
This adds the defaulting for pipelines fetched from bundles.

Signed-off-by: Shivam Mukhade <smukhade@redhat.com>
Passes tasks from bundles through defaulting before execution
56ed957 
for tasks from yaml webhook does the defaulting but for tasks
fetched from bundles, defaulting was not taking place
due to which validation error use to occur at execution.
This adds the defaulting for tasks fetched from bundles.

Signed-off-by: Shivam Mukhade <smukhade@redhat.com>
@sm43 sm43 changed the title Passes pipelines from bundles through defaulting before execution Passes pipelines/tasks from bundles through defaulting before execution Jul 27, 2021
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/pipelinerun/resources/pipelineref.go 70.4% 86.2% 15.8
pkg/reconciler/taskrun/resources/taskref.go 68.3% 77.3% 9.0

@sm43
Copy link
Member Author

sm43 commented Jul 27, 2021

/retest

@sm43
Copy link
Member Author

sm43 commented Jul 27, 2021

/test tekton-pipeline-unit-tests

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2021
@pritidesai
Copy link
Member

thanks @sm43

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2021
@tekton-robot tekton-robot merged commit a728ce3 into tektoncd:main Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdemeester vdemeester vdemeester approved these changes

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

@dlorenc dlorenc Awaiting requested review from dlorenc

Assignees

@pritidesai pritidesai

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pipelines in bundles do not go through defaulting before validation
4 participants
@sm43 @tekton-robot @pritidesai @vdemeester