Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot generate ResourceFailure for cluster scoped resources #5387

Closed
1 task done
randmonkey opened this issue Dec 29, 2023 · 2 comments · Fixed by #5391
Closed
1 task done

Cannot generate ResourceFailure for cluster scoped resources #5387

randmonkey opened this issue Dec 29, 2023 · 2 comments · Fixed by #5391
Labels
area/observability bug Something isn't working

Comments

@randmonkey
Copy link
Contributor

randmonkey commented Dec 29, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

failures.NewResourceFailure checks for non-empty of namespace of causing objects. If one of causing objects has no namespace, it will fail to generate a resource failure. Thus if a translation failure happens on cluster scoped resources (like KongClusterPlugin), KIC cannot create an event for it.

Expected Behavior

If a cluster-scope resource causes a translation failure, we could see the k8s Event related to the object.

Steps To Reproduce

No response

Kong Ingress Controller version

No response

Kubernetes version

No response

Anything else?

Currently KongClusterPlugin and KongVault might be affected. This may not have real impact because invalid config that will cause translation failures may already be rejected by CEL or admission webhook. But if webhooks are accidentally disabled, the bug may cause real affection.

@rainest
Copy link
Contributor

rainest commented Jan 6, 2024

We kinda want to have some way to distinguish these from resources that bugged out and failed to populate part or all of their parent info.

For namespaces at least, there are some characters that can never show up in an actual namespace but are valid Kong tags. For example:

$ kubectl create namespace "\!aaa"         
The Namespace "!aaa" is invalid: 
* metadata.name: Invalid value: "!aaa": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')
* metadata.labels: Invalid value: "!aaa": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')

So maybe we could use !global?

@randmonkey
Copy link
Contributor Author

randmonkey commented Jan 8, 2024

We kinda want to have some way to distinguish these from resources that bugged out and failed to populate part or all of their parent info.

For namespaces at least, there are some characters that can never show up in an actual namespace but are valid Kong tags. For example:

$ kubectl create namespace "\!aaa"         
The Namespace "!aaa" is invalid: 
* metadata.name: Invalid value: "!aaa": a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')
* metadata.labels: Invalid value: "!aaa": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')

So maybe we could use !global?

I tried with KongClusterPlugin and we could attach events to it, then the event appears in the "" (empty) namespace:

kubectl get events | grep "KongConfigurationTranslationFailed"
76s         Warning   KongConfigurationTranslationFailed   kongclusterplugin/p1                      test translation failure on cluster resource

So I think it is OK to create events with empty namespace attached to cluster scoped resources.

But I am not sure if we can create an event with a non-exist "involved object", so I think using a non-exist (or even invalid) value in involvedObject of events is not a feasible way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/observability bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants