Ensure whole import id matches regex, do not allow partial match

[tommilligan]

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.

Description

Proposed change

When parsing an import id with a regex, the regex should match the entire id, and not allow partial matches.

Currently, we allow substring matching of import ids. This can lead to unexpected behaviour (see below).

This could be introduced as a breaking change in the next major version. Before then, it could be surfaced as a warning/deprecation message to the user.

I would be happy to work on this feature, if it would be accepted.

Motivation

When importing resources, if the import id is in the incorrect format, the error message produced can be misleading and unhelpful.

My specific case was importing a google_dns_record_set. I provided the id in the format projects/alice/managedzones/bob/rrsets/charlie/A, and got the opaque failure:

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Error when reading or editing DNS Record Set "managedzones": googleapi: Error 403: Permission denied on resource project projects.
│ Details:
│ [
│   {
│     "@type": "type.googleapis.com/google.rpc.ErrorInfo",
│     "domain": "googleapis.com",
│     "metadata": {
│       "consumer": "projects/projects",
│       "containerInfo": "projects",
│       "service": "dns.googleapis.com"
│     },
│     "reason": "CONSUMER_INVALID"
│   },
│   {
│     "@type": "type.googleapis.com/google.rpc.LocalizedMessage",
│     "locale": "en-US",
│     "message": "Permission denied on resource project projects."
│   },
│   {
│     "@type": "type.googleapis.com/google.rpc.Help",
│     "links": [
│       {
│         "description": "Google developers console",
│         "url": "https://console.developers.google.com"
│       }
│     ]
│   }
│ ]
│ , forbidden
│ 
│ 
╵

Cause

Multiple formats are supported for the google_dns_record_set resource (source code). For an example resource, valid ids would be:

• projects/alice/managedZones/bob/rrsets/charlie/A
• alice/bob/charlie/A
• bob/charlie/A

The import id is checked against each format in turn, with the first match being used. The pattern does not have to match the whole id, just a substring of it (source code).

I mistyped the id above with a lowercase z in managedzones. This causes the id to fail to match the first id format.
However, it matches the second id format coincidentally, as trailing characters are ignored:

pattern: "projects/(?P<project>[^/]+)/managedZones/(?P<managed_zone>[^/]+)/rrsets/(?P<name>[^/]+)/(?P<type>[^/]+)"
value:   "projects/alice/managedzones/bob/rrsets/charlie/A"
match:   nil

pattern: "(?P<project>[^/]+)/(?P<managed_zone>[^/]+)/(?P<name>[^/]+)/(?P<type>[^/]+)"
value:   "projects/alice/managedzones/bob/rrsets/charlie/A"
match:   "projects/alice/managedzones/bob"

This leads to extracting the field values as:

project = "projects"
managed_zone = "alice"
name = "managedzones"
type = "bob"

This then leads to an API request that fails, and confusing error message Error: Error when reading or editing DNS Record Set "managedzones": googleapi: Error 403: Permission denied on resource project projects.

New or Affected Resource(s)

• google_dns_record_set
• google_*

Any resource that calls the ParseImportId function is affected.

Potential Terraform Configuration

Configuration would remain unchanged.

It would be nice to pass this through as an opt-in feature, and make it a breaking change in the next major release, but I don't think there's a nice way to do that without overloading the id field in the import block, which I don't think is a good idea.

References

• Go Regexp.FindStringSubmatch reference: https://pkg.go.dev/regexp#Regexp.FindStringSubmatch
• import block reference: https://developer.hashicorp.com/terraform/language/import#syntax
• resourceDnsRecordSetImportState:

  terraform-provider-google/google/services/dns/resource_dns_record_set.go

  Line 653 in 51fbf6a

  "projects/(?P<project>[^/]+)/managedZones/(?P<managed_zone>[^/]+)/rrsets/(?P<name>[^/]+)/(?P<type>[^/]+)",

• ParseImportId:

  terraform-provider-google/google/tpgresource/import.go

  Line 31 in 51fbf6a

  if fieldValues := re.FindStringSubmatch(d.Id()); fieldValues != nil {

b/392628703

[rileykarson]

We added start and end of string delimiters to most resources in 5.0.0, this seems to have been missed.