set seed sa permissions to folder level #93
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes modify the Terraform configuration for GCP bootstrap project setup. A new variable Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tofu/gcp/bootstrap/seed_project/main.tf (2)
36-45: Document the necessity of high-privilege rolesThe defined roles include powerful permissions like
securityAdminandfolderAdmin. While these might be necessary, it's important to document why each role is required and ensure they align with the principle of least privilege.Add comments explaining the necessity of each role:
locals { tf_sa_org_perms = [ + # Required for managing VPC networks and configurations "roles/compute.networkAdmin", + # Required for managing shared VPC configurations "roles/compute.xpnAdmin", + # Required for managing IAM policies and roles "roles/iam.securityAdmin", + # Required for managing service accounts "roles/iam.serviceAccountAdmin", + # Required for managing logging configurations "roles/logging.configWriter", + # Required for managing folder structure and policies "roles/resourcemanager.folderAdmin", ] }
47-51: LGTM! Consider adding explicit dependencyThe implementation correctly assigns folder-level permissions using
for_each. However, consider adding an explicit dependency on the bootstrap module to ensure proper creation order.Add
depends_onblock to ensure the bootstrap module is fully provisioned:resource "google_folder_iam_member" "tf_sa_org_perms" { for_each = toset(local.tf_sa_org_perms) folder = var.parent_folder_id role = each.value member = "serviceAccount:${module.bootstrap.terraform_sa_email}" + depends_on = [module.bootstrap] }🧰 Tools
🪛 checkov (3.2.334)
[HIGH] 47-52: Ensure no roles that enable to impersonate and manage all service accounts are used at a folder level
(CKV_GCP_44)
[HIGH] 47-52: Ensure no roles that enable to impersonate and manage all service accounts are used at a folder level
(CKV_GCP_44)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tofu/gcp/bootstrap/seed_project/main.tf(2 hunks)
🔇 Additional comments (1)
tofu/gcp/bootstrap/seed_project/main.tf (1)
13-14: LGTM! Verify removal of org-level permissions
Setting sa_org_iam_permissions to an empty list aligns with the goal of moving permissions to folder level, following the principle of least privilege.
Let's verify no org-level permissions remain:
✅ Verification successful
✅ Organization-level permissions removal verified
The change to set sa_org_iam_permissions = [] is correct and safe. The other organization-level IAM configurations found in tofu/gcp/org/cypago/main.tf are unrelated as they belong to a different service account (cypago) and don't affect the bootstrap/seed project's permissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining org-level IAM bindings in the codebase
# that might conflict with this change
# Search for org-level IAM configurations
rg -l "google_organization_iam|sa_org_iam_permissions.*=.*\[.*\]"
Length of output: 135
Script:
#!/bin/bash
# Let's examine the content of these files to understand the org-level IAM configurations
echo "=== Content of tofu/gcp/bootstrap/seed_project/main.tf ==="
rg -A 5 "google_organization_iam|sa_org_iam_permissions.*=.*\[.*\]" "tofu/gcp/bootstrap/seed_project/main.tf"
echo -e "\n=== Content of tofu/gcp/org/cypago/main.tf ==="
rg -A 5 "google_organization_iam|sa_org_iam_permissions.*=.*\[.*\]" "tofu/gcp/org/cypago/main.tf"
Length of output: 1137
fix #92
Summary by CodeRabbit