crd properties with - in name #43
Labels
area/codegen
Affects quality or correctness of generated code
kind/bug
Some behavior is incorrect or out of spec
Milestone
Comments
lblackstone
added
kind/bug
|
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, |
mattolenik
added
the
area/codegen
|
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! |
1 task
mattolenik
pushed a commit
that referenced
this issue
Sep 20, 2022
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.
bors bot
added a commit
to pulumi/pulumi
that referenced
this issue
Oct 11, 2022
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 <molenik@pulumi.com> Co-authored-by: Ian Wahbe <ian@wahbe.com>
3 tasks
bors bot
added a commit
to pulumi/pulumi
that referenced
this issue
Oct 17, 2022
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 <ian@wahbe.com>
bors bot
added a commit
to pulumi/pulumi
that referenced
this issue
Oct 17, 2022
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 <ian@wahbe.com>
bors bot
added a commit
to pulumi/pulumi
that referenced
this issue
Oct 17, 2022
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 <ian@wahbe.com> Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
lblackstone
pushed a commit
that referenced
this issue
Oct 18, 2022
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:
|
lukehoban
added
needs-triage
|
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. |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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: