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

Support location being in the domain of a resource's URLs for only a subset of resources in an MMv1 service #12738

Open
AarshDhokai opened this issue Oct 7, 2022 · 11 comments
Labels
enhancement forward/exempt Never forward this issue mmv1-generator Provider-wide changes to resource templates or other generator changes service/cloudresourcemanager-tags size/l technical-debt
Milestone

Comments

@AarshDhokai
Copy link
Contributor

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment or link the pull request to this issue.

Terraform Version

Affected Resource(s)

  • google_tags_tag_binding

Terraform Configuration Files (if applicable)

resource "google_tags_tag_binding" "binding" {
     parent = "//sqladmin.googleapis.com/projects/crest-359621/instances/for-tag-bindings"
     tag_value = google_tags_tag_value.value.id
     location = "us-central1"
}

Issue Description

The google_tags_tag_binding create API (https://cloud.google.com/resource-manager/reference/rest/v3/tagBindings/create) doesn't support location attribute because at the back-end it uses service endpoints like https://{{location}}-cloudresourcemanager.googleapis.com/v3/tagBindings for regional resources, but for global resources it uses service endpoint like this : http://cloudresourcemanager.googleapis.com/v3/tagBindings , that i have found out via logs of gcloud cli, as well as the documentation has not mentioned such things. So, if it's possible to provide subset of resources or endpoints that can bind tags to regional resources via API or terraform.

Important Factoids

  • We can create tagbindings via gcloud CLI commands but can not create using API for regional resources.

References

Information about referencing Github Issues: #11448

@AarshDhokai AarshDhokai changed the title Magic-modules Technical-debt - Request to provide service endpoints for google_tags_tag_binding to bind location in regional resources. Request to provide service endpoints for google_tags_tag_binding to bind location in regional resources. Oct 7, 2022
@alextodicescu
Copy link

Nice find!
So what you're saying is the API doesn't support a location attribute, but rather the location is selected depending on which API service endpoint is used

I did some digging through the code (disclaimer: it's my 1st time looking through it and go is not my 1st language):

  • looks like there's a config map being built with default service endpoints:
    TagsBasePathKey: "https://cloudresourcemanager.googleapis.com/v3/",
    • we can probably add a regional endpoint in there
  • each CRUD method function gets the url with the replacevars() function:
    url, err := replaceVars(d, config, "{{TagsBasePath}}tagBindings")
    • we can probably add a condition if location resource attribute exists, then generate the url with regional endpoint

If anybody more familiar with the code can point me in the right direction, I can try to create a PR to have something to discuss on

@melinath melinath added technical-debt mmv1-generator Provider-wide changes to resource templates or other generator changes labels Oct 10, 2022
@melinath melinath changed the title Request to provide service endpoints for google_tags_tag_binding to bind location in regional resources. Support location being in the domain of a resource's URLs for only a subset of resources in an MMv1 service Oct 10, 2022
@rileykarson rileykarson added this to the Backlog milestone Oct 10, 2022
@rileykarson
Copy link
Collaborator

Given this is a new pattern, we should handwrite the regional variants of these resources. We're not sure if this pattern will happen in the future, as most APIs we've seen are global-only or regional-only.

@nphilbrook
Copy link
Contributor

Given this is a new pattern, we should handwrite the regional variants of these resources. We're not sure if this pattern will happen in the future, as most APIs we've seen are global-only or regional-only.

@rileykarson So something like a new resource google_tags_location_tag_binding ? I can work on that (which would be part of #11448 of course, not this issue).

@nphilbrook
Copy link
Contributor

I noticed this in the cloud run YAML:
https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/products/cloudrun/api.yaml#L21
It looks like this is doing something similar? I am digging through the generate golang for this to see if maybe this is already possible. From a cursory look at TF_LOG=DEBUG output from an apply creating a google_cloud_run_service resource, I can confirm that it was hitting the host us-west1-run.googleapis.com for my cloud run service hosted in that region.

@nphilbrook
Copy link
Contributor

Can anyone define the acronym CAI in the context of magic-modules for me?

@rileykarson
Copy link
Collaborator

So something like a new resource google_tags_location_tag_binding ?

Yep!

It looks like this is doing something similar? I am digging through the generate golang for this to see if maybe this is already possible.

What's different is that the entire API is regionalized in Cloud Run's case, but only a subset of the API is regionalized for tags. The resource code is probably able to make it most of the way there, but then we end up running into the operation code which differs. There are a few possible ways to work around that (handwrite the operation handler, possible, since the name will give us the URL, create a fake products/ entry for the regional variants and use legacy_name to align the names with the real product, handwrite the resources entirely).

Can anyone define the acronym CAI in the context of magic-modules for me?

Cloud Asset Inventory (https://cloud.google.com/asset-inventory/docs/overview), I think the cai variant in those examples means deregionalized versions of the URLs.

@nphilbrook
Copy link
Contributor

nphilbrook commented Oct 15, 2022

It looks like this is doing something similar? I am digging through the generate golang for this to see if maybe this is already possible.

What's different is that the entire API is regionalized in Cloud Run's case, but only a subset of the API is regionalized for tags. The resource code is probably able to make it most of the way there, but then we end up running into the operation code which differs. There are a few possible ways to work around that (handwrite the operation handler, possible, since the name will give us the URL, create a fake products/ entry for the regional variants and use legacy_name to align the names with the real product, handwrite the resources entirely).

Thanks, it's starting to come together now...handwriting a separate resource seems cromulent for now.

Can anyone define the acronym CAI in the context of magic-modules for me?

Cloud Asset Inventory (https://cloud.google.com/asset-inventory/docs/overview), I think the cai variant in those examples means deregionalized versions of the URLs.

Thank you! It makes more sense now.

@rileykarson
Copy link
Collaborator

Resolved by #10630 if we do it

@melinath
Copy link
Collaborator

@melinath
Copy link
Collaborator

I wonder if it might be easier to set up a regional variant of the product that uses the regional base_url, rather than forcing the resources to be handwritten.

@melinath
Copy link
Collaborator

ah yeah @rileykarson already mentioned that above:

create a fake products/ entry for the regional variants and use legacy_name to align the names with the real product

modular-magician added a commit to modular-magician/terraform-provider-google that referenced this issue Jan 13, 2025
hashicorp#12738)

[upstream:16b37875bc1fc09edf77f284f31bc5a2df9607e4]

Signed-off-by: Modular Magician <[email protected]>
modular-magician added a commit that referenced this issue Jan 13, 2025
#12738) (#20893)

[upstream:16b37875bc1fc09edf77f284f31bc5a2df9607e4]

Signed-off-by: Modular Magician <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement forward/exempt Never forward this issue mmv1-generator Provider-wide changes to resource templates or other generator changes service/cloudresourcemanager-tags size/l technical-debt
Projects
None yet
Development

No branches or pull requests

6 participants