nolar / kopf

A Python framework to write Kubernetes operators in just a few lines of code
https://kopf.readthedocs.io/
MIT License
2.13k stars 162 forks source link

Duplicate name error for multiple Admission handlers #946

Open psontag opened 2 years ago

psontag commented 2 years ago

Long story short

I want to define an admission handler that gets triggered by Pod creation or updates. The documentation says that I need to add two decorators to achieve this:

operation (str) will configure this handler/webhook to be called only for a specific operation. For multiple operations, add several decorators.

But running my example below results in an API error for duplicated webhook names.

Kopf version

1.35.5

Kubernetes version

1.22

Python version

3.10

Code

import kopf

@kopf.on.validate('pods', operation="CREATE")
@kopf.on.validate('pods', operation="UPDATE")
def pods(**_):
    print("hello")

@kopf.on.startup()
def configure(settings, **_):
    settings.admission.managed = "my.admission.hook"
    settings.admission.server = kopf.WebhookServer()

Logs

APIClientError: ('ValidatingWebhookConfiguration.admissionregistration.k8s.io "my.admission.hook'
is invalid: webhooks[1].name: Duplicate value: "pods.my.admission.hook", {'kind': 'Status', 'apiVersion': 'v1', 'metadata': {},
'status': 'Failure', 'message': 'ValidatingWebhookConfiguration.admissionregistration.k8s.io "my.admission.hook" is
invalid: webhooks[1].name: Duplicate value: "pods.my.admission.hook"', 'reason': 'Invalid', 'details': {'name':
my.admission.hook', 'group': 'admissionregistration.k8s.io', 'kind': 'ValidatingWebhookConfiguration',  'causes': [{'reason':
'FieldValueDuplicate', 'message': 'Duplicate value:   "pods.my.admission.hook"', 'field':'webhooks[1].name'}]}, 'code': 422})

Additional information

The webhook name is configured here (the name_suffix comes from settings.admission.managed) https://github.com/nolar/kopf/blob/7d4e7f868c5982c46b8e857c74a119b8c2f3fe2e/kopf/_core/engines/admission.py#L413

and is only based on the handler.id. The handler id is set here and does not seem to account for the operation value. https://github.com/nolar/kopf/blob/7d4e7f868c5982c46b8e857c74a119b8c2f3fe2e/kopf/on.py#L178

It looks like I can specify it myself via the id parameter of the decorator though. Is this the intended behaviour?

I would be happy to provide a PR that adds the operation to the handler id otherwise.

psontag commented 2 years ago

Hey @nolar would you accept a PR that includes the operation in the handler id?