Merge pull request #2213 from coreos-inc/ISSUE-2026-204-response

fix(endpoints/api): return empty 204 resp

endpoints/api/manifest.py

@@ -154,5 +154,5 @@ class ManageRepositoryManifestLabel(RepositoryParamResource):
     }
 
     log_action('manifest_label_delete', namespace, metadata, repo=tag_manifest.tag.repository)
-    return 'Deleted', 204
+    return '', 204
 

endpoints/api/organization.py

@@ -215,7 +215,7 @@ class Organization(ApiResource):
         raise NotFound()
 
       model.user.delete_user(org, all_queues)
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 
@@ -380,7 +380,7 @@ class OrganizationMember(ApiResource):
 
       # Remove the user from the organization.
       model.organization.remove_organization_member(org, user)
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 
@@ -616,7 +616,7 @@ class OrganizationApplicationResource(ApiResource):
       log_action('delete_application', orgname,
                  {'application_name': application.name, 'client_id': client_id})
 
-      return 'Deleted', 204
+      return '', 204
     raise Unauthorized()
 
 

endpoints/api/permission.py

@@ -212,7 +212,7 @@ class RepositoryUserPermission(RepositoryParamResource):
                {'username': username, 'repo': repository},
                repo=model.repository.get_repository(namespace, repository))
 
-    return 'Deleted', 204
+    return '', 204
 
 
 @resource('/v1/repository/<apirepopath:repository>/permissions/team/<teamname>')
@@ -278,4 +278,4 @@ class RepositoryTeamPermission(RepositoryParamResource):
                {'team': teamname, 'repo': repository},
                repo=model.repository.get_repository(namespace, repository))
 
-    return 'Deleted', 204
+    return '', 204

endpoints/api/prototype.py

@@ -229,7 +229,7 @@ class PermissionPrototype(ApiResource):
 
       log_prototype_action('delete_prototype_permission', orgname, prototype)
 
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 

endpoints/api/repository.py

@@ -363,7 +363,7 @@ class Repository(RepositoryParamResource):
 
     log_action('delete_repo', namespace,
                {'repo': repository, 'namespace': namespace})
-    return 'Deleted', 204
+    return '', 204
 
 
 @resource('/v1/repository/<apirepopath:repository>/changevisibility')

endpoints/api/repotoken.py

@@ -134,4 +134,4 @@ class RepositoryToken(RepositoryParamResource):
                 'code': code},
                repo=model.repository.get_repository(namespace, repository))
 
-    return 'Deleted', 204
+    return '', 204

endpoints/api/robot.py

@@ -115,7 +115,7 @@ class UserRobot(ApiResource):
     parent = get_authenticated_user()
     model.user.delete_robot(format_robot_username(parent.username, robot_shortname))
     log_action('delete_robot', parent.username, {'robot': robot_shortname})
-    return 'Deleted', 204
+    return '', 204
 
 
 @resource('/v1/organization/<orgname>/robots')
@@ -178,7 +178,7 @@ class OrgRobot(ApiResource):
     if permission.can():
       model.user.delete_robot(format_robot_username(orgname, robot_shortname))
       log_action('delete_robot', orgname, {'robot': robot_shortname})
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 

endpoints/api/superuser.py

@@ -369,7 +369,7 @@ class SuperUserManagement(ApiResource):
         abort(403)
 
       model.user.delete_user(user, all_queues, force=True)
-      return 'Deleted', 204
+      return '', 204
 
     abort(403)
 
@@ -503,7 +503,7 @@ class SuperUserOrganizationManagement(ApiResource):
       org = model.organization.get_organization(name)
 
       model.user.delete_user(org, all_queues)
-      return 'Deleted', 204
+      return '', 204
 
     abort(403)
 

endpoints/api/tag.py

@@ -122,7 +122,7 @@ class RepositoryTag(RepositoryParamResource):
                {'username': username, 'repo': repository, 'tag': tag},
                repo=model.repository.get_repository(namespace, repository))
 
-    return 'Deleted', 204
+    return '', 204
 
 
 @resource('/v1/repository/<apirepopath:repository>/tag/<tag>/images')

endpoints/api/team.py

@@ -175,7 +175,7 @@ class OrganizationTeam(ApiResource):
     if permission.can():
       model.team.remove_team(orgname, teamname, get_authenticated_user().username)
       log_action('org_delete_team', orgname, {'team': teamname})
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 
@@ -293,11 +293,11 @@ class TeamMember(ApiResource):
           'team': teamname,
           'member': membername
         })
-        return 'Deleted', 204
+        return '', 204
 
       model.team.remove_user_from_team(orgname, teamname, membername, invoking_user)
       log_action('org_remove_team_member', orgname, {'member': membername, 'team': teamname})
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 
@@ -353,7 +353,7 @@ class InviteTeamMember(ApiResource):
         'team': teamname,
         'member': email
       })
-      return 'Deleted', 204
+      return '', 204
 
     raise Unauthorized()
 
@@ -418,4 +418,4 @@ class TeamMemberInvite(ApiResource):
       'inviter': inviter.username
     })
 
-    return 'Deleted', 204
+    return '', 204

endpoints/api/user.py

@@ -421,7 +421,7 @@ class User(ApiResource):
       abort(404)
 
     model.user.delete_user(get_authenticated_user(), all_queues)
-    return 'Deleted', 204
+    return '', 204
 
 
 @resource('/v1/user/private')
@@ -873,7 +873,7 @@ class UserAuthorization(ApiResource):
       raise NotFound()
 
     access_token.delete_instance(recursive=True, delete_nullable=True)
-    return 'Deleted', 204
+    return '', 204
 
 @resource('/v1/user/starred')
 class StarredRepositoryList(ApiResource):
@@ -956,7 +956,7 @@ class StarredRepository(RepositoryParamResource):
 
     if repo:
       model.repository.unstar_repository(user, repo)
-      return 'Deleted', 204
+      return '', 204
 
 
 @resource('/v1/users/<username>')

test/test_api_usage.py

@@ -198,6 +198,12 @@ class ApiTestCase(unittest.TestCase):
     self.assertEquals(rv.status_code, expected_code)
     return rv.data
 
+  def deleteEmptyResponse(self, resource_name, params={}, expected_code=204):
+    rv = self.app.delete(self.url_for(resource_name, params))
+    self.assertEquals(rv.status_code, expected_code)
+    self.assertEquals(rv.data, '') # ensure response body empty
+    return
+
   def postJsonResponse(self, resource_name, params={}, data={},
                        expected_code=200):
     rv = self.app.post(self.url_for(resource_name, params),
@@ -351,7 +357,7 @@ class TestUserStarredRepositoryList(ApiTestCase):
     assert json['namespace'] == 'public'
     assert json['repository'] == 'publicrepo'
 
-    self.deleteResponse(StarredRepository, params=dict(repository='public/publicrepo'),
+    self.deleteEmptyResponse(StarredRepository, params=dict(repository='public/publicrepo'),
                         expected_code=204)
 
     json = self.getJsonResponse(StarredRepositoryList)
@@ -804,9 +810,9 @@ class TestDeleteNamespace(ApiTestCase):
 
     # Delete the two orgs, checking in between.
     with check_transitive_deletes():
-      self.deleteResponse(Organization, params=dict(orgname=ORGANIZATION), expected_code=204)
+      self.deleteEmptyResponse(Organization, params=dict(orgname=ORGANIZATION), expected_code=204)
       self.deleteResponse(User, expected_code=400) # Should still fail.
-      self.deleteResponse(Organization, params=dict(orgname='library'), expected_code=204)
+      self.deleteEmptyResponse(Organization, params=dict(orgname='library'), expected_code=204)
 
     # Add some queue items for the user.
     notification_queue.put([ADMIN_ACCESS_USER, 'somerepo', 'somename'], '{}')
@@ -814,7 +820,7 @@ class TestDeleteNamespace(ApiTestCase):
 
     # Now delete the user.
     with check_transitive_deletes():
-      self.deleteResponse(User, expected_code=204)
+      self.deleteEmptyResponse(User, expected_code=204)
 
     # Ensure the queue items are gone.
     self.assertIsNone(notification_queue.get())
@@ -828,12 +834,12 @@ class TestDeleteNamespace(ApiTestCase):
     model.user.attach_federated_login(user, 'github', 'something', {})
 
     with check_transitive_deletes():
-      self.deleteResponse(User, expected_code=204)
+      self.deleteEmptyResponse(User, expected_code=204)
 
   def test_delete_prompted_user(self):
     self.login('randomuser')
     with check_transitive_deletes():
-      self.deleteResponse(User, expected_code=204)
+      self.deleteEmptyResponse(User, expected_code=204)
 
 
 class TestSignin(ApiTestCase):
@@ -1215,8 +1221,8 @@ class TestDeleteOrganizationPrototypes(ApiTestCase):
     pid = ids[0]
 
     # Delete a prototype.
-    self.deleteResponse(PermissionPrototype,
+    self.deleteEmptyResponse(PermissionPrototype,
-                        params=dict(orgname=ORGANIZATION, prototypeid=pid))
+                            params=dict(orgname=ORGANIZATION, prototypeid=pid))
 
     # Verify the prototype no longer exists.
     json = self.getJsonResponse(PermissionPrototypeList,
@@ -1283,7 +1289,7 @@ class TestRemoveOrganizationMember(ApiTestCase):
     assert ADMIN_ACCESS_USER in membernames
     assert READ_ACCESS_USER in membernames
 
-    self.deleteResponse(OrganizationMember,
+    self.deleteEmptyResponse(OrganizationMember,
                         params=dict(orgname=ORGANIZATION, membername=READ_ACCESS_USER))
 
     json = self.getJsonResponse(OrganizationMemberList,
@@ -1313,7 +1319,7 @@ class TestRemoveOrganizationMember(ApiTestCase):
     assert READ_ACCESS_USER in org_perms
 
     # Remove the user from the org.
-    self.deleteResponse(OrganizationMember,
+    self.deleteEmptyResponse(OrganizationMember,
                         params=dict(orgname=ORGANIZATION, membername=READ_ACCESS_USER))
 
     # Verify that the user's permission on the org repo is gone, but it is still
@@ -1394,8 +1400,8 @@ class TestDeleteOrganizationTeam(ApiTestCase):
   def test_deleteteam(self):
     self.login(ADMIN_ACCESS_USER)
 
-    self.deleteResponse(OrganizationTeam,
+    self.deleteEmptyResponse(OrganizationTeam,
-                        params=dict(orgname=ORGANIZATION, teamname='readers'))
+                          params=dict(orgname=ORGANIZATION, teamname='readers'))
 
     # Make sure the team was deleted
     json = self.getJsonResponse(Organization,
@@ -1593,8 +1599,8 @@ class TestDeclineTeamMemberInvite(ApiTestCase):
     invites = list(model.team.lookup_team_invites(user))
     self.assertEquals(1, len(invites))
 
-    self.deleteResponse(TeamMemberInvite,
+    self.deleteEmptyResponse(TeamMemberInvite,
-                        params=dict(code=invites[0].invite_token))
+                          params=dict(code=invites[0].invite_token))
 
     # Make sure the invite was deleted.
     self.deleteResponse(TeamMemberInvite,
@@ -1631,9 +1637,9 @@ class TestDeleteOrganizationTeamMember(ApiTestCase):
     self.assertEquals(len(json['members']), 4)
 
     # Delete the invite.
-    self.deleteResponse(TeamMember,
+    self.deleteEmptyResponse(TeamMember,
-                        params=dict(orgname=ORGANIZATION, teamname='readers',
+                               params=dict(orgname=ORGANIZATION, teamname='readers',
-                                    membername=membername))
+                                             membername=membername))
 
 
     # Verify the user was removed from the team.
@@ -1648,7 +1654,7 @@ class TestDeleteOrganizationTeamMember(ApiTestCase):
   def test_deletemember(self):
     self.login(ADMIN_ACCESS_USER)
 
-    self.deleteResponse(TeamMember,
+    self.deleteEmptyResponse(TeamMember,
                         params=dict(orgname=ORGANIZATION, teamname='readers',
                                     membername=READ_ACCESS_USER))
 
@@ -2046,7 +2052,7 @@ class TestDeleteRepository(ApiTestCase):
     dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'simple'], '{}')
 
     # Delete the repository.
-    self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO))
+    self.deleteEmptyResponse(Repository, params=dict(repository=self.SIMPLE_REPO))
 
     # Ensure the queue item is gone.
     self.assertIsNone(dockerfile_build_queue.get())
@@ -2068,7 +2074,7 @@ class TestDeleteRepository(ApiTestCase):
     dockerfile_build_queue.put([ADMIN_ACCESS_USER, 'anotherrepo'], '{}', available_after=-1)
 
     # Delete the repository.
-    self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO))
+    self.deleteEmptyResponse(Repository, params=dict(repository=self.SIMPLE_REPO))
 
     # Ensure the other queue item is still present.
     self.assertIsNotNone(dockerfile_build_queue.get())
@@ -2080,7 +2086,7 @@ class TestDeleteRepository(ApiTestCase):
     self.getResponse(Repository,
                      params=dict(repository=self.COMPLEX_REPO))
 
-    self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
+    self.deleteEmptyResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
 
     # Verify the repo was deleted.
     self.getResponse(Repository,
@@ -2150,7 +2156,7 @@ class TestDeleteRepository(ApiTestCase):
 
     # Delete the repository.
     with check_transitive_deletes():
-      self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
+      self.deleteEmptyResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
 
     # Verify the repo was deleted.
     self.getResponse(Repository,
@@ -2625,7 +2631,7 @@ class TestRepositoryNotifications(ApiTestCase):
     assert wid in ids
 
     # Delete the notification.
-    self.deleteResponse(RepositoryNotification,
+    self.deleteEmptyResponse(RepositoryNotification,
                         params=dict(repository=ADMIN_ACCESS_USER + '/simple', uuid=wid),
                         expected_code=204)
 
@@ -2786,7 +2792,7 @@ class TestListAndDeleteTag(ApiTestCase):
     assert len(prod_images) == len(staging_images) + 1
 
     # Delete prod.
-    self.deleteResponse(RepositoryTag,
+    self.deleteEmptyResponse(RepositoryTag,
                         params=dict(repository=ADMIN_ACCESS_USER + '/complex', tag='prod'),
                         expected_code=204)
 
@@ -2849,7 +2855,7 @@ class TestListAndDeleteTag(ApiTestCase):
     assert len(prod_images) > 0
 
     # Delete staging.
-    self.deleteResponse(RepositoryTag,
+    self.deleteEmptyResponse(RepositoryTag,
                         params=dict(repository=ADMIN_ACCESS_USER + '/complex', tag='staging'),
                         expected_code=204)
 
@@ -2989,7 +2995,7 @@ class TestRepoPermissions(ApiTestCase):
     self.assertEquals('admin', permissions[NO_ACCESS_USER]['role'])
 
     # Delete the user's permission.
-    self.deleteResponse(RepositoryUserPermission,
+    self.deleteEmptyResponse(RepositoryUserPermission,
                         params=dict(repository=ADMIN_ACCESS_USER + '/simple',
                                     username=NO_ACCESS_USER))
 
@@ -3040,7 +3046,7 @@ class TestRepoPermissions(ApiTestCase):
     self.assertEquals('admin', permissions['owners']['role'])
 
     # Delete the team's permission.
-    self.deleteResponse(RepositoryTeamPermission,
+    self.deleteEmptyResponse(RepositoryTeamPermission,
                         params=dict(repository=ORGANIZATION + '/' + ORG_REPO, teamname='owners'))
 
     # Verify.
@@ -3090,8 +3096,9 @@ class TestApiTokens(ApiTestCase):
     self.assertEquals('write', json['role'])
 
     # Delete the token.
-    self.deleteResponse(RepositoryToken,
+    self.deleteEmptyResponse(RepositoryToken,
-                        params=dict(repository=ADMIN_ACCESS_USER + '/simple', code=token_code))
+                               params=dict(repository=ADMIN_ACCESS_USER + '/simple',
+                                           code=token_code))
 
     # Verify.
     self.getResponse(RepositoryToken,
@@ -3219,7 +3226,7 @@ class TestUserRobots(ApiTestCase):
     assert NO_ACCESS_USER + '+bender' in robots
 
     # Delete the robot.
-    self.deleteResponse(UserRobot,
+    self.deleteEmptyResponse(UserRobot,
                         params=dict(robot_shortname='bender'))
 
     # Verify.
@@ -3328,7 +3335,7 @@ class TestOrgRobots(ApiTestCase):
     model.log.log_action('pull_repo', ORGANIZATION, performer=pull_robot, repository=repo)
 
     # Delete the robot and verify it works.
-    self.deleteResponse(OrgRobot,
+    self.deleteEmptyResponse(OrgRobot,
                         params=dict(orgname=ORGANIZATION, robot_shortname='bender'))
 
     # Verify the build is still present.
@@ -3362,7 +3369,7 @@ class TestOrgRobots(ApiTestCase):
     assert ORGANIZATION + '+bender' in robots
 
     # Delete the robot.
-    self.deleteResponse(OrgRobot,
+    self.deleteEmptyResponse(OrgRobot,
                         params=dict(orgname=ORGANIZATION, robot_shortname='bender'))
 
     # Verify.
@@ -3515,7 +3522,7 @@ class TestOrganizationApplicationResource(ApiTestCase):
     self.assertEquals(json, edit_json)
 
     # Delete the application.
-    self.deleteResponse(OrganizationApplicationResource,
+    self.deleteEmptyResponse(OrganizationApplicationResource,
                         params=dict(orgname=ORGANIZATION, client_id=FAKE_APPLICATION_CLIENT_ID))
 
     # Make sure the application is gone.
@@ -3660,7 +3667,8 @@ class TestBuildTriggers(ApiTestCase):
     trigger = json['triggers'][0]
 
     # Delete the trigger.
-    self.deleteResponse(BuildTrigger, params=dict(repository=ADMIN_ACCESS_USER + '/building',
+    self.deleteEmptyResponse(BuildTrigger,
+                               params=dict(repository=ADMIN_ACCESS_USER + '/building',
                                                   trigger_uuid=trigger['id']))
 
     # Verify it was deleted.
@@ -3936,7 +3944,7 @@ class TestUserAuthorizations(ApiTestCase):
     self.assertEquals(authorization, get_json)
 
     # Delete the authorization.
-    self.deleteResponse(UserAuthorization, params=dict(access_token_uuid=authorization['uuid']))
+    self.deleteEmptyResponse(UserAuthorization, params=dict(access_token_uuid=authorization['uuid']))
 
     # Verify it has been deleted.
     self.getJsonResponse(UserAuthorization, params=dict(access_token_uuid=authorization['uuid']),
@@ -4274,7 +4282,7 @@ class TestSuperUserKeyManagement(ApiTestCase):
 
     with assert_action_logged('service_key_delete'):
       # Delete the key.
-      self.deleteResponse(SuperUserServiceKey, params=dict(kid=key['kid']))
+      self.deleteEmptyResponse(SuperUserServiceKey, params=dict(kid=key['kid']))
 
       # Ensure the key no longer exists.
       self.getResponse(SuperUserServiceKey, params=dict(kid=key['kid']), expected_code=404)
@@ -4425,7 +4433,7 @@ class TestRepositoryManifestLabels(ApiTestCase):
 
     # Delete a label.
     with assert_action_logged('manifest_label_delete'):
-      self.deleteResponse(ManageRepositoryManifestLabel,
+      self.deleteEmptyResponse(ManageRepositoryManifestLabel,
                           params=dict(repository=repository,
                                       manifestref=tag_manifest.digest,
                                       labelid=label1['label']['id']))
@@ -4528,7 +4536,7 @@ class TestSuperUserManagement(ApiTestCase):
     self.assertEquals('freshuser', json['username'])
 
     # Delete the user.
-    self.deleteResponse(SuperUserManagement, params=dict(username='freshuser'), expected_code=204)
+    self.deleteEmptyResponse(SuperUserManagement, params=dict(username='freshuser'), expected_code=204)
 
     # Verify the user no longer exists.
     self.getResponse(SuperUserManagement, params=dict(username='freshuser'), expected_code=404)
@@ -4580,7 +4588,7 @@ class TestSuperUserManagement(ApiTestCase):
   def test_delete_message(self):
     self.login(ADMIN_ACCESS_USER)
     json = self.getJsonResponse(GlobalUserMessages)
-    self.deleteResponse(GlobalUserMessage, {"uuid": json['messages'][0]['uuid']}, 204)
+    self.deleteEmptyResponse(GlobalUserMessage, {"uuid": json['messages'][0]['uuid']}, 204)
 
     json = self.getJsonResponse(GlobalUserMessages)
 

test/test_endpoints.py

@@ -89,13 +89,20 @@ class EndpointTestCase(unittest.TestCase):
     self.assertEquals(rv.status_code, expected_code)
     return rv.data
 
-  def deleteResponse(self, resource_name, headers=None, expected_code=204, **kwargs):
+  def deleteResponse(self, resource_name, headers=None, expected_code=200, **kwargs):
     headers = headers or {}
     rv = self.app.delete(url_for(resource_name, **kwargs), headers=headers)
     self.assertEquals(rv.status_code, expected_code)
     return rv.data
 
-  def putResponse(self, resource_name, headers=None, data=None, expected_code=204, **kwargs):
+  def deleteEmptyResponse(self, resource_name, headers=None, expected_code=204, **kwargs):
+    headers = headers or {}
+    rv = self.app.delete(url_for(resource_name, **kwargs), headers=headers)
+    self.assertEquals(rv.status_code, expected_code)
+    self.assertEquals(rv.data, '') # ensure response body empty
+    return
+
+  def putResponse(self, resource_name, headers=None, data=None, expected_code=200, **kwargs):
     headers = headers or {}
     data = data or {}
     rv = self.app.put(url_for(resource_name, **kwargs), headers=headers, data=py_json.dumps(data))
@@ -651,7 +658,7 @@ class KeyServerTestCase(EndpointTestCase):
     model.service_keys.set_key_expiration('first', datetime.utcnow() + timedelta(seconds=100))
 
     with assert_action_logged('service_key_delete'):
-      self.deleteResponse('key_server.delete_service_key',
+      self.deleteEmptyResponse('key_server.delete_service_key',
                           headers={'Authorization': 'Bearer %s' % token},
                           expected_code=204, service='sample_service', kid='second')
 
@@ -671,7 +678,7 @@ class KeyServerTestCase(EndpointTestCase):
 
     # Delete our unapproved key with itself.
     with assert_action_logged('service_key_delete'):
-      self.deleteResponse('key_server.delete_service_key',
+      self.deleteEmptyResponse('key_server.delete_service_key',
                           headers={'Authorization': 'Bearer %s' % token},
                           expected_code=204, service='sample_service', kid='unapprovedkeyhere')
 
@@ -699,7 +706,7 @@ class KeyServerTestCase(EndpointTestCase):
 
     # Using the credentials of our approved key, delete our unapproved key
     with assert_action_logged('service_key_delete'):
-      self.deleteResponse('key_server.delete_service_key',
+      self.deleteEmptyResponse('key_server.delete_service_key',
                           headers={'Authorization': 'Bearer %s' % token},
                           expected_code=204, service='sample_service', kid='kid321')
 
@@ -712,7 +719,7 @@ class KeyServerTestCase(EndpointTestCase):
 
     # Delete a self-signed, approved key
     with assert_action_logged('service_key_delete'):
-      self.deleteResponse('key_server.delete_service_key',
+      self.deleteEmptyResponse('key_server.delete_service_key',
                           headers={'Authorization': 'Bearer %s' % token},
                           expected_code=204, service='sample_service', kid='kid123')