Skip to content

Conversation

@adityachoudhari26
Copy link
Contributor

@adityachoudhari26 adityachoudhari26 commented Oct 23, 2025

Summary by CodeRabbit

  • New Features
    • Support for a custom FQDN and Google authentication configuration when deploying modules.
    • Service accounts granted access to a storage bucket with appropriate storage administration role.
    • Exposes the storage bucket name as an output for easier integration and reference.
    • Improved provisioning order so storage is created before service account configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Walkthrough

Adds passing of storage bucket name into service_accounts, exports the storage bucket name from storage, grants the GKE service account storage.objectAdmin on that bucket, updates service_accounts to depend on storage, and adds fqdn and google_auth inputs to the ctrlplane module in examples.

Changes

Cohort / File(s) Summary
Examples / Root module
examples/basic/main.tf, main.tf
Adds fqdn and google_auth inputs to the ctrlplane module invocation; binds bucket_name = module.storage.bucket_name into module.service_accounts and expands depends_on to include module.storage.
Service Accounts (logic & vars)
modules/service_accounts/main.tf, modules/service_accounts/variables.tf
Adds variable "bucket_name" (string) and updates namespace list to include ctrlplane-workspace-engine; creates google_storage_bucket_iam_member gke_sa_bucket_rw granting roles/storage.objectAdmin to the GKE service account using var.bucket_name.
Storage outputs
modules/storage/outputs.tf
Adds output "bucket_name" exposing google_storage_bucket.this.name with a description.

Sequence Diagram

sequenceDiagram
    participant Root as Root Configuration
    participant Storage as Storage Module
    participant Ctrl as Ctrlplane Module
    participant SA as Service Accounts Module
    participant GCS as Google Cloud Storage
    participant GKE as GKE Service Account

    Root->>Storage: create bucket
    Storage-->>Root: return bucket_name
    Root->>Ctrl: pass fqdn, google_auth
    Root->>SA: pass bucket_name (depends_on Storage)
    SA->>GKE: reference GKE service account (namespaces include ctrlplane-workspace-engine)
    SA->>GCS: create IAM member (storage.objectAdmin) on bucket_name for GKE SA
    GKE->>GCS: access bucket objects (read/write)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through modules, small and spry,
I tied a bucket to a GKE eye,
With IAM stitched and inputs neat,
FQDN and secrets meet,
A rabbit cheers — infra works on high!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "chore: Add bucket rw iam binding" clearly and specifically refers to a real and significant part of the changeset. The core infrastructure change being introduced is the addition of a google_storage_bucket_iam_member resource (gke_sa_bucket_rw) that grants the storage.objectAdmin role to the GKE service account, which directly corresponds to what the title describes as "bucket rw iam binding." While the PR includes supporting changes such as adding variables, outputs, and module dependencies to wire this capability together, the title accurately captures the primary feature being implemented. The title is concise, specific, and not misleading or vague.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bucket-rw

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a442f5f and 6663302.

📒 Files selected for processing (5)
  • examples/basic/main.tf (1 hunks)
  • main.tf (1 hunks)
  • modules/service_accounts/main.tf (2 hunks)
  • modules/service_accounts/variables.tf (1 hunks)
  • modules/storage/outputs.tf (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • main.tf
  • modules/storage/outputs.tf
  • modules/service_accounts/variables.tf
🔇 Additional comments (3)
modules/service_accounts/main.tf (2)

22-22: Add new workload member consistently.

Adding ctrlplane-workspace-engine to the members list follows the existing pattern and appears syntactically correct. This workload will receive the same workload identity and IAM bindings as other ctrlplane workloads.


64-68: All verification concerns have been addressed—no issues found.

The implementation is correctly structured:

  • var.bucket_name is properly defined as a required string variable in modules/service_accounts/variables.tf
  • The root module correctly passes bucket_name = module.storage.bucket_name when invoking the service_accounts module
  • The depends_on = [module.gke, module.storage] ensures proper resource creation order
  • The required variable design is intentional and correct—Terraform will properly fail at plan time if the dependency chain is broken, which is the expected behavior
examples/basic/main.tf (1)

27-31: I need to examine the actual module being called in the example file to verify the scope and nature of these inputs. Let me search for the module definition:

Based on the search result, I found that the ctrlplane module uses domains = ["<fqdn>"] in its basic usage example, which suggests fqdn values should be passed via the domains array, not as a separate fqdn input. Let me verify the actual module interface:

Verify module inputs and assess potential redundancy in configuration.

Initial investigation found that the official ctrlplane module documentation shows basic usage with domains = ["<fqdn>"], suggesting domains is the primary input for domain configuration. However, the script results showed both fqdn and google_auth variables are defined at lines 1 and 58 in the root variables.tf, and also in modules/helm_release/variables.tf.

The concerns remain valid but require clarification:

  1. Input scope: Confirm whether fqdn is a valid input to the specific ctrlplane module being called in examples/basic/main.tf, or if it's a locally-scoped variable not intended for the module.
  2. Configuration redundancy: Lines 26 (domains = ["example.com"]) and 27 (fqdn = "example.com") appear redundant if they both configure the same domain—verify if this is intentional or represents misaligned module usage.
  3. google_auth credentials: The placeholder credentials are acceptable for example code, but ensure they're properly documented as examples.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@adityachoudhari26 adityachoudhari26 changed the title chore: add bucket rw iam binding chore: Add bucket rw iam binding Oct 23, 2025
@adityachoudhari26 adityachoudhari26 merged commit 615e253 into main Oct 23, 2025
5 of 6 checks passed
@adityachoudhari26 adityachoudhari26 deleted the bucket-rw branch October 23, 2025 18:11
@adityachoudhari26
Copy link
Contributor Author

This PR is included in version 1.11.6 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant