[K8s Contrib CRD] Duplicate class definitions in generated module

[mruoss]

The example CRD below secifies .first.some_class and .second.some_class. The CRD generator successfully generates a module. However, the module defines the class SomeClass twice which leads to an error upon evaluation.

The CRD

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: foos.example.com
spec:
  group: example.com
  names:
    kind: Foo
    plural: foos
  scope: Namespaced
  versions:
    - name: v1
      schema:
        openAPIV3Schema:
          properties:
            first:
              type: object
              properties:
                some_class: 
                  type: object
                  properties:
                    foo: 
                      type: string
            second:
              type: object
              properties:
                some_class: 
                  type: object
                  properties:
                    bar: 
                      type: string
          type: object
      served: true
      storage: true

Resulting PKL

/// This module was generated from the CustomResourceDefinition at
/// <file:///Users/mruoss/src/community/pkl/playground/example.yaml>.
module com.example.v1.Foo

extends "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/K8sResource.pkl"

import "package://pkg.pkl-lang.org/pkl-k8s/[email protected]#/apimachinery/pkg/apis/meta/v1/ObjectMeta.pkl"

fixed apiVersion: "example.com/v1"

fixed kind: "Foo"

/// Standard object's metadata.
///
/// More info: <https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata>.
metadata: ObjectMeta?

first: First?

second: Second?

class First {
  some_class: SomeClass?
}

class SomeClass {
  foo: String?
}

class Second {
  some_class: SomeClass?
}

class SomeClass { // <-- second definition of SomeClass
  bar: String?
}

[FredrikAugust]

Hi! This happens to me as well. Would it be possible to prefix the class names with the parent(s) to avoid name collisions?

E.g.

spec.template.spec

would become

class Spec

class SpecTemplate

class SpecTemplateSpec

[holzensp]

I don't know the problem space well enough. Are you sure these are always separate classes? Can they be the same class, but with fields that can be omitted? A more Pkly thing could be to have separate modules for First and Second, in which case you already have First.SomeClass "externally," without adding repetitive verbosity to internal use.

[jstrachan]

did this fix it?
#20

[lduvnjak]

I ran into this issue today with trying to convert Elastic Cloud on Kubernetes (ECK) CRDs to PKL.
Here is the command I ran:

pkl eval package://pkg.pkl-lang.org/pkl-pantry/[email protected]#/generate.pkl \
   -m . \
   -p source="https://download.elastic.co/downloads/eck/2.12.1/crds.yaml"

Converting the pkl file back to yaml gives the following error:

# pkl eval Elasticsearch.pkl
–– Pkl Error ––
Duplicate definition of member `Spec`.

142 | class Spec {
      ^^^^^^^^^^
at co.elastic.k8s.elasticsearch.v1.Elasticsearch (file:///root/venv/eck/pkl/test/Elasticsearch.pkl, line 142)

[Avarei]

I have the same problem:

$ pkl eval "package://pkg.pkl-lang.org/pkl-pantry/[email protected]#/generate.pkl" -m . -p source="https://raw.githubusercontent.com/crossplane/crossplane/master/cluster/crds/apiextensions.crossplane.io_compositions.yaml"

$ pkl eval Composition.pkl

–– Pkl Error ––
Duplicate definition of member `String`.

370 | class String {
      ^^^^^^^^^^^^

also the generated class name "String" is not ideal

[Avarei]

I don't know the problem space well enough. Are you sure these are always separate classes? Can they be the same class, but with fields that can be omitted? A more Pkly thing could be to have separate modules for First and Second, in which case you already have First.SomeClass "externally," without adding repetitive verbosity to internal use.

I believe this would be a great solution. Though I think the decision on when and where to create a seperate module should not be made automatically.

Maybe similarly to converters we could decide for which paths a seperate module should be created?

createModules {
  ["foos.v1.example.com"] {
    new List("first")
    new List("second")
  }
}

or a more complex Example the Kubernetes Deployment if it were a CRD:

createModules {
  ["deployments.apps.api.k8s"] {
    [new List("selector")] = "LabelSelector"
    [new List("spec", "template")] = "PodTemplateSpec"
    [new List("spec", "template", "spec")] = "PodSpec"
    [new List("spec", "template", "spec", "imagePullSecrets")] = "LocalObjectReference"
    [new List("spec", "template", "spec", "securityContext", "seLinuxOptions")] = "SELinuxOptions"
    // and so on.
  }
}

So a CRD of a Deployment would contain these Modules:

• PodTemplateSpec
• PodSpec
  • LocalObjectReference
  • SELinuxOptions
  • ... and many more...

[Avarei]

In case it helps someone else:
I just implemented @FredrikAugust's suggestion as a workaround in my local environment.

I replaced

pkl-pantry/packages/k8s.contrib.crd/internal/ModuleGenerator.pkl

Line 307 in 7b16701

let (typeName = determineTypeName(path, path.lastOrNull?.capitalize() ?? "Item", accumulator.values.toSet(), 0))

With

let (typeName = determineTypeName(path, path.fold(List(), (res: List<String>, el: String) -> res.add(el.capitalize())).join(""), accumulator.values.toSet(), 0))

which generates beatiful class names like SpecEnvironmentEnvironmentConfigsEnvironmentConfigSelectorMatchLabelsMatchLabel

[ichekrygin]

@Avarei, have you considered submitting a PR with that ^ fix?