Generate consistent id value for data.google_compute_default_service_account

[sergei-ivanov]

Fixes #2611

Sets id field on the data.google_compute_default_service_account to be in the same format as for google_service_account resource.

The pattern is the same as for the name attribute as extracted by Google Cloud SDK:

$ gcloud iam service-accounts describe [email protected] --project=production
displayName: Compute Engine default service account
email: [email protected]
etag: <...>
name: projects/production/serviceAccounts/[email protected]
oauth2ClientId: '<...>'
projectId: production
uniqueId: '<...>'

With this change, data.google_compute_default_service_account.<...>.id can be used to populate google_service_account_iam_member.service_account_id attribute without the need for additional string concatenation.

[danawillow]

I don't like having people depend on the id field. We specifically don't document its format in order to discourage people from using it, because we'd like the ability to change it without breaking people. As we stand, we do have a major release coming up, and I like the idea of consistency between the resources. However, I think we can have this consistency in a way we both like by having the data source call the service account endpoint after it gets the email of the service account, and then fill out all the same fields that the other service account data source does (the important one being name, which is the fully-qualified name). What do you think?

[sergei-ivanov]

I provided a lot more details on the linked issue #2611 but the main pain point at the moment is in configuring google_service_account_iam_member and friends. The resulting configuration is not consistent between the default service account and user-defined service accounts. And moreover, I'd argue that IAM config it is the only place where service account id is used (AFAIK). Service account email works just fine elsewhere and is much simpler to deal with.

So in a way the better way to resolve the issue would be to deprecate service_account_id on google_service_account_iam_xxxxx resources and introduce service_account_email instead. The IAM resource would then derive the project information (if needed) from service account email and carry on. Is it what you are suggesting too?

[danawillow]

Deprecations are annoying, and calling something id, name, or email can easily be solved with better documentation. Let me come up with a PR real quick to show you that id isn't necessary for that, and then I can come back to my suggestion for this.

[sergei-ivanov]

There's already an existing issue for documentation: #2180
And like the original submitter I spent a considerable amount of time figuring out what the correct configuration might be.

[sergei-ivanov]

@danawillow I saw your PR to close off #2180 and I think it is a great improvement on the documentation part. Thank you very much for that!

If the use of id attribute is discouraged, then the only thing that still lacks consistency is data.google_compute_default_service_account, which does not have a name attribute exported. So, instead of changing the format of the id attribute, shall we have a read-only name (and perhaps also display_name for completeness) attribute alongside email? Then it will be possible to refer to data.google_compute_default_service_account.xxx.name and google_service_account.yyy.name in a consistent way.

[danawillow]

You're welcome! I think we should do both of those things- that's what I was trying to get at in my comment above (#2615 (comment))

[danawillow]

@sergei-ivanov were you planning on making the changes to also have name, or did you want someone from our side to do that?

[sergei-ivanov]

@danawillow I might give it a go, but I am confused about the correct place where I would need to make changes. Do I need to make changes in magic modules here, or in this repository, or in both places?

[danawillow]

Here or there is fine. If you do it here, we'll migrate it to magic modules for you (and give you the credit here).

[sergei-ivanov]

Ok, let me update this PR. Will I need to update the other one ( #2616) against master too?

[danawillow]

We changed last week to only working against master, so probably best to close this one and only use that one, or update the branch on this one to master and close that one.

[sergei-ivanov]

Ok, I finally have a complete patch that also includes documentation updates. I have just verified it on my test GCP project and everything seems to work a treat. Let me submit it directly into magic modules repo for your review. I am going to close these PRs. Please note that I have also updated the title and the description of the linked bug (although it looks more like a feature request now).

[sergei-ivanov]

For the reference:
GoogleCloudPlatform/magic-modules#1124

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!