Validate kuma.io-scoped annotations

jpeach commented 2 years ago

Summary

We should add Kubernetes validation for annotations with a leading kuma.io. For any annotation that begins with the *.kuma.io/ prefix, validate that it's a known and supported Kuma annotation. This helps users catch typographic errors, and preserves the project's ability to use new annotations in this scope without the risk of a collisions.

github-actions[bot] commented 2 years ago

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

PrayagS commented 2 years ago

I'm working on this issue and I managed to write a validation hook.

Next up I have to validate the list of annotations against the global list of valid annotations. I looked around in pkg/plugins/runtime/k8s/metadata/annotations.go. Are there any helper methods to perform a reverse search on the list of valid annotations?

Also, are we looking to validate values of these annotations as well?

lahabana commented 2 years ago

Are there any helper methods to perform a reverse search on the list of valid annotations?

No you'll probably want to introduce a map for this

Also, are we looking to validate values of these annotations as well?

It would be good to do both labels and annotations (the doc should be a good way to check what works on labels vs annotations)

PrayagS commented 2 years ago

No you'll probably want to introduce a map for this.

Thanks for confirming.

both labels and annotations

Shouldn't there be a separate issue for labels? I was asking about values actually. Something like this: Kong/kubernetes-ingress-controller#1340. To spot wrong annotation values passed by the user and notifying them because the end result won't be the same as they expected.

michaelbeaumont commented 2 years ago

In my opinion, it makes sense to cover both labels and annotations for this issue.

I think validating values can be a separate discussion/issue, at least a separate PR.

PrayagS commented 2 years ago

Just pushed a PR for this. One thing I realized that I added this just for pods 😅.

Which other resource kinds should have this? And in that case, I'm assuming I'll also need to change the configuration for the webhook. As for now, it would be great if you folks can go through the current code and point out any changes if required.

github-actions[bot] commented 2 years ago

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

github-actions[bot] commented 2 years ago

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

github-actions[bot] commented 2 years ago

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

github-actions[bot] commented 1 year ago

This issue was inactive for 30 days it will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant please comment on it promptly or attend the next triage meeting.

github-actions[bot] commented 1 year ago