From 89bbe157cb85d4ec64aac24a556473e613d6aa7b Mon Sep 17 00:00:00 2001 From: Kunming Qu <37601826+kunmingg@users.noreply.github.com> Date: Thu, 28 Feb 2019 14:52:06 -0800 Subject: [PATCH] clean up unused bindings in iam (#319) * clean up unused bindings in iam * fix * request body change * address comment * delete unused members for each binding * fix pylint * add unit test for trim bindings logic --- py/kubeflow/testing/cleanup_ci.py | 56 +++++++++++++++++-- py/kubeflow/tests/cleanup_ci_test.py | 38 +++++++++++++ py/kubeflow/tests/test-data/iam_bindings.yaml | 15 +++++ 3 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 py/kubeflow/tests/cleanup_ci_test.py create mode 100644 py/kubeflow/tests/test-data/iam_bindings.yaml diff --git a/py/kubeflow/testing/cleanup_ci.py b/py/kubeflow/testing/cleanup_ci.py index 21a7117ad95..6d403bb9c8a 100644 --- a/py/kubeflow/testing/cleanup_ci.py +++ b/py/kubeflow/testing/cleanup_ci.py @@ -201,19 +201,18 @@ def cleanup_service_accounts(args): keys = keys_client.list(name=a["name"]).execute() - is_expired = False + is_expired = True for k in keys["keys"]: valid_time = date_parser.parse(k["validAfterTime"]) now = datetime.datetime.now(valid_time.tzinfo) age = now - valid_time - if age > datetime.timedelta(hours=args.max_age_hours): - logging.info("Deleting account: %s", a["email"]) - iam.projects().serviceAccounts().delete(name=a["name"]).execute() - is_expired = True + if age < datetime.timedelta(hours=args.max_age_hours): + is_expired = False break - if is_expired: + logging.info("Deleting account: %s", a["email"]) + iam.projects().serviceAccounts().delete(name=a["name"]).execute() expired_emails.append(a["email"]) else: unexpired_emails.append(a["email"]) @@ -222,6 +221,50 @@ def cleanup_service_accounts(args): logging.info("Unexpired emails:\n%s", "\n".join(unexpired_emails)) logging.info("expired emails:\n%s", "\n".join(expired_emails)) + +def trim_unused_bindings(iamPolicy, accounts): + keepBindings = [] + for binding in iamPolicy['bindings']: + members_to_keep = [] + members_to_delete = [] + for member in binding['members']: + if not member.startswith('serviceAccount:'): + members_to_keep.append(member) + else: + accountEmail = member[15:] + if (not is_match(accountEmail)) or (accountEmail in accounts): + members_to_keep.append(member) + else: + members_to_delete.append(member) + if members_to_keep: + binding['members'] = members_to_keep + keepBindings.append(binding) + if members_to_delete: + logging.info("Delete binding for members:\n%s", ", ".join(members_to_delete)) + iamPolicy['bindings'] = keepBindings + +def cleanup_service_account_bindings(args): + credentials = GoogleCredentials.get_application_default() + iam = discovery.build('iam', 'v1', credentials=credentials) + accounts = [] + next_page_token = None + while True: + service_accounts = iam.projects().serviceAccounts().list( + name='projects/' + args.project, pageToken=next_page_token).execute() + for a in service_accounts["accounts"]: + accounts.append(a["email"]) + if not "nextPageToken" in service_accounts: + break + next_page_token = service_accounts["nextPageToken"] + + resourcemanager = discovery.build('cloudresourcemanager', 'v1', credentials=credentials) + iamPolicy = resourcemanager.projects().getIamPolicy(resource=args.project).execute() + trim_unused_bindings(iamPolicy, accounts) + + setBody = {'policy': iamPolicy} + resourcemanager.projects().setIamPolicy(resource=args.project, body=setBody).execute() + + def getAge(tsInRFC3339): # The docs say insert time will be in RFC339 format. # But it looks like it also includes a time zone offset in the form @@ -353,6 +396,7 @@ def cleanup_all(args): cleanup_deployments(args) cleanup_endpoints(args) cleanup_service_accounts(args) + cleanup_service_account_bindings(args) cleanup_workflows(args) cleanup_disks(args) diff --git a/py/kubeflow/tests/cleanup_ci_test.py b/py/kubeflow/tests/cleanup_ci_test.py new file mode 100644 index 00000000000..8cbf85384ac --- /dev/null +++ b/py/kubeflow/tests/cleanup_ci_test.py @@ -0,0 +1,38 @@ +from __future__ import print_function + +import unittest + +from kubeflow.testing import cleanup_ci +import os +import yaml + +class CleanupCiTest(unittest.TestCase): + def setUp(self): + self.test_dir = os.path.join(os.path.dirname(__file__), "test-data") + + def test_wait_for_workflow(self): + with open(os.path.join(self.test_dir, "iam_bindings.yaml")) as bd: + iam_bindings = yaml.load(bd) + self.assertTrue(len(iam_bindings['bindings']) == 3) + members = iam_bindings['bindings'][0]['members'] + for act in ['my-other-app@appspot.gserviceaccount.com', + 'kfctl-in-use@kubeflow-ci.iam.gserviceaccount.com', + 'kfctl-expired@kubeflow-ci.iam.gserviceaccount.com']: + self.assertTrue('serviceAccount:' + act in members) + cleanup_ci.trim_unused_bindings(iam_bindings, + ['kfctl-in-use@kubeflow-ci.iam.gserviceaccount.com']) + # One binding is deleted as it lost all members. + self.assertTrue(len(iam_bindings['bindings']) == 2) + trimed_members = iam_bindings['bindings'][0]['members'] + # binding to non-match service account still exists as it is not created by ci tests. + self.assertTrue( + 'serviceAccount:my-other-app@appspot.gserviceaccount.com'in trimed_members) + # binding to existing service account still exists. + self.assertTrue( + 'serviceAccount:kfctl-in-use@kubeflow-ci.iam.gserviceaccount.com' in trimed_members) + # binding to unexist service account is deleted. + self.assertTrue( + 'serviceAccount:kfctl-expired@kubeflow-ci.iam.gserviceaccount.com' not in trimed_members) + +if __name__ == "__main__": + unittest.main() diff --git a/py/kubeflow/tests/test-data/iam_bindings.yaml b/py/kubeflow/tests/test-data/iam_bindings.yaml new file mode 100644 index 00000000000..2de6fd3c355 --- /dev/null +++ b/py/kubeflow/tests/test-data/iam_bindings.yaml @@ -0,0 +1,15 @@ +bindings: +- members: + - user:mike@example.com + - group:admins@example.com + - domain:google.com + - serviceAccount:my-other-app@appspot.gserviceaccount.com + - serviceAccount:kfctl-in-use@kubeflow-ci.iam.gserviceaccount.com + - serviceAccount:kfctl-expired@kubeflow-ci.iam.gserviceaccount.com + role: roles/owner +- members: + - user:mike@example.com + role: roles/viewer +- members: + - serviceAccount:kfctl-expired@kubeflow-ci.iam.gserviceaccount.com + role: roles/editor