Skip to content

Commit

Permalink
fix(sa-keys): better handle case where service account has no keys (#7)
Browse files Browse the repository at this point in the history
* fix(sa-keys): handle service accounts when they have 0 keys

* fix(sa-keys): correctly get key id from name

* chore(tests): add regression test for getting key info when there are no keys
  • Loading branch information
Avantol13 authored Mar 9, 2018
1 parent 6a8c3f9 commit 638c00c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
9 changes: 5 additions & 4 deletions cirrus/google_cloud/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,9 @@ def get_all_service_accounts(self):

if "nextPageToken" in response:
while response["nextPageToken"]:
response = self._authed_request("GET", api_url +
"&pageToken=" +
response["nextPageToken"]).json()
response = self._authed_request(
"GET", api_url + "&pageToken=" + response["nextPageToken"]
).json()
all_service_accounts.extend(response["accounts"])

return all_service_accounts
Expand Down Expand Up @@ -495,6 +495,7 @@ def delete_service_account_key(self, account, key_name):
key was successfully deleted
`Google API Reference <https://cloud.google.com/iam/reference/rest/v1/projects.serviceAccounts.keys/delete>`_
"""
key_name = key_name.split("/")[-1]
api_url = _get_google_api_url("projects/" + self.project_id +
"/serviceAccounts/" + account + "/keys/" +
key_name,
Expand Down Expand Up @@ -566,7 +567,7 @@ def get_service_account_keys_info(self, account):
GOOGLE_IAM_API_URL)

response = self._authed_request("GET", api_url + "&keyTypes=USER_MANAGED").json()
keys = response["keys"]
keys = response.get("keys", [])

return keys

Expand Down
33 changes: 32 additions & 1 deletion test/test_google_cloud_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def test_get_service_account_keys_info(test_cloud_manager):
assert test_cloud_manager._authed_session.get.called is True
assert len(keys) == 3

# Naive check to see if the new account appears in the call to delete
# Naive check to see if the new account appears in the call
args, kwargs = test_cloud_manager._authed_session.get.call_args
assert (
any(account in str(arg) for arg in args) or
Expand Down Expand Up @@ -913,5 +913,36 @@ def test_handle_expired_service_account_keys(monkeypatch, test_cloud_manager):
)


def test_service_account_keys_when_empty(test_cloud_manager):
"""
Test that getting a service account's keys when there aren't any results
in an empty list.
NOTE: google's api seems to not include the "keys" param at all when
there aren't any keys
"""
# Setup #
account = "some_service_account"
response = {}

test_cloud_manager._authed_session.get.return_value = (
_fake_response(200, json_response_as_dict=response)
)

# Call #
keys = test_cloud_manager.get_service_account_keys_info(account)

# Test #
assert test_cloud_manager._authed_session.get.called is True
assert len(keys) == 0

# Naive check to see if the new account appears in the call
args, kwargs = test_cloud_manager._authed_session.get.call_args
assert (
any(account in str(arg) for arg in args) or
any(account in str(kwarg) for kwarg in kwargs.values())
)


if __name__ == "__main__":
pytest.main(['-x', "-v", '.'])

0 comments on commit 638c00c

Please sign in to comment.