Skip to content
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

Invalid typescript code generated when crd property is hyphenated/pascal case (-) e.g auto-scaler #101

Open
Oyelowo opened this issue Oct 7, 2022 · 4 comments
Labels
area/codegen Affects quality or correctness of generated code kind/bug Some behavior is incorrect or out of spec

Comments

@Oyelowo
Copy link

Oyelowo commented Oct 7, 2022

What happened?

When you generate crd code, the first property (auto-scaler) has issues because there is a hyphen(-) . auto-scaler should either be stringified or camelized i.e "auto-scaler" or autoScaler. Also, hyphen(-) should be removed from the value i.e TidbClusterStatusAutoScalerArgs

export interface TidbClusterStatusArgs {
   auto-scaler?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>; 
   clusterID?: pulumi.Input<string>;
   conditions?: pulumi.Input<pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusConditionsArgs>[]>;
   pd?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPdArgs>;
   pump?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPumpArgs>;
   ticdc?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTicdcArgs>;
   tidb?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTidbArgs>;
   tiflash?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTiflashArgs>;
   tikv?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTikvArgs>;
}

Steps to reproduce

Generate Crd from this:

 sh.exec(`crd2pulumi --nodejsPath ./ https://raw.githubusercontent.com/pingcap/tidb-operator/v1.3.8/manifests/crd.yaml --force`);

The behaviour seems to be coming from this part of the crd if you search auto-scaler:
https://raw.githubusercontent.com/pingcap/tidb-operator/v1.3.8/manifests/crd.yaml

          status:
            properties:
              auto-scaler:
                properties:
                  name:
                    type: string
                  namespace:
                    type: string
                required:
                - name
                - namespace

Expected Behavior

The property should be quoted when it has hyphen i.e "auto-scaler" and the value type(i.e TidbClusterStatusAutoScalerArgs and TidbClusterStatusAutoScalerArgs) should remove the hyphen(-) when concatenating to create the name

export interface TidbClusterStatusArgs {
   "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAutoScalerArgs>; 
   clusterID?: pulumi.Input<string>;
   conditions?: pulumi.Input<pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusConditionsArgs>[]>;
   pd?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPdArgs>;
   pump?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPumpArgs>;
   ticdc?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTicdcArgs>;
   tidb?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTidbArgs>;
   tiflash?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTiflashArgs>;
   tikv?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTikvArgs>;
}


export interface TidbClusterStatusAutoScalerArgs {
   name: pulumi.Input<string>;
   namespace: pulumi.Input<string>;
 }

Actual Behavior

This is an invalid typescript being generated. The property with a hyphen is not being quoted i.e "auto-scaler" and the hyphen(-) is also not removed from the value type which is also an invalid typescript i.e .TidbClusterStatusAuto-ScalerArgs and TidbClusterStatusAuto-ScalerArgs

export interface TidbClusterStatusArgs {
   auto-scaler?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>; 
   clusterID?: pulumi.Input<string>;
   conditions?: pulumi.Input<pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusConditionsArgs>[]>;
   pd?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPdArgs>;
   pump?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusPumpArgs>;
   ticdc?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTicdcArgs>;
   tidb?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTidbArgs>;
   tiflash?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTiflashArgs>;
   tikv?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusTikvArgs>;
}

export interface TidbClusterStatusAuto-ScalerArgs {
   name: pulumi.Input<string>;
   namespace: pulumi.Input<string>;
 }

Output of pulumi about

CLI          
Version      3.41.1
Go Version   go1.19.2
Go Compiler  gc

Plugins
NAME                      VERSION
docker                    3.4.1
kubernetes                3.21.4
kubernetes-ingress-nginx  0.0.9
linode                    3.10.1
nodejs                    unknown
random                    4.8.2

Host     
OS       darwin
Version  12.6
Arch     arm64

This project is written in nodejs: executable='/opt/homebrew/bin/node' version='v18.10.0'

Dependencies:
NAME                              VERSION
eslint-plugin-prettier            4.2.1
@pulumi/kubernetes-ingress-nginx  0.0.9
@types/shelljs                    0.8.11
cli-progress                      3.11.2
find-free-port                    2.0.0
@swc/core                         1.3.4
@types/ramda                      0.28.15
@types/prompt                     1.1.4
chalk                             5.1.0
dotenv                            16.0.3
type-fest                         3.0.0
@types/cli-progress               3.11.0
@types/lodash-es                  4.17.6
json-ts                           1.6.4
prompt                            1.3.0
ts-node                           10.9.1
eslint-config-prettier            8.5.0
eslint-plugin-import              2.26.0
eslint-plugin-promise             6.0.1
jest                              29.1.2
yaml                              2.1.3
@swc/jest                         0.2.23
eslint-plugin-n                   15.3.0
@pulumi/random                    4.8.2
eslint-plugin-unused-imports      2.0.0
inquirer                          9.1.2
json-to-ts                        1.7.0
prettier                          2.7.1
@faker-js/faker                   7.5.0
@types/yargs                      17.0.13
cross-env                         7.0.3
uuid                              9.0.0
get-port                          6.1.2
lodash                            4.17.21
eslint                            8.24.0
json-schema-to-ts                 2.5.5
ramda                             0.28.0
shelljs                           0.8.5
@pulumi/pulumi                    3.40.3
eslint-plugin-unicorn             44.0.1
patch-package                     6.4.7
@pulumi/linode                    3.10.1
@types/inquirer                   9.0.2
bcrypt                            5.0.1
json-schema-to-typescript         11.0.2
lodash-es                         4.17.21
yargs                             17.6.0
zod                               3.19.1
@types/bcrypt                     5.0.0
@types/jest                       29.1.1
@typescript-eslint/parser         5.39.0
@pulumi/docker                    3.4.1
@types/uuid                       8.3.4
@typescript-eslint/eslint-plugin  5.39.0
eslint-config-standard            17.0.0
mock-stdin                        1.0.0
ts-jest                           29.0.3
@pulumi/kubernetes                3.21.4
@pulumi/kubernetesx               0.1.6
typescript                        4.9.1-beta
@types/node                       18.8.2
ts-pattern                        4.0.5

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).

@Oyelowo Oyelowo added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 7, 2022
@lblackstone lblackstone added area/codegen Affects quality or correctness of generated code and removed needs-triage Needs attention from the triage team labels Oct 7, 2022
@Oyelowo
Copy link
Author

Oyelowo commented Oct 9, 2022

A temporary work around if you're using typescript. You could probably adapt to other languages

function main() {
    sh.exec(`crd2pulumi --nodejsPath ${outDir}  ${tempCrdDir}/* --force`);
    const typesPaths = path.join(outDir, 'types');
    const fileMatcher = '*ts';
    sh.exec(`find ${typesPaths} -name "${fileMatcher}"`, { silent: true })
        .trim()
        .split('\n')
        .forEach(path => {
            const data = fs.readFileSync(path, 'utf8');
            const sanitized = sanitizePulumiTypeDefinitions({ data })
            fs.writeFileSync(path, sanitized, 'utf8');
        })

}

export function sanitizePulumiTypeDefinitions({ data }: { data: string; }): string {
    const replacer = (origChar: string) => origChar.split("-").join('');
    return data
        // Wrap quote around the key with `?` coming after the quota e.g
        // auto-scaler?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        // to
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        .replace(/([a-z]+-.*[a-z]-*)(\?)?:/g, '"$1"$2:')
        // Remove the hyphen in the value here e.g
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        // to
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAutoScalerArgs>;
        .replace(/(:.*)(-)(.*;)/g, replacer)
        // Removes hyphen from interface definition e.g
        // export interface TidbClusterStatusAuto-ScalerArgs {
        // to
        // export interface TidbClusterStatusAutoScalerArgs {
        .replace(/(interface.*)(-)(.*{)/g, replacer);
}

Oyelowo added a commit to Oyelowo/oyestack that referenced this issue Oct 9, 2022
Pulumi includes hyphen in the type when a crd property has hyphen:
Can remove after this resolved: pulumi/crd2pulumi#101
@zlepper
Copy link

zlepper commented Jun 8, 2023

Same thing seems to happen with Dotnet for the RabbitMQ charts. I get this code generated:


        [Input("prefetch-count")]
        public Input<int>? Prefetch-count { get; set; }

@UnstoppableMango
Copy link

This has been quite annoying, I've been running some gnarly sed commands to get around it. The regex @Oyelowo provided is a nice improvement, but still would be good to get the underlying issue fixed.

@marcus-sa
Copy link

A temporary work around if you're using typescript. You could probably adapt to other languages

function main() {
    sh.exec(`crd2pulumi --nodejsPath ${outDir}  ${tempCrdDir}/* --force`);
    const typesPaths = path.join(outDir, 'types');
    const fileMatcher = '*ts';
    sh.exec(`find ${typesPaths} -name "${fileMatcher}"`, { silent: true })
        .trim()
        .split('\n')
        .forEach(path => {
            const data = fs.readFileSync(path, 'utf8');
            const sanitized = sanitizePulumiTypeDefinitions({ data })
            fs.writeFileSync(path, sanitized, 'utf8');
        })

}

export function sanitizePulumiTypeDefinitions({ data }: { data: string; }): string {
    const replacer = (origChar: string) => origChar.split("-").join('');
    return data
        // Wrap quote around the key with `?` coming after the quota e.g
        // auto-scaler?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        // to
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        .replace(/([a-z]+-.*[a-z]-*)(\?)?:/g, '"$1"$2:')
        // Remove the hyphen in the value here e.g
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAuto-ScalerArgs>;
        // to
        // "auto-scaler"?: pulumi.Input<inputs.pingcap.v1alpha1.TidbClusterStatusAutoScalerArgs>;
        .replace(/(:.*)(-)(.*;)/g, replacer)
        // Removes hyphen from interface definition e.g
        // export interface TidbClusterStatusAuto-ScalerArgs {
        // to
        // export interface TidbClusterStatusAutoScalerArgs {
        .replace(/(interface.*)(-)(.*{)/g, replacer);
}

replace(/([a-z]+-.*[a-z]-*)(\?)?:/g, '"$1"$2:') didn't work for me.
However replace(/(\s*)([a-zA-Z0-9]+-[a-zA-Z0-9-]+)(\??: [^;]+;)/g, '$1"$2"$3') did

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen Affects quality or correctness of generated code kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

5 participants