Normalise plural naming in webhook operations; bring types to Python …

…3.7/3.8

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>

kopf/__init__.py

@@ -88,7 +88,6 @@
     WebhookClientConfigService,
     WebhookClientConfig,
     Operation,
-    Operations,
     UserInfo,
     Headers,
     SSLPeer,
@@ -203,7 +202,6 @@
     'AdmissionError',
     'WebhookClientConfigService',
     'WebhookClientConfig',
-    'Operations',
     'Operation',
     'UserInfo',
     'Headers',

kopf/_cogs/structs/reviews.py

@@ -11,7 +11,6 @@
 SSLPeer = Mapping[str, Any]
 
 Operation = Literal['CREATE', 'UPDATE', 'DELETE', 'CONNECT']
-Operations = list[Operation]
 
 
 class RequestKind(TypedDict):

kopf/_core/engines/admission.py

@@ -422,11 +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 if isinstance(handler.operation,list) 
-                        else [handler.operation]
-                    ),
+                    'operations': list(handler.operations or ['*']),
                     'scope': '*',  # doesn't matter since a specific resource is used.
                 }
                 for resource in resources

kopf/_core/intents/handlers.py

@@ -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
@@ -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[list[str] | str]
+    operations: Optional[Collection[str]]
     subresource: Optional[str]
     persistent: Optional[bool]
     side_effects: Optional[bool]
@@ -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):

kopf/_core/intents/registries.py

@@ -241,9 +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'] or 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):

kopf/on.py

@@ -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
@@ -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.Operations | reviews.Operation] = None,
+        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
@@ -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()
@@ -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)
@@ -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.Operations | reviews.Operation] = None,
+        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
@@ -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()
@@ -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)
@@ -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],

tests/admission/test_managed_webhooks.py

@@ -126,14 +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(operation=['CREATE','UPDATE']), 'operations', ['CREATE','UPDATE']),
-    (dict(operation=['CREATE','DELETE']), 'operations', ['CREATE','DELETE']),
-    (dict(operation=['UPDATE','DELETE']), 'operations', ['UPDATE','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):

tests/admission/test_serving_handler_selection.py

@@ -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)