Events for cluster scoped resource like `KongClusterPlugin`s 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

https://github.com/kubernetes/client-go/issues/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 https://github.com/kubernetes/client-go/issues/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?

👍 https://github.com/Kong/docs.konghq.com/pull/7203

[lahabana]

This is still impossible in client-go so punting to next release