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

Filter out invalid name information to generate valid hook name #714

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link
Contributor

@stevefan1999-personal stevefan1999-personal commented Feb 10, 2024

Some resources like Service, may not have a group:

apiVersion: v1
kind: Service

So it become some value like this originally: mutate.service..v1

But the regex for the webhook name in k8s demands this syntax:

      k8s.Autorest.HttpOperationException: Operation returned an invalid status code 'UnprocessableEntity', response body {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"MutatingWebhookConfiguration.admissionregistration.k8s.io \"dev-mutators\" is invalid: webhooks[1].name: Invalid value: \"mutate.service..v1\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')","reason":"Invalid","details":{"name":"dev-mutators","group":"admissionregistration.k8s.io","kind":"MutatingWebhookConfiguration","causes":[{"reason":"FieldValueInvalid","message":"Invalid value: \"mutate.service..v1\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')","field":"webhooks[1].name"}]},"code":422}

Fixes #713

mutatorConfig.Webhooks.Add(new V1MutatingWebhook
{
Name = $"mutate.{hook.Metadata.SingularName}.{hook.Metadata.Group}.{hook.Metadata.Version}",
Name = $"mutate.{hookName}",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, if no "group" is present, Kubernetes internal stuff defaults to "Core".

Shouldn't we default to "Core" if there is no group present? Either in the metadata class or in this logic

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame to let this fix languish, I got a way into building a validating webhook before finding it doesn't work with pods. Is there a workaround?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. Never had the issue and never got a serious answer 🤷🏻

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addition: If you have a good take on it, or an opinion, feel free to share.
I thought using "default" for the group is a promising idea since it is the default behavior of Kubernetes.

@buehler buehler force-pushed the main branch 2 times, most recently from 80132ea to 4283cf6 Compare February 13, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: Cannot create mutation webhook with built-in resources
4 participants