Skip to content

Commit bcced0a

Browse files
[auth|batch|aiogoogle] Favor latent credentials over specifying /gsa-key/key.json in application code (#13596)
This is mostly cleanup reducing the amount of the codebase that depends on GSA key files (which are not available in terra or a future keyless hail-vdc). The core bit is instead of threading `credentials_file="/gsa-key/key.json` through the batch and auth codebase we set `GOOGLE_APPLICATION_CREDENTIALS` in their deployments. I can't think of a scenario where `auth` or `batch` would need to use multiple identities so better to remove their ability to do so and always use the default credentials. I also did a bit of tidying up, using `$GOOGLE_APPLICATION_CREDENTIALS` instead of the hard-coded path and removing the credentials endpoints on the batch-driver which have been unused by workers for many months now.
1 parent efb233d commit bcced0a

File tree

14 files changed

+37
-80
lines changed

14 files changed

+37
-80
lines changed

auth/deployment.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ spec:
4545
value: "{{ default_ns.name }}"
4646
- name: HAIL_DEPLOY_CONFIG_FILE
4747
value: /deploy-config/deploy-config.json
48+
- name: GOOGLE_APPLICATION_CREDENTIALS
49+
value: /gsa-key/key.json
4850
- name: HAIL_DOMAIN
4951
valueFrom:
5052
secretKeyRef:

batch/batch/cloud/azure/driver/driver.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ async def create(
2828
machine_name_prefix: str,
2929
namespace: str,
3030
inst_coll_configs: InstanceCollectionConfigs,
31-
credentials_file: str,
3231
task_manager: aiotools.BackgroundTaskManager, # BORROWED
3332
) -> 'AzureDriver':
3433
azure_config = get_azure_config()
@@ -56,12 +55,10 @@ async def create(
5655
with open(os.environ['HAIL_SSH_PUBLIC_KEY'], encoding='utf-8') as f:
5756
ssh_public_key = f.read()
5857

59-
arm_client = aioazure.AzureResourceManagerClient(
60-
subscription_id, resource_group, credentials_file=credentials_file
61-
)
62-
compute_client = aioazure.AzureComputeClient(subscription_id, resource_group, credentials_file=credentials_file)
63-
resources_client = aioazure.AzureResourcesClient(subscription_id, credentials_file=credentials_file)
64-
network_client = aioazure.AzureNetworkClient(subscription_id, resource_group, credentials_file=credentials_file)
58+
arm_client = aioazure.AzureResourceManagerClient(subscription_id, resource_group)
59+
compute_client = aioazure.AzureComputeClient(subscription_id, resource_group)
60+
resources_client = aioazure.AzureResourcesClient(subscription_id)
61+
network_client = aioazure.AzureNetworkClient(subscription_id, resource_group)
6562
pricing_client = aioazure.AzurePricingClient()
6663

6764
region_monitor = await RegionMonitor.create(region)

batch/batch/cloud/driver.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,12 @@ async def get_cloud_driver(
1414
machine_name_prefix: str,
1515
namespace: str,
1616
inst_coll_configs: InstanceCollectionConfigs,
17-
credentials_file: str,
1817
task_manager: aiotools.BackgroundTaskManager,
1918
) -> CloudDriver:
2019
cloud = get_global_config()['cloud']
2120

2221
if cloud == 'azure':
23-
return await AzureDriver.create(
24-
app, db, machine_name_prefix, namespace, inst_coll_configs, credentials_file, task_manager
25-
)
22+
return await AzureDriver.create(app, db, machine_name_prefix, namespace, inst_coll_configs, task_manager)
2623

2724
assert cloud == 'gcp', cloud
28-
return await GCPDriver.create(
29-
app, db, machine_name_prefix, namespace, inst_coll_configs, credentials_file, task_manager
30-
)
25+
return await GCPDriver.create(app, db, machine_name_prefix, namespace, inst_coll_configs, task_manager)

batch/batch/cloud/gcp/driver/billing_manager.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212

1313
class GCPBillingManager(CloudBillingManager):
1414
@staticmethod
15-
async def create(db: Database, credentials_file: str, regions: List[str]):
16-
billing_client = aiogoogle.GoogleBillingClient(credentials_file=credentials_file)
15+
async def create(db: Database, regions: List[str]):
16+
billing_client = aiogoogle.GoogleBillingClient()
1717
product_versions_dict = await refresh_product_versions_from_db(db)
1818
bm = GCPBillingManager(db, product_versions_dict, billing_client, regions)
1919
await bm.refresh_resources()

batch/batch/cloud/gcp/driver/driver.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ async def create(
2525
machine_name_prefix: str,
2626
namespace: str,
2727
inst_coll_configs: InstanceCollectionConfigs,
28-
credentials_file: str,
2928
task_manager: aiotools.BackgroundTaskManager, # BORROWED
3029
) -> 'GCPDriver':
3130
gcp_config = get_gcp_config()
@@ -50,10 +49,9 @@ async def create(
5049
assert max(db_regions.values()) < 64, str(db_regions)
5150
app['regions'] = db_regions
5251

53-
compute_client = aiogoogle.GoogleComputeClient(project, credentials_file=credentials_file)
52+
compute_client = aiogoogle.GoogleComputeClient(project)
5453

5554
activity_logs_client = aiogoogle.GoogleLoggingClient(
56-
credentials_file=credentials_file,
5755
# The project-wide logging quota is 60 request/m. The event
5856
# loop sleeps 15s per iteration, so the max rate is 4
5957
# iterations/m. Note, the event loop could make multiple
@@ -65,7 +63,7 @@ async def create(
6563
)
6664

6765
zone_monitor = await ZoneMonitor.create(compute_client, regions, zone)
68-
billing_manager = await GCPBillingManager.create(db, credentials_file, regions)
66+
billing_manager = await GCPBillingManager.create(db, regions)
6967
inst_coll_manager = InstanceCollectionManager(db, machine_name_prefix, zone_monitor, region, regions)
7068
resource_manager = GCPResourceManager(project, compute_client, billing_manager)
7169

batch/batch/driver/main.py

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -242,22 +242,6 @@ async def delete_batch(request):
242242
return web.Response()
243243

244244

245-
# deprecated
246-
async def get_gsa_key_1(instance):
247-
log.info(f'returning gsa-key to activating instance {instance}')
248-
with open('/gsa-key/key.json', 'r', encoding='utf-8') as f:
249-
key = json.loads(f.read())
250-
return json_response({'key': key})
251-
252-
253-
async def get_credentials_1(instance):
254-
log.info(f'returning {instance.inst_coll.cloud} credentials to activating instance {instance}')
255-
credentials_file = '/gsa-key/key.json'
256-
with open(credentials_file, 'r', encoding='utf-8') as f:
257-
key = json.loads(f.read())
258-
return json_response({'key': key})
259-
260-
261245
async def activate_instance_1(request, instance):
262246
body = await json_request(request)
263247
ip_address = body['ip_address']
@@ -270,20 +254,6 @@ async def activate_instance_1(request, instance):
270254
return json_response({'token': token})
271255

272256

273-
# deprecated
274-
@routes.get('/api/v1alpha/instances/gsa_key')
275-
@activating_instances_only
276-
async def get_gsa_key(_, instance: Instance) -> web.Response:
277-
return await asyncio.shield(get_gsa_key_1(instance))
278-
279-
280-
# deprecated
281-
@routes.get('/api/v1alpha/instances/credentials')
282-
@activating_instances_only
283-
async def get_credentials(_, instance: Instance) -> web.Response:
284-
return await asyncio.shield(get_credentials_1(instance))
285-
286-
287257
@routes.post('/api/v1alpha/instances/activate')
288258
@activating_instances_only
289259
@add_metadata_to_request
@@ -1618,9 +1588,8 @@ async def on_startup(app):
16181588

16191589
inst_coll_configs = await InstanceCollectionConfigs.create(db)
16201590

1621-
credentials_file = '/gsa-key/key.json'
16221591
app['driver'] = await get_cloud_driver(
1623-
app, db, MACHINE_NAME_PREFIX, DEFAULT_NAMESPACE, inst_coll_configs, credentials_file, task_manager
1592+
app, db, MACHINE_NAME_PREFIX, DEFAULT_NAMESPACE, inst_coll_configs, task_manager
16241593
)
16251594

16261595
app['canceller'] = await Canceller.create(app)

batch/deployment.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ spec:
175175
env:
176176
- name: PORT
177177
value: "5000"
178+
- name: GOOGLE_APPLICATION_CREDENTIALS
179+
value: /gsa-key/key.json
178180
- name: HAIL_DOMAIN
179181
valueFrom:
180182
secretKeyRef:
@@ -403,6 +405,8 @@ spec:
403405
secretKeyRef:
404406
name: global-config
405407
key: internal_ip
408+
- name: GOOGLE_APPLICATION_CREDENTIALS
409+
value: /gsa-key/key.json
406410
- name: HAIL_SHA
407411
value: "{{ code.sha }}"
408412
{% if scope != "test" %}

build.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,8 @@ steps:
536536
set -ex
537537
export HAIL_DEFAULT_NAMESPACE={{ default_ns.name }}
538538
export HAIL_SCOPE={{ scope }}
539+
export GOOGLE_APPLICATION_CREDENTIALS=/auth-gsa-key/key.json
540+
539541
python3 /io/bootstrap_create_accounts.py
540542
serviceAccount:
541543
name: admin

ci/bootstrap_create_accounts.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ async def main():
9696
try:
9797
app['k8s_client'] = k8s_client
9898

99-
app['identity_client'] = get_identity_client(credentials_file='/auth-gsa-key/key.json')
99+
app['identity_client'] = get_identity_client()
100100

101101
for username, login_id, is_developer, is_service_account in users:
102102
user_id = await insert_user_if_not_exists(app, username, login_id, is_developer, is_service_account)

datasets/extract/extract_CADD.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
j = batch.new_job(name=f"{name}_{version}_{build}")
2626
j.image("gcr.io/broad-ctsa/datasets:050521")
27-
j.command("gcloud -q auth activate-service-account --key-file=/gsa-key/key.json")
27+
j.command("gcloud -q auth activate-service-account --key-file=$GOOGLE_APPLICATION_CREDENTIALS")
2828
j.command(f"wget -c -O - {snvs_url} {indels_url} | "
2929
"zcat | "
3030
"grep -v '^#' | "

0 commit comments

Comments
 (0)