Handle compute.googleapis.com properly when passed in via activate_api_identities #628
Conversation
|
Thanks for the PR! 🚀 |
|
Instead of raising an error/warning, should we simply filter this value out of the list automatically? |
That's an interesting suggestion. I like it because it defines the error out of existence and the outcome is what the user would probably expect. I would be in favor of that interface. Since I'm in the code at the moment, I'll go ahead and push a proposed implementation. |
rsinnet
changed the title
rsinnet
changed the title
…ely from other service identities when passed via activate_api_identities. This service identity cannot be created with the `google_project_service_identity` resource. Attempting to activate this service identity leads to an error response. It seems that this API call is not permitted despite lack of documentation. This can be seen by calling: ```sh gcloud alpha services identity create --service=compute.googleapis.com --log-http ``` Calling this same command with, e.g., cloudbuild.googleapis.com does not result in an error. It seems the service is activated by default, so instead just retrieve the email using `google_compute_default_service_account`. So get this email, create a combined map with this service identity (if present) and other service identities (if present), and then process this map as before, adding each role for each service identity.
rsinnet
force-pushed
the
user/rsinnet/update-readme
branch
from
ded4d38 to
aa659f8
Compare
@morgante I thought through it a bit more and modified it so it would still activate the specified roles for the compute account as it would for any other service identity to keep the interface consistent. Let me know what you think! |
| @@ -15,7 +15,8 @@ | |||
| */ | |||
|
|
|||
| locals { | |||
| services = var.enable_apis ? toset(concat(var.activate_apis, [for i in var.activate_api_identities : i.api])) : toset([]) | |||
| activate_compute_identity = 0 != length([for i in var.activate_api_identities : i if i.api == "compute.googleapis.com"]) | |||
There was a problem hiding this comment.
This logic can be simplified:
| activate_compute_identity = 0 != length([for i in var.activate_api_identities : i if i.api == "compute.googleapis.com"]) | |
| activate_compute_identity = contains(var.activate_api_identities, "compute.googleapis.com") |
There was a problem hiding this comment.
Oh cool didn't know that worked for keys in maps. Thanks!
There was a problem hiding this comment.
Oh actually it might not. Nevermind.
There was a problem hiding this comment.
We could use a splat expression if that is better. Could change the two lines like this:
activate_compute_identity = contains(var.activate_api_identities[*].api, "compute.googleapis.com")
services = var.enable_apis ? toset(concat(var.activate_apis, var.activate_api_identities[*].api)) : toset([])
There was a problem hiding this comment.
I ended up merging your original approach since it worked fine.
Synopsis
Process roles for the compute.googleapis.com service identity separately from other service identities when passed via activate_api_identities.
Details
This service identity cannot be created with the
google_project_service_identityresource. Attempting to activate this service identity leads to an error response. It seems that this API call is not permitted despite lack of documentation. This can be seen by calling:Calling this same command with, e.g., cloudbuild.googleapis.com does not result in an error.
It seems the service is activated by default, so instead just retrieve the email using
google_compute_default_service_account. So get this email, create a combined map with this service identity (if present) and other service identities (if present), and then process this map as before, adding each role for each service identity.