Slack interceptor 1542 #1548

ilan-pinto · 2023-03-06T09:54:41Z

Changes

Added a Slack Interceptor that allows you to extract fields from a slack slash command payload which are sent in the http form-data section.

Submitter Checklist

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

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

See the contribution guide for more details.

Release Notes


Added a Slack `Interceptor` that allows you to extract fields from a slack slash command [payload](https://api.slack.com/interactivity/slash-commands#app_command_handling) which are sent in the  http form-data section.

tekton-robot · 2023-03-06T09:54:50Z

Hi @ilan-pinto. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

khrm

/ok-to-test

tekton-robot · 2023-03-06T09:57:39Z

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/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	71.1%	-0.9

ilan-pinto · 2023-03-06T19:17:33Z

@khrm how can i fix the failing test ?

It seems to be coming from the framework

khrm

I think you are using go1.18 or go1.20. While using scripts in hack folder, you need to use go1.19 version because test scripts uses go1.19 version.

tekton-robot · 2023-03-07T10:04:43Z

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/interceptors/server/server.go	78.9%	79.3%	0.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	71.1%	-0.9

tekton-robot · 2023-03-07T11:36:43Z

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/interceptors/server/server.go	78.9%	79.3%	0.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	71.1%	-0.9

tekton-robot · 2023-03-07T12:11:42Z

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/interceptors/server/server.go	78.9%	79.3%	0.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	71.1%	-0.9

tekton-robot · 2023-03-07T14:05:48Z

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/interceptors/server/server.go	78.9%	82.3%	3.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	71.1%	-0.9

tekton-robot · 2023-03-10T13:13:51Z

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/interceptors/server/server.go	78.9%	79.3%	0.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	69.7%	-2.3

tekton-robot · 2023-03-12T07:19:46Z

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/interceptors/server/server.go	78.9%	79.3%	0.3
pkg/interceptors/slack/slack.go	Do not exist	84.6%
pkg/sink/sink.go	72.0%	69.7%	-2.3

dibyom · 2023-03-20T19:54:10Z

docs/interceptors.md

+
+```yaml
+  annotations:
+    tekton.dev/payload-validation: "false"


I wonder if we should update our payload validation to allow json and form/urlencoded content types and return invalid for anything else

dibyom · 2023-03-20T19:55:16Z

test/e2e-tests-examples.sh

  kubectl get eventlisteners
 }

 ignoreExamples=( cron trigger-ref triggergroups )

 port_forward_and_curl() {
-
+sleep 10


why do we need a sleep 10?

I faced issues on my local when using kind. it took more than 3 sec for the event listener to be available

test/e2e-tests-examples.sh

dibyom · 2023-03-20T19:56:58Z

pkg/apis/triggers/v1beta1/trigger_types.go

+	// the Requested fields to be extracted from data form
+
+	// +listType=atomic
+	RequestedFields []string `json:"requestedFields,omitempty"`


is this a mandatory field? or do we extract all fields by default if no requestedFields is specifed

We extract only the requested fields.
it wont do any work if nothing is specified

dibyom · 2023-03-20T20:01:47Z

pkg/sink/sink.go

@@ -554,7 +558,24 @@ func (r Sink) ExecuteInterceptors(trInt []*triggersv1.TriggerInterceptor, in *ht
 		}
 		// Clear interceptorParams for the next interceptor in chain
 		request.InterceptorParams = map[string]interface{}{}
+
+		// check if string is urlencoded


The logic here looks fine but shouldn't we call it when we first create the request object around line 467?

@dibyom, good call. It simplified the Slack interceptor.
All tests are ok on my local but for some reason this test fails in GitHub action

tekton-robot · 2023-03-21T13:29:23Z

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/interceptors/slack/slack.go	Do not exist	80.0%
pkg/sink/sink.go	72.0%	70.8%	-1.2

ilan-pinto · 2023-03-21T14:21:53Z

/test pull-tekton-triggers-integration-tests

khrm · 2023-03-21T17:40:42Z

The test is failing because resources don't exist in 0.46. We need to update the getting started pipeline. So it's not an issue with this pr.

savitaashture · 2023-03-24T14:35:10Z

/test pull-tekton-triggers-integration-tests

dibyom · 2023-04-19T20:14:18Z

/approve

Thanks so much @ilan-pinto

tekton-robot · 2023-04-19T20:14:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom, khrm

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~~ [dibyom,khrm]

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

tekton-robot · 2023-04-26T07:37:56Z

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/interceptors/server/server.go	78.9%	81.0%	2.0
pkg/interceptors/slack/slack.go	Do not exist	80.0%
pkg/sink/sink.go	72.0%	70.8%	-1.2

ilan-pinto · 2023-04-26T08:01:05Z

/retest

pkg/interceptors/slack/slack.go

…ommand payload A Slack allows you to extract fields from a slack slash command [payload](https://api.slack.com/interactivity/slash-commands#app _command_handling) which are sent in the http form-data section. Closes tektoncd#1542 added generic urlencode handling modified go mod file

tekton-robot · 2023-04-27T07:47:56Z

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/interceptors/slack/slack.go	Do not exist	80.0%
pkg/sink/sink.go	72.0%	70.8%	-1.2

khrm

/lgtm

khrm

/kind feature

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 6, 2023

tekton-robot requested review from iancoffey and wlynch March 6, 2023 09:54

tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 6, 2023

khrm reviewed Mar 6, 2023

View reviewed changes

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 6, 2023

ilan-pinto mentioned this pull request Mar 6, 2023

Slack interceptor #1547

Closed

1 task

ilan-pinto mentioned this pull request Mar 6, 2023

Issue 1526 slack interceptors #1528

Closed

4 tasks

khrm reviewed Mar 6, 2023

View reviewed changes

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 0193bb1 to 500f0ca Compare March 7, 2023 09:58

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 7, 2023

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 500f0ca to 698e9df Compare March 7, 2023 11:32

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 698e9df to 2d89d9f Compare March 7, 2023 12:08

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 2d89d9f to f34dcf5 Compare March 7, 2023 14:02

ilan-pinto force-pushed the slack_Interceptor_1542 branch from f34dcf5 to b2f5c92 Compare March 10, 2023 13:11

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

ilan-pinto force-pushed the slack_Interceptor_1542 branch from b2f5c92 to a66d1fe Compare March 12, 2023 07:17

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

dibyom reviewed Mar 20, 2023

View reviewed changes

ilan-pinto force-pushed the slack_Interceptor_1542 branch from fc16165 to 564a1a8 Compare March 21, 2023 13:23

savitaashture added this to the Triggers v0.24 milestone Mar 22, 2023

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2023

inital changes

e766a20

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 564a1a8 to ccbc807 Compare April 25, 2023 09:45

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 25, 2023

ilan-pinto force-pushed the slack_Interceptor_1542 branch from ccbc807 to 2f0d660 Compare April 26, 2023 07:35

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 26, 2023

khrm reviewed Apr 27, 2023

View reviewed changes

pkg/interceptors/slack/slack.go Show resolved Hide resolved

ilan-pinto force-pushed the slack_Interceptor_1542 branch from 2f0d660 to 65b1d92 Compare April 27, 2023 07:45

khrm reviewed Apr 28, 2023

View reviewed changes

tekton-robot assigned khrm Apr 28, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2023

tekton-robot merged commit f031e3c into tektoncd:main Apr 28, 2023
6 checks passed

khrm reviewed May 11, 2023

View reviewed changes

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 11, 2023

This was referenced May 30, 2023

Feature: EventListener response customization #788

Open

Support trigger via CloudEvents #1439

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Slack interceptor 1542 #1548

Slack interceptor 1542 #1548

ilan-pinto commented Mar 6, 2023 •

edited

tekton-robot commented Mar 6, 2023

khrm left a comment

tekton-robot commented Mar 6, 2023

ilan-pinto commented Mar 6, 2023

khrm left a comment

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 10, 2023

tekton-robot commented Mar 12, 2023

dibyom Mar 20, 2023

dibyom Mar 20, 2023

dibyom Mar 20, 2023

ilan-pinto Mar 21, 2023

dibyom Mar 20, 2023

ilan-pinto Mar 21, 2023 •

edited

dibyom Mar 20, 2023

ilan-pinto Mar 21, 2023

tekton-robot commented Mar 21, 2023

ilan-pinto commented Mar 21, 2023

khrm commented Mar 21, 2023

savitaashture commented Mar 24, 2023

dibyom commented Apr 19, 2023

tekton-robot commented Apr 19, 2023

tekton-robot commented Apr 26, 2023

ilan-pinto commented Apr 26, 2023

tekton-robot commented Apr 27, 2023

khrm left a comment

khrm left a comment

Slack interceptor 1542 #1548

Slack interceptor 1542 #1548

Conversation

ilan-pinto commented Mar 6, 2023 • edited

Changes

Submitter Checklist

Release Notes

tekton-robot commented Mar 6, 2023

khrm left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 6, 2023

ilan-pinto commented Mar 6, 2023

khrm left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 7, 2023

tekton-robot commented Mar 10, 2023

tekton-robot commented Mar 12, 2023

dibyom Mar 20, 2023

Choose a reason for hiding this comment

dibyom Mar 20, 2023

Choose a reason for hiding this comment

dibyom Mar 20, 2023

Choose a reason for hiding this comment

ilan-pinto Mar 21, 2023

Choose a reason for hiding this comment

dibyom Mar 20, 2023

Choose a reason for hiding this comment

ilan-pinto Mar 21, 2023 • edited

Choose a reason for hiding this comment

dibyom Mar 20, 2023

Choose a reason for hiding this comment

ilan-pinto Mar 21, 2023

Choose a reason for hiding this comment

tekton-robot commented Mar 21, 2023

ilan-pinto commented Mar 21, 2023

khrm commented Mar 21, 2023

savitaashture commented Mar 24, 2023

dibyom commented Apr 19, 2023

tekton-robot commented Apr 19, 2023

tekton-robot commented Apr 26, 2023

ilan-pinto commented Apr 26, 2023

tekton-robot commented Apr 27, 2023

khrm left a comment

Choose a reason for hiding this comment

khrm left a comment

Choose a reason for hiding this comment

ilan-pinto commented Mar 6, 2023 •

edited

ilan-pinto Mar 21, 2023 •

edited