Cannot import Istio resource due to duplicate field

[Sanhajio]

Description of the bug:

When using cdk8s import --language python /dev/stdin on istio's virtual service crd on python I get an import error

cat virtual_service_crd.json | cdk8s import --language python /dev/stdin
Importing resources, this may take a few moments...
networking.istio.io
  networking.istio.io/virtualservice
NOTE: Temp directory retained due to an error: /tmp/temp-Cae5Ex
Error: jsii compilation failed with non-zero exit code: 1
  | [2021-08-24T18:02:42.216] [ERROR] jsii/compiler - Compilation errors prevented the JSII assembly from being created
  | warning JSII3: There is no "README.md" file. It is required in order to generate valid PyPI (Python) packages.
  | io.istio.networking.ts:203:12 - error TS2300: Duplicate identifier 'mirrorPercent'.
  | 203   readonly mirrorPercent?: number;
  |                ~~~~~~~~~~~~~
  | io.istio.networking.ts:217:12 - error TS2300: Duplicate identifier 'mirrorPercent'.
  | 217   readonly mirrorPercent?: number;
  |                ~~~~~~~~~~~~~
  +----------------------------------------------------------------------------------
  | Command: /home/hf/.nvm/versions/node/v16.5.0/lib/node_modules/cdk8s-cli/node_modules/jsii/bin/jsii --silence-warnings reserved-word
  | Workdir: /tmp/temp-Cae5Ex
  +----------------------------------------------------------------------------------
    at newError (/home/hf/.nvm/versions/node/v16.5.0/lib/node_modules/cdk8s-cli/node_modules/jsii-srcmak/lib/util.js:38:20)
    at ChildProcess.<anonymous> (/home/hf/.nvm/versions/node/v16.5.0/lib/node_modules/cdk8s-cli/node_modules/jsii-srcmak/lib/util.js:55:29)
    at Object.onceWrapper (node:events:514:26)
    at ChildProcess.emit (node:events:394:28)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:290:12)

I get this error only using cdk8s import python language, I don't get it on This is because on the virtual service crd, there are two fields named almost the same

                                                         "mirrorPercent": {
 +----------------  3 lines: "description": "Percentage of the traffic to be mirrored by the `mirror` field.",---------------------------------------
                                                         },
                                                         "mirrorPercentage": {
 +----------------  8 lines: "description": "Percentage of the traffic to be mirrored by the `mirror` field.",---------------------------------------
                                                         },
                                                         "mirror_percent": {
 +----------------  3 lines: "description": "Percentage of the traffic to be mirrored by the `mirror` field.",---------------------------------------
                                                         },

it ends up being duplicated in the code generation:

export interface VirtualServiceSpecHttp {
  /**  
   * Percentage of the traffic to be mirrored by the `mirror` field.
   *
   * @schema VirtualServiceSpecHttp#mirrorPercent
   */
  readonly mirrorPercent?: number;

  /**  
   * Percentage of the traffic to be mirrored by the `mirror` field.
   *
   * @schema VirtualServiceSpecHttp#mirrorPercentage
   */
  readonly mirrorPercentage?: VirtualServiceSpecHttpMirrorPercentage;

  /**  
   * Percentage of the traffic to be mirrored by the `mirror` field.
   *
   * @schema VirtualServiceSpecHttp#mirror_percent
   */
  readonly mirrorPercent?: number;

a workaround I found is to delete the mirror_percent in the json crd definition

I think duplicate fields could be safely ignored, as the issue does not come from the jsii compiler but from the crd definition, IMHO

Reproduction Steps:

1. Get the istio crds somehow
2. run cdk8s import --language python

---

This is :bug: Bug Report

[Chriscbr]

Hmm, that's slightly weird, I wonder why they have both variations in the spec...

I think ignoring duplicate fields (and instead just printing a warning) for cases like these makes more sense than completely failing the import command, especially since users are usually not the authors of the specs.

[Sanhajio]

I opened an issue in istio, it seems to be a legacy field that is deprecated but kept for backward compatibility. Kubernetes API standard is to write fields in camel case,

[github-actions[bot]]

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

[ajshock]

I just observed this doing the same thing for the virtualservice CRD, using --language go

[saquibmian]

I've also encountered the same thing. I think duplicate fields could safely be ignored, and we should prefer camelCase ones as they are less likely to be deprecated.

[github-actions[bot]]

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

[github-actions[bot]]

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

[iliapolo]

I'm not entirely comfortable with ignoring properties, but maybe in case of a duplicate, we can include the second property in its original casing. Requires a bit of research.

[KrishnaSindhur]

Hi is this issue resolved? duplicate error due to different casing?

[bderrly]

The istio VirtualService CRD is still causing the same errors on import (cdk8s-cli 2.2.83) due to the duplicate field names. What is the plan to address this?

[iliapolo]

Currently this would require some pre-processing before executing cdk8s import. We will consider putting this on our near term roadmap as looks like this issue is gaining popularity.

[bderrly]

@iliapolo, has this made it to the roadmap?

[aikoven]

We're currently using a preprocess step in a form of this patch-crds.js script:

const fs = require('fs');
const yaml = require('yaml');

const filePath = process.argv[2];

const crds = yaml.parse(fs.readFileSync(filePath, 'utf-8'));

/**
 * Patch postgresql
 */
{
  const crd = crds.items.find(
    crd => crd.metadata.name === 'postgresqls.acid.zalan.do',
  );

  const {properties} =
    crd.spec.versions[0].schema.openAPIV3Schema.properties.spec;

  delete properties.init_containers;
  delete properties.pod_priority_class_name;

  properties.users.additionalProperties.items.enum = new Set(
    properties.users.additionalProperties.items.enum.map(item =>
      item.toLowerCase(),
    ),
  );
}

fs.writeFileSync(filePath, yaml.stringify(crds));

We call it in package.json script like this:

{
  "scripts": {
    "import:k8s": "cdk8s import --language typescript --output src/imports k8s",
    "import:crds": "kubectl get crds -o yaml > src/imports/crds.yaml && node patch-crds.js src/imports/crds.yaml && cdk8s import --language typescript --output src/imports src/imports/crds.yaml",
    "import": "rimraf src/imports && npm run import:k8s && npm run import:crds"
  }
}

[iliapolo]

@bderrly We are actually discussing this internally these days to see if there's a reasonable thing cdk8s can do here.