tektoncd / triggers

Event triggering with Tekton!
Apache License 2.0
546 stars 416 forks source link

`namespaceSelector` under `tiggerGroups` will not set `--is-multi-ns=true` on `EventListener` `Pod`. #1652

Closed flatheadmill closed 2 months ago

flatheadmill commented 9 months ago

Expected Behavior

Specifying a namespaceSelector under triggerGroups for an EventListener should create an event listener pod with with the --is-multi-ns flag set to true.

Actual Behavior

When a namespaceSelector under triggerGroups for an EventListener is specified without additionally specifying a namespaceSelector under spec the pod is created with --is-multi-ns=false.

Steps to Reproduce the Problem

With the following manifests in is-multi-ns-false.yaml...

apiVersion: v1
kind: Namespace
metadata:
    name: builderify
---
apiVersion: triggers.tekton.dev/v1beta1
kind: EventListener
metadata:
  name: github
  namespace: builderify
spec:
  triggerGroups:
  - name: push
    interceptors:
    - ref:
        name: "cel"
      params:
      - name: "filter"
        value: "header.match('X-GitHub-Event', 'push')"
    triggerSelector:
      labelSelector:
        matchLabels:
          github: push
      namespaceSelector:
        matchNames:
        - '*'

Run the following...

 $ kubectl apply -f is-multi-ns-false.yaml 
 $ kubectl -n builderify get pods -o yaml | grep is-multi-ns
      - --is-multi-ns=false
 $ kubectl delete -f is-multi-ns-false.yaml

With the following manifests in is-multi-ns-true.yaml...

apiVersion: v1
kind: Namespace
metadata:
    name: builderify
---
apiVersion: triggers.tekton.dev/v1beta1
kind: EventListener
metadata:
  name: github
  namespace: builderify
spec:
  namespaceSelector:
    matchNames:
    - will-not-match
  triggerGroups:
  - name: push
    interceptors:
    - ref:
        name: "cel"
      params:
      - name: "filter"
        value: "header.match('X-GitHub-Event', 'push')"
    triggerSelector:
      labelSelector:
        matchLabels:
          github: push
      namespaceSelector:
        matchNames:
        - '*'

Run the following...

 $ kubectl apply -f is-multi-ns-true.yaml 
 $ kubectl -n builderify get pods -o yaml | grep is-multi-ns
      - --is-multi-ns=true
 $ kubectl delete -f is-multi-ns-true.yaml 

With a namespaceSelector under spec set to match a non-existent namespace the pod is configured to inspect namespaces and the namespaceSelectors under triggerGroups works as expected. The namespaceSelector under spec will not match anything so there will be no duplicates. A temporary fix for people who encounter the same problem. (Assuming the maintainers agree it is a problem.)

Additional Info

Client Version: v1.28.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.6
Client version: 0.32.0
Pipeline version: v0.52.0
Triggers version: v0.25.0
Dashboard version: v0.40.1
khrm commented 9 months ago

/assign @khrm

cugykw commented 8 months ago

/cc @cugykw

khrm commented 3 months ago

/assign @khrm

seternate commented 2 months ago

@khrm I tried to implement a fix for this one. Let me know if anything needs to be changed: https://github.com/tektoncd/triggers/pull/1725