Migrate core interceptors to use InterceptorType CRD #976

dibyom · 2021-02-26T21:08:26Z

Changes

Migrate core interceptors to use InterceptorType CRD

This commit does the following:

Add YAML definitions for installing each core interceptor using the
new InterceptorType CRD.
Modify the EventListener to use the InterceptorType CRD to find the
Interceptor's URL.

Fixes #868

This is NOT a breaking change since users can still configure their interceptors in the same way was before. Changes to the way a TriggerInterceptor is configured is coming in #869.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices
Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Release Notes

Depends on #960

tekton-robot · 2021-02-26T21:09:31Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_defaults.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_types.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	91.6%	-0.5
pkg/reconciler/v1alpha1/clusterinterceptor/clusterinterceptor.go	Do not exist	90.9%
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	73.1%	71.4%	-1.7

tekton-robot · 2021-03-04T20:43:33Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_defaults.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_types.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	91.6%	-0.5
pkg/reconciler/v1alpha1/clusterinterceptor/clusterinterceptor.go	Do not exist	90.9%
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	72.5%	71.1%	-1.4

tekton-robot · 2021-03-10T15:08:12Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_defaults.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_types.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	91.6%	-0.5
pkg/reconciler/v1alpha1/clusterinterceptor/clusterinterceptor.go	Do not exist	90.9%
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	72.5%	71.1%	-1.4

tekton-robot · 2021-03-11T17:18:12Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	91.6%	-0.5
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	72.5%	71.1%	-1.4

dibyom · 2021-03-12T16:37:01Z

/assign @wlynch
/assign @savitaashture
/assign @khrm

tekton-robot · 2021-03-15T23:00:02Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	91.6%	-0.5
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/reconciler/v1alpha1/eventlistener/eventlistener.go	78.0%	79.7%	1.7
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	72.5%	71.1%	-1.4

dibyom · 2021-03-16T14:35:52Z

/test pull-tekton-triggers-integration-tests

bigkevmcd

Some relatively minor mostly cosmetic issues, otherwise looks good to me.

pkg/interceptors/interceptors.go

pkg/interceptors/interceptors_test.go

pkg/reconciler/v1alpha1/clusterinterceptor/controller.go

pkg/interceptors/interceptors_test.go

pkg/interceptors/interceptors.go

tekton-robot · 2021-03-16T19:29:32Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/trigger_types.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	90.8%	-1.3
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/reconciler/v1alpha1/eventlistener/eventlistener.go	78.0%	79.7%	1.7
test/controller.go	72.5%	71.1%	-1.4

This commit does the following: 1. Add YAML definitions for installing each core interceptor using the new InterceptorType CRD. 2. Modify the EventListener to use the InterceptorType CRD to find the Interceptor's URL. Fixes tektoncd#868 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>

tekton-robot · 2021-03-16T19:43:02Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go	Do not exist	100.0%
pkg/apis/triggers/v1alpha1/trigger_types.go	Do not exist	100.0%
pkg/interceptors/interceptors.go	92.1%	90.8%	-1.3
pkg/reconciler/v1alpha1/clusterinterceptor/controller.go	Do not exist	0.0%
pkg/reconciler/v1alpha1/eventlistener/eventlistener.go	78.0%	79.7%	1.7
pkg/sink/sink.go	78.4%	77.1%	-1.3
test/controller.go	72.5%	71.1%	-1.4

savitaashture

/lgtm

Verified locally it works as expected
Thank you 👍

Just one question if user want to write custom interceptor what all things they have to take care? do we have any steps/instructions?

dibyom · 2021-03-17T14:36:18Z

Just one question if user want to write custom interceptor what all things they have to take care? do we have any steps/instructions?

Docs coming for those as part of #1001

tekton-robot · 2021-03-18T04:54:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: savitaashture

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:

~~OWNERS~~ [savitaashture]

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

With tektoncd#976, core interceptors are now installed as instances of the ClusterInterceptor CRD. They are also bundled in a separate YAML file. We need to wait for the Interceptors CRD to become established before we can create the core interceptors. Otherwise, we run the risk of trying to create the core-interceptors too early leading to installation errors. In tektoncd#976 we fixed the e2e install scripts to account for these changes but not the release pipeline. This commit updates the release pipeline to add the new interceptors YAML as part of the release assets. Signed-off-by: Dibyo Mukherjee <dibyo@google.com>

With tektoncd#976, core interceptors are now installed as instances of the ClusterInterceptor CRD. They are also bundled in a separate YAML file. We need to wait for the Interceptors CRD to become established before we can create the core interceptors. Otherwise, we run the risk of trying to create the core-interceptors too early leading to installation errors. In tektoncd#976 we fixed the e2e install scripts to account for these changes but not the release pipeline. This commit updates the release pipeline to add the new interceptors YAML as part of the release assets. Fixes tektoncd#1006 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>

With #976, core interceptors are now installed as instances of the ClusterInterceptor CRD. They are also bundled in a separate YAML file. We need to wait for the Interceptors CRD to become established before we can create the core interceptors. Otherwise, we run the risk of trying to create the core-interceptors too early leading to installation errors. In #976 we fixed the e2e install scripts to account for these changes but not the release pipeline. This commit updates the release pipeline to add the new interceptors YAML as part of the release assets. Fixes #1006 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>

tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 26, 2021

tekton-robot requested review from ncskier and wlynch February 26, 2021 21:08

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 26, 2021

dibyom mentioned this pull request Feb 26, 2021

Add a CRD for Interceptors #921

Closed

4 tasks

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 3, 2021

dibyom force-pushed the coreinter branch from 3aa6eb9 to a90fa14 Compare March 4, 2021 20:41

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2021

Base automatically changed from master to main March 10, 2021 15:03

dibyom force-pushed the coreinter branch from a90fa14 to 6bb2644 Compare March 11, 2021 17:16

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 11, 2021

dibyom marked this pull request as ready for review March 11, 2021 17:17

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2021

tekton-robot assigned khrm, savitaashture and wlynch Mar 12, 2021

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2021

dibyom force-pushed the coreinter branch from 6bb2644 to 430b518 Compare March 15, 2021 22:58

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2021

bigkevmcd reviewed Mar 16, 2021

View reviewed changes

dibyom mentioned this pull request Mar 16, 2021

Add API to configure Interceptors from a Trigger #1001

Merged

4 tasks

dibyom force-pushed the coreinter branch from 430b518 to ca81b29 Compare March 16, 2021 19:28

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 16, 2021

dibyom force-pushed the coreinter branch from ca81b29 to 052cf8c Compare March 16, 2021 19:41

savitaashture reviewed Mar 17, 2021

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021

savitaashture approved these changes Mar 18, 2021

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2021

tekton-robot merged commit 02c3896 into tektoncd:main Mar 18, 2021

dibyom deleted the coreinter branch March 18, 2021 17:00

dibyom mentioned this pull request Mar 18, 2021

Triggers release pipeline does not contain core interceptors #1006

Closed

dibyom mentioned this pull request Mar 18, 2021

Update publish task to include core interceptors #1007

Merged

4 tasks

concaf mentioned this pull request Mar 19, 2021

🤖 Triggering CI on branch 'release-next' after synching to upstream/master openshift/tektoncd-triggers#403

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate core interceptors to use InterceptorType CRD #976

Migrate core interceptors to use InterceptorType CRD #976

dibyom commented Feb 26, 2021 •

edited

Loading

tekton-robot commented Feb 26, 2021

tekton-robot commented Mar 4, 2021

tekton-robot commented Mar 10, 2021

tekton-robot commented Mar 11, 2021

dibyom commented Mar 12, 2021

tekton-robot commented Mar 15, 2021

dibyom commented Mar 16, 2021

bigkevmcd left a comment

tekton-robot commented Mar 16, 2021

tekton-robot commented Mar 16, 2021

savitaashture left a comment

dibyom commented Mar 17, 2021

tekton-robot commented Mar 18, 2021

Migrate core interceptors to use InterceptorType CRD #976

Migrate core interceptors to use InterceptorType CRD #976

Conversation

dibyom commented Feb 26, 2021 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Feb 26, 2021

tekton-robot commented Mar 4, 2021

tekton-robot commented Mar 10, 2021

tekton-robot commented Mar 11, 2021

dibyom commented Mar 12, 2021

tekton-robot commented Mar 15, 2021

dibyom commented Mar 16, 2021

bigkevmcd left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 16, 2021

tekton-robot commented Mar 16, 2021

savitaashture left a comment

Choose a reason for hiding this comment

dibyom commented Mar 17, 2021

tekton-robot commented Mar 18, 2021

dibyom commented Feb 26, 2021 •

edited

Loading