Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: adding a module for networking/iam requirements and example for composer-v2 #60

Merged

Conversation

sanmaym
Copy link
Contributor

@sanmaym sanmaym commented Oct 24, 2022

This feature incorporates the networking and IAM requirements that are pre requisite for having a successful composer-v2 environment within Shared VPC and within VPC Service Control Perimeter

@sanmaym sanmaym marked this pull request as draft October 25, 2022 03:18
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing seemed off regarding the missing project_id var you mentioned as I see the project_id plumbed correctly. Could you add this to the https://github.com/terraform-google-modules/terraform-google-composer/blob/master/build/int.cloudbuild.yaml to kick off tests in CI and see if any issues happen

test/fixtures/composer_v2_sharedvpc_prereq/main.tf Outdated Show resolved Hide resolved
Comment on lines 52 to 55
resource "google_project_iam_member" "service_account_user" {
project = var.service_project_id
role = "roles/iam.serviceAccountUser"
member = "serviceAccount:${google_service_account.composer_sa.email}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we scope this to just a single SA rather than giving access to all service accounts in a project?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it wasn't possible. I ran into error messages otherwise. both for IAM binding as well as composer.worker.

modules/composer_net/variables.tf Outdated Show resolved Hide resolved
modules/composer_net/variables.tf Outdated Show resolved Hide resolved
@sanmaym
Copy link
Contributor Author

sanmaym commented Nov 2, 2022

@bharathkkb i am in process of completely overhauling the setup process. I am including creation of two projects (host/ service project), creating dns zones, records (for restricted VIP, etc). The attachment of service project to host project requires "roles/compute.xpnAdmin" at folder/org level. However, I am worried how we can do this via your CI pipeline. I think the orchestrator service account gets org viewer, project creator, and thats about it. How do we ensure we get folder IAM permissions so that it can do that IAM binding ? Which step in your CI pipeline does that? or should I just do it via gcloud in the init pipeline?

@sanmaym
Copy link
Contributor Author

sanmaym commented Nov 3, 2022

@bharathkkb sorry for the excessive nudge. I have it almost figured out. I have an e2e project setup that creates all pre reqs as well. to summarize I have two questions

  1. If i create a host project/ service project, I need to use google-beta provider to associate service project but that needs shared vpc admin at folder level for the orchestrator service account. right now, the orchestrator service account gets org viewer, billing account user and project creator i believe. but I need to grant folder IAM admin as well wherehever this gets created within google org
  2. How are the variables getting passed during runtime to examples module? (such as the project that is being created during setup phase to the integration test). I believe its two separate state files as its separate directories.

Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments

default = null
}

variable "pod_ip_allocation_range_name" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sanmaym should we remove this?

examples/composer_v2_sharedvpc_prereq/variables.tf Outdated Show resolved Hide resolved
modules/composer_net/outputs.tf Outdated Show resolved Hide resolved
@comment-bot-dev
Copy link

@sanmaym
Thanks for the PR! 🚀
✅ Lint checks have passed.

@bharathkkb bharathkkb merged commit b44bb6f into terraform-google-modules:master Nov 8, 2022
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.

3 participants