-
Notifications
You must be signed in to change notification settings - Fork 543
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
bug: fix svpc regression #438
Conversation
modules/shared_vpc_access/main.tf
Outdated
) : "" | ||
active_api_s_accounts = compact([local.gke_s_account, local.dataproc_s_account]) | ||
)] : [] | ||
active_api_s_accounts = flatten([local.gke_s_account, local.dataproc_s_account]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compact
was giving me the count cannot determine number of resources error
. Weirdly enough this approach does not. I think it has to do something with this hashicorp/terraform#25152
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think looping on the actual accounts is actually dangerous. I think something like this would be better:
locals {
apis = {
"container.googleapis.com": format("service-%s@container-engine-robot.iam.gserviceaccount.com", data.google_project.service_project.number
}
active_apis = setintersection(keys(local.apis), var.active_apis
subnetwork_api = setproduct(local.active_apis, var.shared_vpc_subnets)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up having to do tolist(setproduct(local.active_apis, var.shared_vpc_subnets))
as it was otherwise a set and throwing error This value does not have any indices
. In this case I dont think element ordering matters as it becomes a list of elements of type [api,subnet].
modules/shared_vpc_access/main.tf
Outdated
) : "" | ||
active_api_s_accounts = compact([local.gke_s_account, local.dataproc_s_account]) | ||
)] : [] | ||
active_api_s_accounts = flatten([local.gke_s_account, local.dataproc_s_account]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think looping on the actual accounts is actually dangerous. I think something like this would be better:
locals {
apis = {
"container.googleapis.com": format("service-%s@container-engine-robot.iam.gserviceaccount.com", data.google_project.service_project.number
}
active_apis = setintersection(keys(local.apis), var.active_apis
subnetwork_api = setproduct(local.active_apis, var.shared_vpc_subnets)
}
Co-authored-by: Morgante Pell <morgantep@google.com>
terraform-google-modules#438 broke this module
roles/compute.networkUser
at project level if subnets emptyservice project b
)service project
)roles/container.hostServiceAgentUser
granted at project level