diff --git a/kopf/clients/fetching.py b/kopf/clients/fetching.py index 412715cc..4648f867 100644 --- a/kopf/clients/fetching.py +++ b/kopf/clients/fetching.py @@ -17,8 +17,8 @@ class _UNSET(enum.Enum): async def read_obj( *, resource: resources.Resource, - namespace: Optional[str] = None, - name: Optional[str] = None, + namespace: Optional[str], + name: Optional[str], default: Union[_T, _UNSET] = _UNSET.token, context: Optional[auth.APIContext] = None, # injected by the decorator ) -> Union[bodies.RawBody, _T]: diff --git a/kopf/clients/patching.py b/kopf/clients/patching.py index 42297b08..5c23a96f 100644 --- a/kopf/clients/patching.py +++ b/kopf/clients/patching.py @@ -1,4 +1,4 @@ -from typing import Optional, cast +from typing import Optional from kopf.clients import auth, discovery, errors from kopf.structs import bodies, patches, resources @@ -8,18 +8,14 @@ async def patch_obj( *, resource: resources.Resource, + namespace: Optional[str], + name: Optional[str], patch: patches.Patch, - namespace: Optional[str] = None, - name: Optional[str] = None, - body: Optional[bodies.Body] = None, context: Optional[auth.APIContext] = None, # injected by the decorator ) -> Optional[bodies.RawBody]: """ Patch a resource of specific kind. - Either the namespace+name should be specified, or the body, - which is used only to get namespace+name identifiers. - Unlike the object listing, the namespaced call is always used for the namespaced resources, even if the operator serves the whole cluster (i.e. is not namespace-restricted). @@ -38,20 +34,9 @@ async def patch_obj( if context is None: raise RuntimeError("API instance is not injected by the decorator.") - if body is not None and (name is not None or namespace is not None): - raise TypeError("Either body, or name+namespace can be specified. Got both.") - - namespace = body.get('metadata', {}).get('namespace') if body is not None else namespace - name = body.get('metadata', {}).get('name') if body is not None else name - is_namespaced = await discovery.is_namespaced(resource=resource, context=context) namespace = namespace if is_namespaced else None - if body is None: - body = cast(bodies.Body, {'metadata': {'name': name}}) - if namespace is not None: - body['metadata']['namespace'] = namespace - as_subresource = await discovery.is_status_subresource(resource=resource, context=context) body_patch = dict(patch) # shallow: for mutation of the top-level keys below. status_patch = body_patch.pop('status', None) if as_subresource else None diff --git a/kopf/clients/watching.py b/kopf/clients/watching.py index af6792a9..74b54284 100644 --- a/kopf/clients/watching.py +++ b/kopf/clients/watching.py @@ -169,7 +169,7 @@ async def watch_objs( *, settings: configuration.OperatorSettings, resource: resources.Resource, - namespace: Optional[str] = None, + namespace: Optional[str], timeout: Optional[float] = None, since: Optional[str] = None, context: Optional[auth.APIContext] = None, # injected by the decorator diff --git a/kopf/reactor/effects.py b/kopf/reactor/effects.py index 0a3a1f8a..a5afc397 100644 --- a/kopf/reactor/effects.py +++ b/kopf/reactor/effects.py @@ -117,7 +117,12 @@ async def patch_and_check( """ if patch: logger.debug(f"Patching with: {patch!r}") - resulting_body = await patching.patch_obj(resource=resource, patch=patch, body=body) + resulting_body = await patching.patch_obj( + resource=resource, + namespace=body.metadata.namespace, + name=body.metadata.name, + patch=patch, + ) inconsistencies = diffs.diff(patch, resulting_body, scope=diffs.DiffScope.LEFT) inconsistencies = diffs.Diff( diffs.DiffItem(op, field, old, new) diff --git a/kopf/structs/callbacks.py b/kopf/structs/callbacks.py index 5d470c3f..bbc61f65 100644 --- a/kopf/structs/callbacks.py +++ b/kopf/structs/callbacks.py @@ -44,8 +44,8 @@ def __call__( # lgtm[py/similar-function] meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], patch: patches.Patch, logger: Union[logging.Logger, logging.LoggerAdapter], @@ -62,8 +62,8 @@ def __call__( # lgtm[py/similar-function] meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], patch: patches.Patch, logger: Union[logging.Logger, logging.LoggerAdapter], @@ -82,8 +82,8 @@ def __call__( # lgtm[py/similar-function] # << different mode meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], logger: Union[logging.Logger, logging.LoggerAdapter], stopped: primitives.SyncDaemonStopperChecker, # << different type @@ -99,8 +99,8 @@ async def __call__( # lgtm[py/similar-function] # << different mode meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], logger: Union[logging.Logger, logging.LoggerAdapter], stopped: primitives.AsyncDaemonStopperChecker, # << different type @@ -119,8 +119,8 @@ def __call__( # lgtm[py/similar-function] meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], logger: Union[logging.Logger, logging.LoggerAdapter], **kwargs: Any, @@ -140,8 +140,8 @@ def __call__( # lgtm[py/similar-function] meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], patch: patches.Patch, logger: Union[logging.Logger, logging.LoggerAdapter], @@ -161,8 +161,8 @@ def __call__( # lgtm[py/similar-function] meta: bodies.Meta, spec: bodies.Spec, status: bodies.Status, - uid: str, - name: str, + uid: Optional[str], + name: Optional[str], namespace: Optional[str], patch: patches.Patch, logger: Union[logging.Logger, logging.LoggerAdapter], diff --git a/tests/k8s/test_patching.py b/tests/k8s/test_patching.py index 36650f63..7bea2b7e 100644 --- a/tests/k8s/test_patching.py +++ b/tests/k8s/test_patching.py @@ -3,12 +3,11 @@ from kopf.clients.errors import APIError from kopf.clients.patching import patch_obj -from kopf.structs.bodies import Body from kopf.structs.patches import Patch @pytest.mark.resource_clustered # see `resp_mocker` -async def test_by_name_clustered( +async def test_clustered( resp_mocker, aresponses, hostname, resource): patch_mock = resp_mocker(return_value=aiohttp.web.json_response({})) @@ -24,7 +23,7 @@ async def test_by_name_clustered( assert data == {'x': 'y'} -async def test_by_name_namespaced( +async def test_namespaced( resp_mocker, aresponses, hostname, resource): patch_mock = resp_mocker(return_value=aiohttp.web.json_response({})) @@ -40,46 +39,10 @@ async def test_by_name_namespaced( assert data == {'x': 'y'} -@pytest.mark.resource_clustered # see `resp_mocker` -async def test_by_body_clustered( - resp_mocker, aresponses, hostname, resource): - - patch_mock = resp_mocker(return_value=aiohttp.web.json_response({})) - aresponses.add(hostname, resource.get_url(namespace=None, name='name1'), 'patch', patch_mock) - - body = Body({'metadata': {'name': 'name1'}}) - patch = Patch({'x': 'y'}) - await patch_obj(resource=resource, body=body, patch=patch) - - assert patch_mock.called - assert patch_mock.call_count == 1 - - data = patch_mock.call_args_list[0][0][0].data # [callidx][args/kwargs][argidx] - assert data == {'x': 'y'} - - -async def test_by_body_namespaced( - resp_mocker, aresponses, hostname, resource): - - patch_mock = resp_mocker(return_value=aiohttp.web.json_response({})) - aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) - - body = Body({'metadata': {'namespace': 'ns1', 'name': 'name1'}}) - patch = Patch({'x': 'y'}) - await patch_obj(resource=resource, body=body, patch=patch) - - assert patch_mock.called - assert patch_mock.call_count == 1 - - data = patch_mock.call_args_list[0][0][0].data # [callidx][args/kwargs][argidx] - assert data == {'x': 'y'} - - async def test_status_as_subresource_with_combined_payload( resp_mocker, aresponses, hostname, resource, version_api_with_substatus): # Simulate Kopf's initial state and intention. - body = Body({'metadata': {'namespace': 'ns1', 'name': 'name1'}}) patch = Patch({'spec': {'x': 'y'}, 'status': {'s': 't'}}) # Simulate K8s API's behaviour. Assume something extra is added remotely. @@ -95,7 +58,7 @@ async def test_status_as_subresource_with_combined_payload( aresponses.add(hostname, object_url, 'patch', object_patch_mock) aresponses.add(hostname, status_url, 'patch', status_patch_mock) - reconstructed = await patch_obj(resource=resource, body=body, patch=patch) + reconstructed = await patch_obj(resource=resource, namespace='ns1', name='name1', patch=patch) assert object_patch_mock.called assert object_patch_mock.call_count == 1 @@ -116,7 +79,6 @@ async def test_status_as_subresource_with_object_fields_only( resp_mocker, aresponses, hostname, resource, version_api_with_substatus): # Simulate Kopf's initial state and intention. - body = Body({'metadata': {'namespace': 'ns1', 'name': 'name1'}}) patch = Patch({'spec': {'x': 'y'}}) # Simulate K8s API's behaviour. Assume something extra is added remotely. @@ -132,7 +94,7 @@ async def test_status_as_subresource_with_object_fields_only( aresponses.add(hostname, object_url, 'patch', object_patch_mock) aresponses.add(hostname, status_url, 'patch', status_patch_mock) - reconstructed = await patch_obj(resource=resource, body=body, patch=patch) + reconstructed = await patch_obj(resource=resource, namespace='ns1', name='name1', patch=patch) assert object_patch_mock.called assert object_patch_mock.call_count == 1 @@ -150,7 +112,6 @@ async def test_status_as_subresource_with_status_fields_only( resp_mocker, aresponses, hostname, resource, version_api_with_substatus): # Simulate Kopf's initial state and intention. - body = Body({'metadata': {'namespace': 'ns1', 'name': 'name1'}}) patch = Patch({'status': {'s': 't'}}) # Simulate K8s API's behaviour. Assume something extra is added remotely. @@ -166,7 +127,7 @@ async def test_status_as_subresource_with_status_fields_only( aresponses.add(hostname, object_url, 'patch', object_patch_mock) aresponses.add(hostname, status_url, 'patch', status_patch_mock) - reconstructed = await patch_obj(resource=resource, body=body, patch=patch) + reconstructed = await patch_obj(resource=resource, namespace='ns1', name='name1', patch=patch) assert not object_patch_mock.called assert status_patch_mock.called @@ -182,7 +143,6 @@ async def test_status_as_body_field_with_combined_payload( resp_mocker, aresponses, hostname, resource): # Simulate Kopf's initial state and intention. - body = Body({'metadata': {'namespace': 'ns1', 'name': 'name1'}}) patch = Patch({'spec': {'x': 'y'}, 'status': {'s': 't'}}) # Simulate K8s API's behaviour. Assume something extra is added remotely. @@ -198,7 +158,7 @@ async def test_status_as_body_field_with_combined_payload( aresponses.add(hostname, object_url, 'patch', object_patch_mock) aresponses.add(hostname, status_url, 'patch', status_patch_mock) - reconstructed = await patch_obj(resource=resource, body=body, patch=patch) + reconstructed = await patch_obj(resource=resource, namespace='ns1', name='name1', patch=patch) assert object_patch_mock.called assert object_patch_mock.call_count == 1 @@ -212,51 +172,6 @@ async def test_status_as_body_field_with_combined_payload( 'status': '...'} -async def test_raises_when_body_conflicts_with_namespace( - resp_mocker, aresponses, hostname, resource): - - patch_mock = resp_mocker(return_value=aiohttp.web.json_response()) - aresponses.add(hostname, resource.get_url(namespace=None, name='name1'), 'patch', patch_mock) - aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) - - patch = {'x': 'y'} - body = {'metadata': {'namespace': 'ns1', 'name': 'name1'}} - with pytest.raises(TypeError): - await patch_obj(resource=resource, body=body, namespace='ns1', patch=patch) - - assert not patch_mock.called - - -async def test_raises_when_body_conflicts_with_name( - resp_mocker, aresponses, hostname, resource): - - patch_mock = resp_mocker(return_value=aiohttp.web.json_response()) - aresponses.add(hostname, resource.get_url(namespace=None, name='name1'), 'patch', patch_mock) - aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) - - patch = {'x': 'y'} - body = {'metadata': {'namespace': 'ns1', 'name': 'name1'}} - with pytest.raises(TypeError): - await patch_obj(resource=resource, body=body, name='name1', patch=patch) - - assert not patch_mock.called - - -async def test_raises_when_body_conflicts_with_ids( - resp_mocker, aresponses, hostname, resource): - - patch_mock = resp_mocker(return_value=aiohttp.web.json_response()) - aresponses.add(hostname, resource.get_url(namespace=None, name='name1'), 'patch', patch_mock) - aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) - - patch = {'x': 'y'} - body = {'metadata': {'namespace': 'ns1', 'name': 'name1'}} - with pytest.raises(TypeError): - await patch_obj(resource=resource, body=body, namespace='ns1', name='name1', patch=patch) - - assert not patch_mock.called - - @pytest.mark.parametrize('namespace', [None, 'ns1'], ids=['without-namespace', 'with-namespace']) @pytest.mark.parametrize('status', [404]) async def test_ignores_absent_objects( @@ -267,10 +182,9 @@ async def test_ignores_absent_objects( aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) patch = {'x': 'y'} - body = {'metadata': {'namespace': namespace, 'name': 'name1'}} - reconstructed = await patch_obj(resource=resource, body=body, patch=patch) + result = await patch_obj(resource=resource, namespace=namespace, name='name1', patch=patch) - assert reconstructed is None + assert result is None @pytest.mark.parametrize('namespace', [None, 'ns1'], ids=['without-namespace', 'with-namespace']) @@ -283,7 +197,6 @@ async def test_raises_api_errors( aresponses.add(hostname, resource.get_url(namespace='ns1', name='name1'), 'patch', patch_mock) patch = {'x': 'y'} - body = {'metadata': {'namespace': namespace, 'name': 'name1'}} with pytest.raises(APIError) as e: - await patch_obj(resource=resource, body=body, patch=patch) + await patch_obj(resource=resource, namespace=namespace, name='name1', patch=patch) assert e.value.status == status