-
Notifications
You must be signed in to change notification settings - Fork 17
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
crd properties with - in name #43
crd properties with - in name #43
Comments
Unless I'm mistaken, this is likely more complicated than "just" fixing the First, we would need to translate both type and property names so that the Pulumi names don't contain However, unless I'm mistaken, we don't currently support having a different Pulumi names for the CRD's type or property names, since we use type tokens and direct names in most places, and as far as I can tell, rely on the Pulumi resource object's names matching at runtime too. (An alternative approach would be to have @henderiw, are you ok sharing your repro file here? |
Quite possibly. I haven't dug deep on the specifics here, but can think of a couple options that may be relevant.
|
Hi, |
i'm also currently running into this problem, with python, rather than go - it's generating code like |
Hi @doy-materialize, would it be possible to get the full CRD so that I can reproduce your problem? |
sure!
|
The fix for the root cause is in pulumi/pulumi. Updating this repo will pull in that fix. Added regression tests and verified that they fail against master.
10738: Go and Python codegen support symbols with hyphens in their names r=iwahbe a=mattolenik # Description This change enables the fix for pulumi/crd2pulumi#43, wherein hyphens `-` were mishandled and led to generation of invalid Go and Python. For example, code such as `func My-Thing() {}` or `class My-Thing`. This change improves the existing casing functions and extracts them to a new subpackage, `cgstrings`. Resolves pulumi/crd2pulumi#43. When this change is merged `go get -u` will fix the crd2pulumi issue. ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [ ] I have added tests that prove my fix is effective or that my feature works Co-authored-by: Matthew Olenik <[email protected]> Co-authored-by: Ian Wahbe <[email protected]>
11049: Go & Python handle hypen in sdk gen (cgstrings take 2) r=iwahbe a=iwahbe <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes pulumi/crd2pulumi#43 ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [X] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [X] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Ian Wahbe <[email protected]>
11049: Go & Python handle hyphen in sdk gen (cgstrings take 2) r=iwahbe a=iwahbe <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes pulumi/crd2pulumi#43 ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [X] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [X] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> Co-authored-by: Ian Wahbe <[email protected]>
11049: Go & Python handle hyphen in sdk gen (cgstrings take 2) r=iwahbe a=iwahbe <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes pulumi/crd2pulumi#43 ## Checklist <!--- Please provide details if the checkbox below is to be left unchecked. --> - [X] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [X] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Service, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. --> 11055: feat(programtest): Enable ProgramTest for Azure-Native split modules r=AaronFriel a=AaronFriel Unblocks Azure Native usage of ProgramTests in Go, resolves #11050's example by permitting usage such as: ```go func genReplace(t *testing.T, subPkg string) string { moduleDir, err := filepath.Abs(fmt.Sprintf("../pulumi-azure-native-sdk/%s", subPkg)) require.NoError(t, err) return fmt.Sprintf("github.com/pulumi/pulumi-azure-native-sdk/%s=%s", subPkg, moduleDir) } func getGoBaseOptions(t *testing.T) integration.ProgramTestOptions { base := getBaseOptions(t) baseGo := base.With(integration.ProgramTestOptions{ Dependencies: map[string]string{ genReplace("aad"), }, }) return baseGo } ``` Resolves #11050 Co-authored-by: Ian Wahbe <[email protected]> Co-authored-by: Aaron Friel <[email protected]>
The fix for the root cause is in pulumi/pulumi. Updating this repo will pull in that fix. Added regression tests and verified that they fail against master.
Hi I am getting this error for C# so the issue does not seam to be fixed. |
Re-opening because this is not fixed. Reproduction:
|
From a very basic overview (running the |
It looks like we'd need the fixes in upstream pu/pu as well for .Net, nodejs, an Java. Note, Java codegen was only recently enabled in this project, but a release has not been cut yet that includes this feature. A local build of crd2pulumi also encountered issues with Java generation. Looking at the history for fixing this, pulumi/pulumi#11049 was merged that closed this issue. However, this upstream pu/pu fix only targeted Python and Go. We'd need something similar for the other languages. |
Created an issue on pu/pu here pulumi/pulumi#15874 I tried to get a minimal repro with just a failing schema, and that did reproduce the issue for me in ts/cs but I didn't see an issue with Java. It's unclear if this is due to my repro not capturing something specific to crd2pulumi; maybe the issue was fixed in a newer pulumi-java release than we're using; or something else. |
Added to epic https://github.com/pulumi/home/issues/3431 |
This is blocked on a change in pulumi/pulumi. We either need Engine support for extension parameterization · pulumi/17059 so we can replace crd2pulumi with parameterization of the Kubernetes provider, or a workaround for Dashed properties generate invalid Node & Dotnet SDKs· pulumi/15874 in codegen. |
Is there anything that we can do to help out with this issue? My workaround is to rename something in the CRD, generate, then rename it in the generated code again. If there's anything I can do to help out, I'd love to! |
I am using a crd which has property names with - in it and the crd2pulumi failed with
input file:
I changed the code as per below and this solves it for me.
The text was updated successfully, but these errors were encountered: