Interceptors: Implement new config API aka how to use interceptors in Triggers

[dibyom]

The work for this issue is to implement changes to how users writing a Trigger configure Interceptors as described in TEP-0026 i.e. how to supply params declared by an Interceptor in a Trigger.

What?

Discuss options for how we should configure interceptors inside a Trigger.

Why?

TEP-009: Since interceptors are pluggabble, we can't make them part of the API spec anymore. We need a pluggable mechanism!

How do we do it today?

interceptors:
  - cel:
       filter:  `body.comment == "plz run my test"` # simplest case: string key-val
       overlays:                   # most complex obj that contains array of objs
          - key: "a_new_field"
            value: "2 * 5"
          - key: "another_field"
            value: "bar"
  -  github:
        eventTypes: ["pull_request", "push"]   # val is of type array of strings
        secretRef:                             # complex obj
           name: "github-secret"
           key: "webhook-token"

Design:

1. We need a way to refer to the Interceptor. The simplest, most consistent way would be to use a ref as we do with other resources e.g.

interceptors:
  - ref:
      apiVersion: interceptors.triggers.tekton.dev/v1alpha1
      kind: cel

1. (Based on a previous WG discussion) It might also makes sense to add an optional name field to identify what interceptor block we are using. This makes it more descriptive when you have multiple interceptors, and can be useful when using kustomize.

interceptors:
  - name: "payload-validation"
    ref:
      apiVersion: interceptors.triggers.tekton.dev/v1alpha1
      kind: github
  - name: "Cel -filter"
    ref:
      apiVersion: interceptors.triggers.tekton.dev/v1alpha1
      kind: cel

1. Finally, we need a way to supply parameters. Today, the paramter names are always keys while the values can be strings, array of strings (eventTypes in github), or complex objects (e.g. overlays in CEL, secretRef in github). The following section has some ideas for representing these:

Ideas for Params

1. Only allow string values

1. This is the simplest option and consistent with how we use params elsewhere.
2. We'd have to figure out a way to represent the current params that use non string types:

interceptors:
- name: "github-validation"
  ref:
    kind: GitHub
  params:
  - name: "eventTypes"
    value: "pull_request,"push" # part of same string separated by commas
  - name: "secretName"  # we have to split secretRed into two params
    value: "secret-name"
  - name: "secretKey"
    value: "key"

- name: "cel-filters-overlays"
  ref:
    kind: CEL
  params:
  - name: filter # filter is a special field. everything else is overlay?
  - value: `body.comment == "plz run my test"`
  - name: "a_new_field" # overlay fields are implicit
    value: "2 * 5"
  - name: "another_field"
    value: "bar"

1b. Only string value params but CEL is split into two interceptors

Basically the same as 1, but we split CEL into two CELFilter and CELOverlay(or CELExtensions)

interceptors:
- name: "github-validation"
  ref:
    kind: GitHub
  params:
  - name: "eventTypes"
    value: "pull_request,"push" # part of same string separated by commas
  - name: "secretName"  # we have to split secretRed into two params
    value: "secret-name"
  - name: "secretKey"
    value: "key"

- name: "cel-filters-overlays"
  ref:
    kind: CELFilter
  params:
  - name: filter # filter is a special field. everything else is overlay?
  - value: `body.comment == "plz run my test"`

- name: "cel-filters-overlays"
  ref:
    kind: CELOverlay
  params:
  - name: "a_new_field"
    value: "2 * 5"
  - name: "another_field"
    value: "bar"

2. Make params values be any valid JSON

1. Internally, we would model the param type as map[string]interface{}

2. It makes it look like the params are part of the API and is closest to what we do today

3. Later, we can add validation (OpenAPI) on what exactly is accepted via the API i.e. the Interceptor CRD will declare what param names and values it accepts and the controller/webhook can validate those on creation.

interceptors:
- name: "cel"
  ref:
    kind: cel
  params: # param value is no longer an array. It can be any valid JSON
    eventTypes: ["pull_request,"push"]
    overlays:
    - key: "a_new_field"
      value: "2 * 5"

3. Separate out params and extensions explicitly

The params today are combinations of two things -- input values (e.g. secretRef, filters), and output results (overlays).This option splits two explicitly.

1. This approach makes it explicit which extensions (or results) are accepted.

2. This approach will require a change to the Interceptor HTTP API (specifically the InterceptorRequest type).

interceptors:
- name: "cel"
  ref:
    kind: cel
  params:
    - name: "eventTypes"
      value: ["pull_request,"push"] # TODO: String or interface{} for the value?
  extensions:
    - name: "a_new_field"
      value: "2 * 5"

[dibyom]

/cc @bigkevmcd @wlynch would like to get your thoughts on the ideas above.

(I'm currently doing a PoC with options 2 and 3)

[jmcshane]

I've been reading through the docs on TEP-0026 and TEP-0022 and would definitely think that option 3 makes sense because params and extensions inherently have a different meaning in the processing of an interceptor.

When I think about migrating our current webhook extensions to the format outlined in TEP-0026 (and turning it into an InterceptorConfig CRD kind), I think that each of the webhooks explicitly sets specific extension fields. I would just ask that value be optional on the extension field because the webhook may not need any data from the Trigger itself to determine the value of the extension in the response.

I would think that params can be a generic interface{} type. Even looking at the current git interceptor configurations, this makes sense:

# Current config
- gitlab:
    secretRef:
      secretName: foo
      secretKey: bar
    eventTypes:
      - Push Hook

becomes

- type: gitlab
  params:
  - name: secretRef
    value:
      {"secretName":"foo", "secretName":"bar"} # or however you want to get a JSON map into YAML
  - name: eventTypes
    value:
    - Push Hook

I'm thinking about a pluggable oauth client config interceptor where you would specify the jwks URL and required roles in an JWT token. Now, we could return the roles to the interceptor chain as an extension for further processing or validate the roles in the interceptor by specifying extensions or not and having the downstream API respond appropriately.

# First model where roles are validated in the downstream plugin
- type: jwt
  params:
  - name: jwks_url
     value: https://
  - name: roles
    value:
    - group_1
    - group_2
# Second model where roles are passed back up to the chain as extensions:
- type: jwt
  params:
  - name: jwks_url
     value: https://
  extensions:
  - name: roles

Then, the downstream plugin could make a determination on behavior based on the params/extensions passed in. (Apologies for the wall of text I've just been thinking about this project a lot lately)

[dibyom]

@jmcshane Thanks for the feedback and the use case is much appreciated!

[dibyom]

Closed by #1001