Fix non-pulumi owned provider import alias #10447
Conversation
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
pkg/codegen/nodejs/utilities.go
Outdated
| if strings.HasPrefix(name, "pulumi_") { | ||
| name = strings.TrimPrefix(name, "pulumi_") | ||
| } |
There was a problem hiding this comment.
Putting this here in makeValidIdentifier seems like it would affect more than just imports. It looks like the package name is inconsistently handled. I'd look at genPreamble to figure out why the import is generated as pulumi_eventstorecloud.
The schema for Pulumi package says its name is eventstorecloud: https://github.com/EventStore/pulumi-eventstorecloud/blob/main/provider/cmd/pulumi-resource-eventstorecloud/schema.json
But the NPM package is @eventstore/pulumi-eventstorecloud.
Is genPreamble generating the import * as name from the name of the NPM package instead of the name of the Pulumi package? That'd be my hunch.
There was a problem hiding this comment.
There was a problem hiding this comment.
Right - I think the issue is that we're working with the wrong string here. The issue isn't detecting whether it's "Pulumi owned", it's that we're doing:
makeValidIdentifier(NPM package name after the /)
Instead of:
makeValidIdentifier(Pulumi package name)
Where:
- NPM package name:
@eventstore/pulumi-eventstorecloud - Pulumi package name:
eventstorecloud
The NPM package name ending in the same string as the Pulumi package name here is a coincidence and a misdirect.
There was a problem hiding this comment.
I think that makeValidIdentifier is (and should remain) special purposed to take an identifier and deterministically make it valid. It should not be used for correcting pulumi names.
Pulumi does not reserve the pulumi_ namespace, so this is not a reliable way to determine if a package is Pulumi owned. The schema allows you to override package import names, so the only valid way to get an import name is to look at the schema:
pulumi/pkg/codegen/nodejs/importer.go
Lines 29 to 32 in 1cc5bdc
The only way to correctly determine if a package is Pulumi owned is to check the Package.Publisher == "Pulumi" field. I'm scared of code that relies on that for correctness though.
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
Double checked this for other languages – python and dotnet look fine, but in go, we incorrectly import it as Update: looks like this is an error in the provider schema. We should update that and can ignore it here? @iwahbe |
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
|
Please view the results of the Downstream Codegen Tests Here |
Description
Fixes pulumi/pulumi-yaml#309
Checklist