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

Feature multiple operations webhook #1058

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kopf/_core/engines/admission.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ def build_webhooks(
[resource.plural] if handler.subresource is None else
[f'{resource.plural}/{handler.subresource}']
),
'operations': ['*'] if handler.operation is None else [handler.operation],
'operations': list(handler.operations or ['*']),
'scope': '*', # doesn't matter since a specific resource is used.
}
for resource in resources
Expand Down
17 changes: 15 additions & 2 deletions kopf/_core/intents/handlers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import dataclasses
from typing import Optional, cast
import warnings
from typing import Collection, Optional, cast

from kopf._cogs.structs import dicts, diffs, references
from kopf._core.actions import execution
Expand Down Expand Up @@ -40,7 +41,7 @@ def adjust_cause(self, cause: execution.CauseT) -> execution.CauseT:
class WebhookHandler(ResourceHandler):
fn: callbacks.WebhookFn # typing clarification
reason: causes.WebhookType
operation: Optional[str]
operations: Optional[Collection[str]]
subresource: Optional[str]
persistent: Optional[bool]
side_effects: Optional[bool]
Expand All @@ -49,6 +50,18 @@ class WebhookHandler(ResourceHandler):
def __str__(self) -> str:
return f"Webhook {self.id!r}"

@property
def operation(self) -> Optional[str]: # deprecated
warnings.warn("handler.operation is deprecated, use handler.operations", DeprecationWarning)
if not self.operations:
return None
elif len(self.operations) == 1:
return list(self.operations)[0]
else:
raise ValueError(
f"{len(self.operations)} operations in the handler. Use it as handler.operations."
)


@dataclasses.dataclass(frozen=True)
class IndexingHandler(ResourceHandler):
Expand Down
2 changes: 1 addition & 1 deletion kopf/_core/intents/registries.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def iter_handlers(
# For deletion, exclude all mutation handlers unless explicitly enabled.
non_mutating = handler.reason != causes.WebhookType.MUTATING
non_deletion = cause.operation != 'DELETE'
explicitly_for_deletion = handler.operation == 'DELETE'
explicitly_for_deletion = set(handler.operations or []) == {'DELETE'}
if non_mutating or non_deletion or explicitly_for_deletion:
# Filter by usual criteria: labels, annotations, fields, callbacks.
if match(handler=handler, cause=cause):
Expand Down
30 changes: 24 additions & 6 deletions kopf/on.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ def creation_handler(**kwargs):

This module is a part of the framework's public interface.
"""

import warnings
# TODO: add cluster=True support (different API methods)
from typing import Any, Callable, Optional, Union
from typing import Any, Callable, Collection, Optional, Union

from kopf._cogs.structs import dicts, references, reviews
from kopf._core.actions import execution
Expand Down Expand Up @@ -153,7 +153,8 @@ def validate( # lgtm[py/similar-function]
# Handler's behaviour specification:
id: Optional[str] = None,
param: Optional[Any] = None,
operation: Optional[reviews.Operation] = None, # -> .webhooks.*.rules.*.operations[0]
operation: Optional[reviews.Operation] = None, # deprecated -> .webhooks.*.rules.*.operations[0]
operations: Optional[Collection[reviews.Operation]] = None, # -> .webhooks.*.rules.*.operations
subresource: Optional[str] = None, # -> .webhooks.*.rules.*.resources[]
persistent: Optional[bool] = None,
side_effects: Optional[bool] = None, # -> .webhooks.*.sideEffects
Expand All @@ -171,6 +172,8 @@ def validate( # lgtm[py/similar-function]
def decorator( # lgtm[py/similar-function]
fn: callbacks.WebhookFn,
) -> callbacks.WebhookFn:
nonlocal operations
operations = _verify_operations(operation, operations)
_warn_conflicting_values(field, value)
_verify_filters(labels, annotations)
real_registry = registry if registry is not None else registries.get_default_registry()
Expand All @@ -186,7 +189,7 @@ def decorator( # lgtm[py/similar-function]
errors=None, timeout=None, retries=None, backoff=None, # TODO: add some meaning later
selector=selector, labels=labels, annotations=annotations, when=when,
field=real_field, value=value,
reason=causes.WebhookType.VALIDATING, operation=operation, subresource=subresource,
reason=causes.WebhookType.VALIDATING, operations=operations, subresource=subresource,
persistent=persistent, side_effects=side_effects, ignore_failures=ignore_failures,
)
real_registry._webhooks.append(handler)
Expand All @@ -210,7 +213,8 @@ def mutate( # lgtm[py/similar-function]
# Handler's behaviour specification:
id: Optional[str] = None,
param: Optional[Any] = None,
operation: Optional[reviews.Operation] = None, # -> .webhooks.*.rules.*.operations[0]
operation: Optional[reviews.Operation] = None, # deprecated -> .webhooks.*.rules.*.operations[0]
operations: Optional[Collection[reviews.Operation]] = None, # -> .webhooks.*.rules.*.operations
subresource: Optional[str] = None, # -> .webhooks.*.rules.*.resources[]
persistent: Optional[bool] = None,
side_effects: Optional[bool] = None, # -> .webhooks.*.sideEffects
Expand All @@ -228,6 +232,8 @@ def mutate( # lgtm[py/similar-function]
def decorator( # lgtm[py/similar-function]
fn: callbacks.WebhookFn,
) -> callbacks.WebhookFn:
nonlocal operations
operations = _verify_operations(operation, operations)
_warn_conflicting_values(field, value)
_verify_filters(labels, annotations)
real_registry = registry if registry is not None else registries.get_default_registry()
Expand All @@ -243,7 +249,7 @@ def decorator( # lgtm[py/similar-function]
errors=None, timeout=None, retries=None, backoff=None, # TODO: add some meaning later
selector=selector, labels=labels, annotations=annotations, when=when,
field=real_field, value=value,
reason=causes.WebhookType.MUTATING, operation=operation, subresource=subresource,
reason=causes.WebhookType.MUTATING, operations=operations, subresource=subresource,
persistent=persistent, side_effects=side_effects, ignore_failures=ignore_failures,
)
real_registry._webhooks.append(handler)
Expand Down Expand Up @@ -883,6 +889,18 @@ def create_single_task(task=task, **_):
return decorator(fn)


def _verify_operations(
operation: Optional[reviews.Operation] = None, # deprecated
operations: Optional[Collection[reviews.Operation]] = None,
) -> Optional[Collection[reviews.Operation]]:
if operation is not None:
warnings.warn("operation= is deprecated, use operations={...}.", DeprecationWarning)
operations = frozenset([] if operations is None else operations) | {operation}
if operations is not None and not operations:
raise ValueError(f"Operations should be either None or non-empty. Got empty {operations}.")
return operations


def _verify_filters(
labels: Optional[filters.MetaFilter],
annotations: Optional[filters.MetaFilter],
Expand Down
10 changes: 7 additions & 3 deletions tests/admission/test_managed_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,15 @@ def fn(**_):
assert webhooks[0]['admissionReviewVersions'] == ['v1', 'v1beta1']


# Mind the different supported collection types for operations, all converted to JSON lists.
@pytest.mark.parametrize('opts, key, val', [
(dict(), 'operations', ['*']),
(dict(operation='CREATE'), 'operations', ['CREATE']),
(dict(operation='UPDATE'), 'operations', ['UPDATE']),
(dict(operation='DELETE'), 'operations', ['DELETE']),
(dict(operations={'CREATE'}), 'operations', ['CREATE']),
(dict(operations={'UPDATE'}), 'operations', ['UPDATE']),
(dict(operations={'DELETE'}), 'operations', ['DELETE']),
(dict(operations=['CREATE','UPDATE']), 'operations', ['CREATE','UPDATE']),
(dict(operations=['CREATE','DELETE']), 'operations', ['CREATE','DELETE']),
(dict(operations=['UPDATE','DELETE']), 'operations', ['UPDATE','DELETE']),
])
@pytest.mark.parametrize('decorator', [kopf.on.validate, kopf.on.mutate])
def test_rule_options_are_mapped(registry, resource, decorator, opts, key, val):
Expand Down
2 changes: 1 addition & 1 deletion tests/admission/test_serving_handler_selection.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ async def test_mutating_handlers_are_selected_for_deletion_if_explicitly_marked(
def v_fn(**kwargs):
v_mock(**kwargs)

@kopf.on.mutate(*resource, operation='DELETE')
@kopf.on.mutate(*resource, operations=['DELETE'])
def m_fn(**kwargs):
m_mock(**kwargs)

Expand Down