From 55713f39ec157fa72057a2590d26fd79da53d540 Mon Sep 17 00:00:00 2001 From: Kyle Burton Date: Thu, 4 May 2023 11:59:39 -0500 Subject: [PATCH] feat(backoff): Add backoff to give_service_account_billing_access --- .gitignore | 1 + .secrets.baseline | 10 ++++----- fence/resources/google/utils.py | 17 ++++++++++++---- tests/google/test_access_utils.py | 1 - tests/google/test_utils.py | 34 +++++++++++++++++++++++++++++++ 5 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 tests/google/test_utils.py diff --git a/.gitignore b/.gitignore index f02fbfd3a..4a76c3a3e 100644 --- a/.gitignore +++ b/.gitignore @@ -107,3 +107,4 @@ tests/resources/keys/*.pem .DS_Store .vscode +.idea diff --git a/.secrets.baseline b/.secrets.baseline index 315a41fbb..2423d6566 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -139,7 +139,7 @@ "filename": "fence/blueprints/storage_creds/google.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 139 + "line_number": 140 } ], "fence/blueprints/storage_creds/other.py": [ @@ -189,7 +189,7 @@ "filename": "fence/resources/google/utils.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 133 + "line_number": 137 } ], "fence/utils.py": [ @@ -241,14 +241,14 @@ "filename": "tests/conftest.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 1574 + "line_number": 1556 }, { "type": "Base64 High Entropy String", "filename": "tests/conftest.py", "hashed_secret": "227dea087477346785aefd575f91dd13ab86c108", "is_verified": false, - "line_number": 1597 + "line_number": 1579 } ], "tests/credentials/google/test_credentials.py": [ @@ -368,5 +368,5 @@ } ] }, - "generated_at": "2023-05-10T22:20:13Z" + "generated_at": "2023-05-15T16:30:09Z" } diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index 1cca206a9..74e43cb3c 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -1,6 +1,8 @@ import time import json import os + +import backoff from cryptography.fernet import Fernet import flask from flask import current_app @@ -36,6 +38,8 @@ from cdislogging import get_logger +from fence.utils import DEFAULT_BACKOFF_SETTINGS + logger = get_logger(__name__) @@ -221,10 +225,7 @@ def give_service_account_billing_access_if_necessary( # the SA access to bill the project provided # NOTE: this may fail if our fence SA doesn't have the right permissions # to add this role and update the project policy - with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: - g_cloud_manager.give_service_account_billing_access( - sa_account_id, project_id=r_pays_project - ) + _give_service_account_billing_access(sa_account_id, r_pays_project) except Exception as exc: logger.error( "Unable to create a custom role in Google Project {} to " @@ -265,6 +266,14 @@ def give_service_account_billing_access_if_necessary( ) +@backoff.on_exception(backoff.expo, Exception, **DEFAULT_BACKOFF_SETTINGS) +def _give_service_account_billing_access(sa_account_id, r_pays_project): + with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: + g_cloud_manager.give_service_account_billing_access( + sa_account_id, project_id=r_pays_project + ) + + def create_google_access_key(client_id, user_id, username, proxy_group_id): """ Return an access key for current user and client. diff --git a/tests/google/test_access_utils.py b/tests/google/test_access_utils.py index bd0a6f103..355f74e8b 100644 --- a/tests/google/test_access_utils.py +++ b/tests/google/test_access_utils.py @@ -5,7 +5,6 @@ from sqlalchemy import or_ from cirrus.errors import CirrusError -from cirrus.google_cloud import GoogleCloudManager from cirrus.google_cloud.iam import GooglePolicyMember import fence diff --git a/tests/google/test_utils.py b/tests/google/test_utils.py new file mode 100644 index 000000000..61f0568b3 --- /dev/null +++ b/tests/google/test_utils.py @@ -0,0 +1,34 @@ +from unittest.mock import patch, MagicMock + +import pytest + +from fence.resources.google.utils import ( + give_service_account_billing_access_if_necessary, + GoogleCloudManager, +) +from fence.utils import DEFAULT_BACKOFF_SETTINGS + + +def test_give_service_account_billing_access_if_necessary_fails(cloud_manager): + """ + Tests that the GCM give_service_account_billing_access backs off the right number of times when + give_service_account_billing_access_if_necessary calls it. + """ + + sa_private_key = {"client_email": "paying_requestor@somewhere.com"} + r_pays_project = "some_project_id" + + # Grab the GCM instance from the with block in _give_service_account_billing_access + cloud_manager_instance = cloud_manager.return_value.__enter__.return_value + # Set the GCM method to raise an exception. + cloud_manager_instance.give_service_account_billing_access.side_effect = Exception( + "Something's wrong" + ) + + with pytest.raises(Exception): + give_service_account_billing_access_if_necessary(sa_private_key, r_pays_project) + + assert ( + cloud_manager_instance.give_service_account_billing_access.call_count + == DEFAULT_BACKOFF_SETTINGS["max_tries"] + )