feat: add confidential space #1435
Conversation
* Adds Confidential Space
renato-rudnicki
changed the title
renato-rudnicki
requested review from
a team,
eeaton,
rjerrems and
sleighton2022
as code owners
There was a problem hiding this comment.
Thanks @renato-rudnicki - I'll defer to others on the intended functionality.
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Google Cloud Confidential Space by adding a new Terraform module and updating existing configurations across various stages of the infrastructure deployment. The changes are comprehensive, covering IAM, networking, project setup, and integration tests. My review highlights several opportunities to enhance security by adhering more closely to the principle of least privilege, particularly by reducing overly permissive IAM roles such as roles/storage.admin and roles/artifactregistry.admin. Additionally, I've identified areas to improve the robustness and maintainability of the Terraform code by addressing fragile logic, redundant retries, and dependency management. Minor corrections in documentation and scripts are also suggested.
| locals { | ||
| repo_names = ["bu1-example-app"] | ||
| repo_names = ["bu1-example-app"] | ||
| cmd_prompt = "gcloud builds submit . --tag ${local.confidential_space_image_tag} --project=${local.cloudbuild_project_id} --service-account=projects/${local.cloudbuild_project_id}/serviceAccounts/tf-cb-builder-sa@${local.cloudbuild_project_id}.iam.gserviceaccount.com --gcs-log-dir=gs://${module.infra_pipelines[0].log_buckets["bu1-example-app"]} --worker-pool=${local.cloud_build_private_worker_pool_id} || ( sleep 46 && gcloud builds submit . --tag ${local.confidential_space_image_tag} --project=${local.cloudbuild_project_id} --service-account=projects/${local.cloudbuild_project_id}/serviceAccounts/tf-cb-builder-sa@${local.cloudbuild_project_id}.iam.gserviceaccount.com --gcs-log-dir=gs://${module.infra_pipelines[0].log_buckets["bu1-example-app"]} --worker-pool=${local.cloud_build_private_worker_pool_id})" |
There was a problem hiding this comment.
This cmd_prompt local is extremely long and contains duplicated command strings, making it hard to read and maintain. The retry logic (|| ( sleep 46 && ... )) is also duplicated. Consider defining a base command and applying the retry logic separately to improve clarity.
| } | ||
|
|
||
| create_cmd_entrypoint = "bash" | ||
| create_cmd_body = "${local.cmd_prompt} || ( sleep 45 && ${local.cmd_prompt})" |
There was a problem hiding this comment.
There appears to be a redundant, nested retry mechanism here. The local.cmd_prompt variable already contains a retry with sleep 46. This create_cmd_body then wraps it in another retry with sleep 45. This is confusing and should be simplified to have a single, clear retry mechanism.
| "roles/iam.serviceAccountUser", | ||
| "roles/confidentialcomputing.workloadUser", | ||
| "roles/iam.workloadIdentityPoolAdmin", | ||
| "roles/storage.admin", |
There was a problem hiding this comment.
The iam_roles list for the workload service account includes roles/storage.admin. This is a highly permissive role. Please consider if a more scoped-down role like roles/storage.objectAdmin or more granular roles (objectCreator, objectViewer) would be sufficient to adhere to the principle of least privilege.
2 participants
This PR adds a new module for confidential space workspace. The module creates a single project with an instance using a confidential image to run into the confidential space.