Ensure the reason is never an empty string, but None in HTTP responses

kopf/_kits/webhooks.py

@@ -214,13 +214,13 @@ async def _serve(
             response = await fn(data, webhook=webhook, sslpeer=sslpeer, headers=headers)
             return aiohttp.web.json_response(response)
         except admission.AmbiguousResourceError as e:
-            raise aiohttp.web.HTTPConflict(reason=str(e))
+            raise aiohttp.web.HTTPConflict(reason=str(e) or None)
         except admission.UnknownResourceError as e:
-            raise aiohttp.web.HTTPNotFound(reason=str(e))
+            raise aiohttp.web.HTTPNotFound(reason=str(e) or None)
         except admission.WebhookError as e:
-            raise aiohttp.web.HTTPBadRequest(reason=str(e))
+            raise aiohttp.web.HTTPBadRequest(reason=str(e) or None)
         except json.JSONDecodeError as e:
-            raise aiohttp.web.HTTPBadRequest(reason=str(e))
+            raise aiohttp.web.HTTPBadRequest(reason=str(e) or None)
 
     @staticmethod
     def _allocate_free_port() -> int:

tests/apis/test_api_requests.py

@@ -45,7 +45,7 @@ async def test_raw_requests_are_not_parsed(
 async def test_server_errors_escalate(
         resp_mocker, aresponses, hostname, method, settings, logger):
 
-    mock = resp_mocker(return_value=aiohttp.web.json_response({}, status=666))
+    mock = resp_mocker(return_value=aiohttp.web.json_response({}, status=666, reason='oops'))
     aresponses.add(hostname, '/url', method, mock)
     with pytest.raises(APIError) as err:
         await request(method, '/url', settings=settings, logger=logger)

tests/apis/test_error_retries.py

@@ -39,7 +39,7 @@ async def test_client_errors_escalate_without_retries(
     caplog.set_level(0)
 
     # side_effect instead of return_value -- to generate a new response on every call, not reuse it.
-    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=status))
+    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=status, reason='oops'))
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
@@ -60,7 +60,7 @@ async def test_server_errors_escalate_with_retries(
     caplog.set_level(0)
 
     # side_effect instead of return_value -- to generate a new response on every call, not reuse it.
-    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=status))
+    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=status, reason='oops'))
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
@@ -123,8 +123,8 @@ async def test_retried_until_succeeded(
     caplog.set_level(0)
 
     mock = resp_mocker(side_effect=[
-        aiohttp.web.json_response({}, status=505),
-        aiohttp.web.json_response({}, status=505),
+        aiohttp.web.json_response({}, status=505, reason='oops'),
+        aiohttp.web.json_response({}, status=505, reason='oops'),
         aiohttp.web.json_response({}),
     ])
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
@@ -158,7 +158,7 @@ async def test_backoffs_as_lists(
     caplog.set_level(0)
 
     # side_effect instead of return_value -- to generate a new response on every call, not reuse it.
-    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500))
+    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500, reason='oops'))
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
@@ -178,7 +178,7 @@ async def test_backoffs_as_floats(
     caplog.set_level(0)
 
     # side_effect instead of return_value -- to generate a new response on every call, not reuse it.
-    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500))
+    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500, reason='oops'))
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
 
@@ -200,7 +200,7 @@ def __iter__(self):
             return iter([1, 2, 3])
 
     # side_effect instead of return_value -- to generate a new response on every call, not reuse it.
-    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500))
+    mock = resp_mocker(side_effect=lambda: aiohttp.web.json_response({}, status=500, reason='oops'))
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts
     aresponses.add(hostname, '/url', 'get', mock)  # repeat=N would copy the mock, lose all counts

tests/k8s/test_creating.py

@@ -58,7 +58,7 @@ async def test_raises_api_errors(
         resp_mocker, aresponses, hostname, settings, status, resource, namespace, logger,
         cluster_resource, namespaced_resource):
 
-    post_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    post_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     cluster_url = cluster_resource.get_url(namespace=None)
     namespaced_url = namespaced_resource.get_url(namespace='ns')
     aresponses.add(hostname, cluster_url, 'post', post_mock)

tests/k8s/test_errors.py

@@ -40,6 +40,7 @@ async def test_no_error_on_success(
 
     resp = aresponses.Response(
         status=status,
+        reason='oops',
         headers={'Content-Type': 'application/json'},
         text='{"kind": "Status", "code": "xxx", "message": "msg"}',
     )
@@ -67,6 +68,7 @@ async def test_error_with_payload(
 
     resp = aresponses.Response(
         status=status,
+        reason='oops',
         headers={'Content-Type': 'application/json'},
         text='{"kind": "Status", "code": 123, "message": "msg", "details": {"a": "b"}}',
     )
@@ -89,6 +91,7 @@ async def test_error_with_nonjson_payload(
 
     resp = aresponses.Response(
         status=status,
+        reason='oops',
         headers={'Content-Type': 'application/json'},
         text='unparsable json',
     )
@@ -109,6 +112,7 @@ async def test_error_with_parseable_nonk8s_payload(
 
     resp = aresponses.Response(
         status=status,
+        reason='oops',
         headers={'Content-Type': 'application/json'},
         text='{"kind": "NonStatus", "code": "xxx", "message": "msg"}',
     )

tests/k8s/test_events.py

@@ -76,7 +76,7 @@ async def test_no_events_for_events(
 async def test_api_errors_logged_but_suppressed(
         resp_mocker, aresponses, hostname, settings, logger, assert_logs):
 
-    post_mock = resp_mocker(return_value=aresponses.Response(status=555))
+    post_mock = resp_mocker(return_value=aresponses.Response(status=555, reason='oops'))
     aresponses.add(hostname, '/api/v1/namespaces/ns/events', 'post', post_mock)
 
     obj = {'apiVersion': 'group/version',
@@ -161,7 +161,7 @@ async def test_message_is_cut_to_max_length(
 async def test_headers_are_not_leaked(
         resp_mocker, aresponses, hostname, settings, logger, assert_logs, status):
 
-    post_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    post_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     aresponses.add(hostname, '/api/v1/namespaces/ns/events', 'post', post_mock)
 
     obj = {'apiVersion': 'group/version',

tests/k8s/test_list_objs.py

@@ -35,7 +35,7 @@ async def test_raises_direct_api_errors(
         resp_mocker, aresponses, hostname, settings, logger, status, resource, namespace,
         cluster_resource, namespaced_resource):
 
-    list_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    list_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     cluster_url = cluster_resource.get_url(namespace=None)
     namespaced_url = namespaced_resource.get_url(namespace='ns')
     aresponses.add(hostname, cluster_url, 'get', list_mock)

tests/k8s/test_patching.py

@@ -200,7 +200,7 @@ async def test_ignores_absent_objects(
         resp_mocker, aresponses, hostname, settings, status, resource, namespace, logger,
         cluster_resource, namespaced_resource):
 
-    patch_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    patch_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     cluster_url = cluster_resource.get_url(namespace=None, name='name1')
     namespaced_url = namespaced_resource.get_url(namespace='ns', name='name1')
     aresponses.add(hostname, cluster_url, 'patch', patch_mock)
@@ -225,7 +225,7 @@ async def test_raises_api_errors(
         resp_mocker, aresponses, hostname, settings, status, resource, namespace, logger,
         cluster_resource, namespaced_resource):
 
-    patch_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    patch_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     cluster_url = cluster_resource.get_url(namespace=None, name='name1')
     namespaced_url = namespaced_resource.get_url(namespace='ns', name='name1')
     aresponses.add(hostname, cluster_url, 'patch', patch_mock)

tests/k8s/test_scanning.py

@@ -240,7 +240,7 @@ async def test_http404_returns_no_resources_from_old_apis(
 
     core_mock = resp_mocker(return_value=aiohttp.web.json_response({'versions': ['v1']}))
     apis_mock = resp_mocker(return_value=aiohttp.web.json_response({'groups': []}))
-    status_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    status_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     aresponses.add(hostname, '/api', 'get', core_mock)
     aresponses.add(hostname, '/apis', 'get', apis_mock)
     aresponses.add(hostname, '/api/v1', 'get', status_mock)
@@ -259,7 +259,7 @@ async def test_http404_returns_no_resources_from_new_apis(
     apis_mock = resp_mocker(return_value=aiohttp.web.json_response({'groups': [
         {'name': 'g1', 'preferredVersion': {'version': ''}, 'versions': [{'version': 'g1v1'}]},
     ]}))
-    status_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    status_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     aresponses.add(hostname, '/api', 'get', core_mock)
     aresponses.add(hostname, '/apis', 'get', apis_mock)
     aresponses.add(hostname, '/apis/g1/g1v1', 'get', status_mock)
@@ -276,7 +276,7 @@ async def test_unknown_api_statuses_escalate_from_old_apis(
 
     core_mock = resp_mocker(return_value=aiohttp.web.json_response({'versions': ['v1']}))
     apis_mock = resp_mocker(return_value=aiohttp.web.json_response({'groups': []}))
-    status_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    status_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     aresponses.add(hostname, '/api', 'get', core_mock)
     aresponses.add(hostname, '/apis', 'get', apis_mock)
     aresponses.add(hostname, '/api/v1', 'get', status_mock)
@@ -296,7 +296,7 @@ async def test_unknown_api_statuses_escalate_from_new_apis(
     apis_mock = resp_mocker(return_value=aiohttp.web.json_response({'groups': [
         {'name': 'g1', 'preferredVersion': {'version': ''}, 'versions': [{'version': 'g1v1'}]},
     ]}))
-    status_mock = resp_mocker(return_value=aresponses.Response(status=status))
+    status_mock = resp_mocker(return_value=aresponses.Response(status=status, reason='oops'))
     aresponses.add(hostname, '/api', 'get', core_mock)
     aresponses.add(hostname, '/apis', 'get', apis_mock)
     aresponses.add(hostname, '/apis/g1/g1v1', 'get', status_mock)

tests/k8s/test_watching_infinitely.py

@@ -39,7 +39,7 @@ async def test_exception_escalates(
 
 async def test_infinite_watch_never_exits_normally(
         settings, resource, stream, namespace, aresponses):
-    error = aresponses.Response(status=555)
+    error = aresponses.Response(status=555, reason='oops')
     stream.feed(
         STREAM_WITH_ERROR_410GONE,  # watching restarted
         STREAM_WITH_UNKNOWN_EVENT,  # event ignored

tests/observation/test_processing_of_resources.py

@@ -77,7 +77,7 @@ def group1_empty_mock(resp_mocker, aresponses, hostname, apis_mock):
 
 @pytest.fixture()
 def group1_404mock(resp_mocker, aresponses, hostname, apis_mock):
-    mock = resp_mocker(return_value=aresponses.Response(status=404))
+    mock = resp_mocker(return_value=aresponses.Response(status=404, reason='oops'))
     aresponses.add(hostname, '/apis/group1/version1', 'get', mock)
     return mock
 

tests/peering/test_peer_patching.py

@@ -116,7 +116,7 @@ async def test_logs_are_logged_when_absent(
     caplog.set_level(0)
     settings.peering.stealth = stealth
     settings.peering.name = 'name0'
-    patch_mock = resp_mocker(return_value=aresponses.Response(status=404))
+    patch_mock = resp_mocker(return_value=aresponses.Response(status=404, reason='oops'))
     url = peering_resource.get_url(name='name0', namespace=peering_namespace)
     aresponses.add(hostname, url, 'patch', patch_mock)
 

tests/reactor/test_patching_inconsistencies.py

@@ -137,7 +137,7 @@ async def test_patching_with_disappearance(
 
     patch = {'spec': {'x': 'y'}, 'status': {'s': 't'}}  # irrelevant
     url = resource.get_url(namespace=namespace, name='name1')
-    patch_mock = resp_mocker(return_value=aresponses.Response(status=404))
+    patch_mock = resp_mocker(return_value=aresponses.Response(status=404, reason='oops'))
     aresponses.add(hostname, url, 'patch', patch_mock)
 
     body = Body({'metadata': {'namespace': namespace, 'name': 'name1'}})