Events for cluster scoped resource like KongClusterPlugins are created in default namespace

[pmalek]

Problem statement

KIC supports emitting kubernetes native events https://docs.konghq.com/kubernetes-ingress-controller/latest/production/observability/events/ like KongConfigurationApplyFailed

These are created for all resources that are found to be relevant for an issue.

This system fails to create events for cluster scoped resources because events need to be created in a concrete namespace.

Related issues

kubernetes/client-go#1262

Acceptance criteria

• Has introduction in documents to introduce the status quo
• Confirm the behavior when --watch-namespace is set so KIC may not be able to create events in default namespace
  • Able to create events related cluster scoped resources if KIC cannot create events in default namespace

[pmalek]

EDIT:

Actually I somehow missed the event generated in the default namespace when deploying examples and I actually got this:

default     33m         Warning   KongConfigurationApplyFailed   kongclusterplugin/global-exit-transformer   invalid config.functions.1: Error parsing function: /usr/local/share/lua/5.1/kong/tools/sandbox.lua:128: loading of untrusted Lua code disabled because 'untrusted_lua' config option is set to 'off'

This still is not technically correct because the cluster plugin is not related to the namespace in anyway. 🤔

Not sure if a better option for this wouldn't be to try to attach this to KIC's pod.

Leaving this open for now.

[rainest]

Do we have an example broken plugin to easily replicate this?

I'd recommend our install namespace, though still using the kind/name/etc. for the affected object. We can leave namespace in https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#objectreference-v1-core blank; it's not required.

We should have permission to our own namespace always, but won't necessarily have access to default. That said, I'm not sure if we have the watchNamespaces chart role generation set to always include our own namespace (offhand it looks like we don't, we just use the value directly and don't append our namespace to it). Adding our own namespace should be less a concern than adding default assuming we do need to fix that.

That said, it looks like we have no control over this unless we use a hack to forcibly add a namespace to an object that wouldn't normally have one (dunno if we actually can do that--probably, but I didn't try). We just call eventRecorder.Event(), which is a a type and method implemented by client-go. That just says the Event is "created in the same namespace as the reference object", and we don't provide it with a namespace separately, just the runtime.Object.

Digging through the implementation confirms that it does use default as an arbitrary fallback when the subject object has none set, so unless we modify the input object, that's what we'll get. Others have raised this issue a few times upstream, but it's never gotten traction, just stalebotted.

[pmalek]

From the options mentioned in kubernetes/client-go#781 it seems that

setting event' namespace to our CRD controller's namespace (app-specific): this seems to be the most reasonable option, however, its rejected by the server due to "involved object's namespace does not match event.namespace":

can't be done because the event and object namespace have to match.

https://github.com/kubernetes/kubernetes/blob/9791f0d1f39f3f1e0796add7833c1059325d5098/pkg/apis/core/validation/events.go#L152-L155

---

As for an example that would actually trigger this:

apiVersion: configuration.konghq.com/v1
kind: KongClusterPlugin
config:
  config:
    latency_metrics: true
metadata:
  annotations:
    kubernetes.io/ingress.class: kong
  labels:
    global: "true"
  name: prometheus
plugin: prometheus

k get events --field-selector reason=KongConfigurationApplyFailed -A
NAMESPACE   LAST SEEN   TYPE      REASON                         OBJECT                         MESSAGE
default     3s          Warning   KongConfigurationApplyFailed   kongclusterplugin/prometheus   invalid config.config: unknown field

or a full working manifest:

apiVersion: v1
kind: Service
metadata:
  name: echo
spec:
  ports:
    - protocol: TCP
      name: http
      port: 80
      targetPort: http
  selector:
    app: echo
---
apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: echo
  name: echo
spec:
  replicas: 1
  selector:
    matchLabels:
      app: echo
  template:
    metadata:
      labels:
        app: echo
    spec:
      containers:
        - name: echo
          image: registry.k8s.io/e2e-test-images/agnhost:2.40
          command:
            - /agnhost
            - netexec
            - --http-port=8080
          ports:
            - containerPort: 8080
              name: http
          env:
            - name: NODE_NAME
              valueFrom:
                fieldRef:
                  fieldPath: spec.nodeName
            - name: POD_NAME
              valueFrom:
                fieldRef:
                  fieldPath: metadata.name
            - name: NAMESPACE
              valueFrom:
                fieldRef:
                  fieldPath: metadata.namespace
            - name: POD_IP
              valueFrom:
                fieldRef:
                  fieldPath: status.podIP
          resources:
            requests:
              cpu: 10m
---
apiVersion: gateway.networking.k8s.io/v1
kind: GatewayClass
metadata:
  name: kong
  annotations:
    konghq.com/gatewayclass-unmanaged: "true"
spec:
  controllerName: konghq.com/kic-gateway-controller
---
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
  name: kong
spec:
  gatewayClassName: kong
  listeners:
  - name: http
    protocol: HTTP
    port: 80
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: echo
spec:
  parentRefs:
  - name: kong
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /
    backendRefs:
    - name: echo
      kind: Service
      port: 80
---
apiVersion: configuration.konghq.com/v1
kind: KongClusterPlugin
config:
  config:
    latency_metrics: true
metadata:
  annotations:
    kubernetes.io/ingress.class: kong
  labels:
    global: "true"
  name: prometheus
plugin: prometheus

[czeslavo]

Not sure if a better option for this wouldn't be to try to attach this to KIC's pod.

I think it's better to stick to the default behavior client-go provides (creating events attached to cluster-scoped objects in the default namespace) as this is correctly handled e.g. in kubectl describe command. Despite the event's namespace, kubectl uses an object reference to fetch the object-related events (source code).

k describe kongclusterplugin exit-transformer-example
Name:         exit-transformer-example
Namespace:
Labels:       <none>
Annotations:  kubernetes.io/ingress.class: kong
API Version:  configuration.konghq.com/v1
Config:
  Functions:
    return function(status, body, headers) return status, body, headers end
Kind:  KongClusterPlugin
Metadata:
  Creation Timestamp:  2024-04-10T08:46:16Z
  Generation:          1
  Resource Version:    1906
  UID:                 81739b94-6dcf-4bf5-949c-9e4b53e380eb
Plugin:                exit-transformer
Events:
  Type     Reason                        Age                      From         Message
  ----     ------                        ----                     ----         -------
  Warning  KongConfigurationApplyFailed  2m26s (x102 over 7m29s)  kong-client  invalid config.functions.1: Error parsing function: /usr/local/share/lua/5.1/kong/tools/sandbox.lua:128: loading of untrusted Lua code disabled because 'untrusted_lua' config option is set to 'off'

IMO we should only change our behavior if the upstream invents another way to handle that. Also, in Events docs we advise looking for events in all namespaces - we might consider being more explicit and adding a note about the fact that events for cluster-scoped objects are created in the default namespace, WDYT?

As for the permissions, I don't think it is a real issue. KIC uses ClusterRole for events resource so we can create events in any namespace (and we rely on that to create events in arbitrary namespaces for objects that users have created).

[pmalek]

IMO we should only change our behavior if the upstream invents another way to handle that.

👍

we might consider being more explicit and adding a note about the fact that events for cluster-scoped objects are created in the default namespace, WDYT?

👍 Kong/docs.konghq.com#7203