Weaken provider name segment validation to match real Terraform registries #20
Conversation
…type Per the registry and Terraform Cloud, underscores are valid in both segments.
The parsed value is empty if the parse fails. This resulted in error messages like: `Invalid provider namespace "" in source "example.com/badnamespace_/aws"`
Same story as the last one, parsed value is empty if parse fails. This resulted in errors like: ``` Invalid provider source hostname: Invalid provider source hostname namespace "" in source "badhost!/hashicorp/aws": idna: disallowed rune U+0021" ```
…vior This is a breaking change to Terraform, but should be observable as a bugfix: - Terraform no longer accepts provider name(space)s that the registry wouldn't allow, like `Испытание`. - Terraform now accepts provider name(space)s that it previously didn't, but which the registry has always allowed (like `example_corp` or `abc--123`). - Terraform no longer lowercases name(space)s that contain capital letters. This is a no-op against registry.terraform.io, which appears to route `thing` and `thING` to the same place (and thus must deduplicate on lowercased values), but is meaningful on Terraform Cloud, which does not collapse cases.
| Type: "AWS", | ||
| Namespace: "HashiCorp", |
There was a problem hiding this comment.
One of the benefits of this library, though maybe implied, rather than explicitly documented, was canonical representation, which in turn made comparison and sorting possible.
For example, hashicorp/aws today is treated as equal to HashiCorp/AWS when it comes to cache entries (the lock file) etc. It is also one of the reasons Terraform LS uses this library and why we extracted it.
My understanding is that the intention was merely to allow -, so I'm assuming there isn't a strong reason to introduce case sensitivity?
There was a problem hiding this comment.
As best as I was able to determine, the private registry in Terraform Cloud (unlike the public registry) does not consider myorg/aws and myorg/AWS to be the same address, which would mean automatic lowercasing is a wrong behavior.
But unlike the underscores, that one was based on a reading of the code rather than an actual experiment. I didn't attempt publishing a provider with capital letters in its name (which, to be fair, would be a pretty perverse thing to do, and I'm not sure anyone would try it if they weren't attempting to break something).
Org names with uppercase letters are a bit more of a pressing concern, but I didn't attempt that yet either. I could give that a shot, it's somewhat easier than re-publishing my test provider with a vandalized name.
I'm open to adding some basic string lowercasing to restore case-collapse, if you're confident that we still need it.
There was a problem hiding this comment.
Yeah on further thought, if capitalized org names are a problem in TFC, then the solution is to add case normalization to its registry's routing, not to change the semantics over in this library. We already know TFC enforces org name uniqueness across any combination of letter cases, and the top-level org route accounts for that; if we missed handling that somewhere deeper in the routing, we'll just fix it.
Fixes hashicorp/terraform#32694.
This library was using an extra-strict validation scheme for the namespace and type segments of a provider source address, approximately treating them as hostnames. However, this made many providers in real-world registries inaccessible, since both Terraform Cloud and registry.terraform.io allow underscores in provider namespaces and types.
This is a breaking change to Terraform, but should be observable as a bugfix:
Terraform no longer accepts provider name(space)s that the registry wouldn't allow, like
Испытание.Terraform now accepts provider name(space)s that it previously didn't, but which the registry has always allowed (like
example_corporabc--123).Terraform no longer lowercases name(space)s that contain capital letters. This is a no-op against registry.terraform.io, which appears to route
thingandthINGto the same place (and thus must deduplicate on lowercased values), but is meaningful on Terraform Cloud, which does not collapse cases.