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

Standardize type and naming for numeric IDs #20530

Open
wyardley opened this issue Dec 2, 2024 · 3 comments
Open

Standardize type and naming for numeric IDs #20530

wyardley opened this issue Dec 2, 2024 · 3 comments

Comments

@wyardley
Copy link

wyardley commented Dec 2, 2024

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.
  • If an issue is assigned to a user, that user is claiming responsibility for the issue.
  • Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Terraform Version & Provider Version(s)

Terraform v1.9.8
on darwin_arm64

  • provider registry.terraform.io/hashicorp/google v6.12.0

Affected Resource(s)

google_compute_network (numeric_id)
google_project (number)

Terraform Configuration

Debug Output

No response

Expected Behavior

  • The numeric ID should use a similar convention to other similar resources (so probably google_compute_network.network_idvsgoogle_compute_network.numeric_id`, if that's the convention?)
  • The numeric ID should have a similar type to other similar resources, matching the API (see comment here

While it would be a breaking change to change the type of numeric_id discussion here, maybe it would be possible to add network_id first, and then remove numeric_id as a breaking change (at least for that example)?

I'm mostly opening this as a way that breaking changes related to updating the type of these IDs could be tracked in GH.

Actual Behavior

  • The numeric ID type doesn't match the API (e.g., string instead of int)
  • The naming should match the provider's convention for server-side attribute names of this type

Steps to reproduce

  1. terraform apply

Important Factoids

There may well be other resources with similar problems. This affects both the resource and the data source, and presumably these should be updated in tandem if any updates are made.

With google_project, project_id already represents (confusingly) the project name (and number, which is a string, represents the numeric integer value), so it definitely can't follow the convention for that specific case, though I don't know if there is something other than number that could / should be used.

References

@rileykarson
Copy link
Collaborator

I would treat google_project as an exception- the project_id is a specific named value in the resource domain (and it is not a name, that's name which in modern APIs would be called display_name/displayName) and the project number is also a specific name for a value in the resource domain, covered by guidance like https://google.aip.dev/cloud/2510

Most of the rest of the time we're dealing with resource names (https://google.aip.dev/122) which we map into the Terraform id field (and sometimes name, although we often use name for the final segment of the AIP-122 name, i.e. the resource ID, because of GCE doing that. GCE predates that guidance).

Most APIs don't expose numeric ids, and GCE sometimes decides that its AIP-122 resource name includes the numeric id rather than the resource id (name field) even if that is inconsistent with the self link.

The obvious outcome here is- for GCE resources, we should expose the numeric id in an integer field called {{resource}}_id such as network_id or forwarding_rule_id. For any other outcome we probably need to make a grungy decision that balances consistency, churn, and API/AIP [guidance] alignment.

@NickElliot NickElliot removed their assignment Dec 3, 2024
@wyardley
Copy link
Author

wyardley commented Dec 5, 2024

@rileykarson I am working on adding network_id to network, but I did notice that a lot of docs seem to be referencing things like projects/{{project_id}}/global/networks/{{network_id}} and projects/{project_id}/regions/{region}/subnetworks/{subnetwork_id}, which I'm assuming refers to the network name vs. the numeric ID (since the latter hasn't been exposed until recently anyway, though I imagine numeric might work as well)? Will these docs cause confusion if / now that subnetwork_id and network_id exist as attributes??

Example:

projects/{network_project}/global/networks/{network_id}.

@rileykarson
Copy link
Collaborator

Possibly- we could consider changing those to network_name or similar for other resource kinds if they reference the name field in the final part. This goes back to inconsistency between AIP-122 and GCE- that final part is the resourceId value, so someone not familiar with GCE will assume it can be called an id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants