Skip to content

Commit

Permalink
Fix check_sa_exists not checking all service accounts in python libra…
Browse files Browse the repository at this point in the history
…ry (kubeflow#718)

* Fix check_sa_exists not checking all service accounts

* Fixed lint errors
  • Loading branch information
johnbuluba authored Mar 7, 2020
1 parent 48c7791 commit fd2acd8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
4 changes: 1 addition & 3 deletions python/kfserving/kfserving/api/creds_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ def check_sa_exists(namespace, service_account):
'''Check if the specified service account existing.'''
sa_list = client.CoreV1Api().list_namespaced_service_account(namespace=namespace)

sa_name_list = []
for item in range(0, len(sa_list.items)-1):
sa_name_list.append(sa_list.items[item].metadata.name)
sa_name_list = [sa.metadata.name for sa in sa_list.items]

if service_account in sa_name_list:
return True
Expand Down
19 changes: 19 additions & 0 deletions python/kfserving/test/test_creds_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from unittest import mock

from kubernetes.client import V1ServiceAccountList, V1ServiceAccount, V1ObjectMeta

from kfserving.api.creds_utils import check_sa_exists


@mock.patch('kubernetes.client.CoreV1Api.list_namespaced_service_account')
def test_check_sa_exists(mock_client):
# Mock kubernetes client to return 2 accounts
accounts = V1ServiceAccountList(
items=[V1ServiceAccount(metadata=V1ObjectMeta(name=n)) for n in ['a', 'b']]
)
mock_client.return_value = accounts

# then a, b should exist, c should not
assert check_sa_exists('kubeflow', 'a') is True
assert check_sa_exists('kubeflow', 'b') is True
assert check_sa_exists('kubeflow', 'c') is False

0 comments on commit fd2acd8

Please sign in to comment.