Migrate core interceptors to new format #1029
Conversation
tekton-robot
added
do-not-merge/work-in-progress
tekton-robot
added
the
size/L
|
The following is the coverage report on the affected files.
|
tekton-robot
added
the
needs-rebase
tekton-robot
removed
the
needs-rebase
|
The following is the coverage report on the affected files.
|
tekton-robot
removed
the
do-not-merge/work-in-progress
|
(The drop in coverage is due to missing tests for some err cases where the error happens due to malformed JSON. this should never happen in practice since we are converting from concrete structs) |
|
/hold cancel /assign @wlynch |
tekton-robot
removed
the
do-not-merge/hold
👍 How about to pass invalid json data from testcase to increase the coverage 🤔 Otherwise |
| ti.Params = []InterceptorParams{} | ||
| switch ti.Ref.Name { | ||
| case "bitbucket": | ||
| if err := addToParams(&ti.Params, "secretRef", ti.DeprecatedBitbucket.SecretRef); err != nil { |
There was a problem hiding this comment.
These could nil panic if ti or DeprecatedBitbucket is nil. We should add checks to prevent this (as well as for the other fields)
There was a problem hiding this comment.
Added a check for when ti is nil.
A bit convoluted but the checks for the other fields being not nil happens in the call ti.GetName() which ensures that if the name is bitbucket, then ti.DeprecatedBitbucket is not nil: https://github.com/tektoncd/triggers/blob/main/pkg/apis/triggers/v1alpha1/trigger_types.go#L145
That's going to be hard since we are not actually passing JSON but a TriggerInterceptor struct which will always serialize to valid JSON. |
This commit updates the defaulting webhook to automatically set any Triggers using the old style syntax for core interceptors to the new ref/params based syntax. Fixes tektoncd#1009 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
|
The following is the coverage report on the affected files.
|
tekton-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: savitaashture, wlynch 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 |
Changes
This commit updates the defaulting webhook to automatically set any Triggers
using the old style syntax for core interceptors to the new ref/params based
syntax.
Fixes #1009
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes