github.com/gedex/inflector appears to cause `ContentCaches` to resolve to `ContentCach`

[zbuchheit]

What happened?

While using the tf-bridge, the library we are using is incorrectly singularizing ContentCaches to ContentCach with a type. The library referenced hasn't been updated in >7 years so it might be worth discussing we fork it or come up with an alternative.

This typo doesn't exist in the terraform provider, but does exist in the schema.json generated from it.

Example

package main

import (
    "fmt"

    "github.com/gedex/inflector"
)

func main() {
    fmt.Println(inflector.Singularize("ContentCaches"))
}

$ go run .
ContentCach

Output of pulumi about

N/A

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

[iwahbe]

Hey @zbuchheit. Thanks for pointing this out.

Unfortunately, taking any change to the inflector is breaking to effected users.

We could support some type of inferred property field aliases by comparing the old and new inflector results, but that would only get us so far and be bug prone.

Conflict Scenario

If there was a TF resource with two properties: ``` cache: {Type: T} caches: {Type: List[T], MaxItems: 1 } ``` We would project the resource into pulumi as: ``` cache: T, cach: T ``` If we fixed the inflector, we would instead project to: ``` cache: T caches: T // Not singularized because of a name conflict ``` We would need to detect this and support migrating from `cach` to `caches`, but not `cache` (when there is a conflict, or there was ever a conflict).

Unfortunately, the fastest solution would be to override the tfbridge.SchemaInfo.Name property to fix the problem at the provider level. To allow a smooth migration from the old name to the new name, you would need to specify a state migration like this:

https://github.com/pulumi/pulumi-aws/blob/ba6524913c1a4b5b254bcc20465ed59b83b92b34/provider/resources.go#L3088-L3101

[mikhailshilkov]

@iwahbe Thank you for providing the immediate advice, it all makes sense.

I think we still need to decide and write down a long-term plan here. We depend on a library that hasn't been updated in seven years, so we should probably decide how we can migrate off it and how we could roll out such a change (an opt-in bridge flag on a major provider release?)

[iwahbe]

We depend on a library that hasn't been updated in seven years, so we should probably decide how we can migrate off it and how we could roll out such a change (an opt-in bridge flag on a major provider release?)

The only way we can use this type of inflector is if it is stable. If the library was updated regularly, we wouldn't be able to use it like we do.

migrate off it and how we could roll out such a change (an opt-in bridge flag on a major provider release?)

Unless we were willing to require effected users to make state edits on upgrade, we would still need a automatic migration story.

[mikhailshilkov]

We've vendored the dependency and brought it under go modules. The Cache singularization issue was fixed in https://github.com/pulumi/inflector/releases/tag/v0.2.1

We can't immediately toll this change out in the bridge because it's a breaking change. The gradual rollout is tracked in https://github.com/pulumi/pulumi-terraform-bridge/issues/2132

For now, any provider will need to add a replacement directive to their go.mod file:

replace github.com/pulumi/inflector => github.com/pulumi/inflector v0.2.1

[zbuchheit]

@mikhailshilkov we should re-open this issue. ContentCaches and content_caches is still not getting singularized correctly as desired. This did fix caches correctly.

[mikhailshilkov]

The above is fixed with the v0.2.1 version of inflector