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

kopium do not generate struct with nesting additionalProperties #202

Open
chenyuanrun opened this issue Mar 5, 2024 · 8 comments · May be fixed by #212
Open

kopium do not generate struct with nesting additionalProperties #202

chenyuanrun opened this issue Mar 5, 2024 · 8 comments · May be fixed by #212
Labels
bug Something isn't working

Comments

@chenyuanrun
Copy link

This problem could be reproduced with crd cephclusters.ceph.rook.io: https://github.com/rook/rook/blob/v1.13.5/deploy/examples/crds.yaml#L894-L912

kopium generate something like:

#[derive(CustomResource, Serialize, Deserialize, Clone, Debug, JsonSchema)]
#[kube(group = "ceph.rook.io", version = "v1", kind = "CephCluster", plural = "cephclusters")]
#[kube(namespaced)]
#[kube(status = "CephClusterStatus")]
pub struct CephClusterSpec {
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub annotations: Option<BTreeMap<String, CephClusterAnnotations>>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "cephConfig")]
    pub ceph_config: Option<BTreeMap<String, CephClusterCephConfig>>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "cephVersion")]
    pub ceph_version: Option<CephClusterCephVersion>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "cleanupPolicy")]
    pub cleanup_policy: Option<CephClusterCleanupPolicy>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "continueUpgradeAfterChecksEvenIfNotHealthy")]
    pub continue_upgrade_after_checks_even_if_not_healthy: Option<bool>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "crashCollector")]
    pub crash_collector: Option<CephClusterCrashCollector>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub csi: Option<CephClusterCsi>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub dashboard: Option<CephClusterDashboard>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "dataDirHostPath")]
    pub data_dir_host_path: Option<String>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "disruptionManagement")]
    pub disruption_management: Option<CephClusterDisruptionManagement>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub external: Option<CephClusterExternal>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "healthCheck")]
    pub health_check: Option<CephClusterHealthCheck>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub labels: Option<BTreeMap<String, CephClusterLabels>>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "logCollector")]
    pub log_collector: Option<CephClusterLogCollector>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub mgr: Option<CephClusterMgr>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub mon: Option<CephClusterMon>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub monitoring: Option<CephClusterMonitoring>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub network: Option<CephClusterNetwork>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub placement: Option<BTreeMap<String, CephClusterPlacement>>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "priorityClassNames")]
    pub priority_class_names: Option<BTreeMap<String, String>>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "removeOSDsIfOutAndSafeToRemove")]
    pub remove_os_ds_if_out_and_safe_to_remove: Option<bool>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub resources: Option<BTreeMap<String, CephClusterResources>>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub security: Option<CephClusterSecurity>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "skipUpgradeChecks")]
    pub skip_upgrade_checks: Option<bool>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub storage: Option<CephClusterStorage>,
    #[serde(default, skip_serializing_if = "Option::is_none", rename = "waitTimeoutForHealthyOSDInMinutes")]
    pub wait_timeout_for_healthy_osd_in_minutes: Option<i64>,
}

and failed to compile:

error[E0412]: cannot find type `CephClusterAnnotations` in this scope
   --> src/crds/cephclusters_ceph_rook_io.rs:17:46
    |
17  |     pub annotations: Option<BTreeMap<String, CephClusterAnnotations>>,
    |                                              ^^^^^^^^^^^^^^^^^^^^^^
...
846 | pub struct CephClusterMonitoring {
    | -------------------------------- similarly named struct `CephClusterMonitoring` defined here
    |
help: a struct with a similar name exists
    |
17  |     pub annotations: Option<BTreeMap<String, CephClusterMonitoring>>,
    |                                              ~~~~~~~~~~~~~~~~~~~~~
help: you might be missing a type parameter
    |
15  | pub struct CephClusterSpec<CephClusterAnnotations> {
    |                           ++++++++++++++++++++++++

error[E0412]: cannot find type `CephClusterCephConfig` in this scope
  --> src/crds/cephclusters_ceph_rook_io.rs:19:46
   |
19 |     pub ceph_config: Option<BTreeMap<String, CephClusterCephConfig>>,
   |                                              ^^^^^^^^^^^^^^^^^^^^^
...
71 | pub struct CephClusterCephVersion {
   | --------------------------------- similarly named struct `CephClusterCephVersion` defined here
   |
help: a struct with a similar name exists
   |
19 |     pub ceph_config: Option<BTreeMap<String, CephClusterCephVersion>>,
   |                                              ~~~~~~~~~~~~~~~~~~~~~~
help: you might be missing a type parameter
   |
15 | pub struct CephClusterSpec<CephClusterCephConfig> {
   |                           +++++++++++++++++++++++

error[E0412]: cannot find type `CephClusterLabels` in this scope
  --> src/crds/cephclusters_ceph_rook_io.rs:41:41
   |
41 |     pub labels: Option<BTreeMap<String, CephClusterLabels>>,
   |                                         ^^^^^^^^^^^^^^^^^ not found in this scope
   |
help: you might be missing a type parameter
   |
15 | pub struct CephClusterSpec<CephClusterLabels> {
   |                           +++++++++++++++++++

error[E0412]: cannot find type `CephClusterAnnotations` in this scope
   --> src/crds/cephclusters_ceph_rook_io.rs:17:46
    |
17  |     pub annotations: Option<BTreeMap<String, CephClusterAnnotations>>,
    |                                              ^^^^^^^^^^^^^^^^^^^^^^ help: a struct with a similar name exists: `CephClusterMonitoring`
...
846 | pub struct CephClusterMonitoring {
    | -------------------------------- similarly named struct `CephClusterMonitoring` defined here

error[E0412]: cannot find type `CephClusterCephConfig` in this scope
  --> src/crds/cephclusters_ceph_rook_io.rs:19:46
   |
19 |     pub ceph_config: Option<BTreeMap<String, CephClusterCephConfig>>,
   |                                              ^^^^^^^^^^^^^^^^^^^^^ help: a struct with a similar name exists: `CephClusterCephVersion`
...
71 | pub struct CephClusterCephVersion {
   | --------------------------------- similarly named struct `CephClusterCephVersion` defined here

error[E0412]: cannot find type `CephClusterLabels` in this scope
  --> src/crds/cephclusters_ceph_rook_io.rs:41:41
   |
41 |     pub labels: Option<BTreeMap<String, CephClusterLabels>>,
   |                                         ^^^^^^^^^^^^^^^^^ not found in this scope

For more information about this error, try `rustc --explain E0412`.
error: could not compile `operator` (bin "operator") due to 6 previous errors
@chenyuanrun
Copy link
Author

These are the fields that do not generate:

annotations:
  additionalProperties:
    additionalProperties:
      type: string
    type: object
  nullable: true
  type: object
  x-kubernetes-preserve-unknown-fields: true
cephConfig:
  additionalProperties:
    additionalProperties:
      type: string
    type: object
  nullable: true
  type: object
labels:
  additionalProperties:
    additionalProperties:
      type: string
    type: object
  nullable: true
  type: object
  x-kubernetes-preserve-unknown-fields: true

All of those crd have a nesting additionalProperties

@chenyuanrun
Copy link
Author

Here is where the annotation crd field generated from: https://github.com/rook/rook/blob/master/pkg/apis/ceph.rook.io/v1/annotations.go#L23-L33

// AnnotationsSpec is the main spec annotation for all daemons
// +kubebuilder:pruning:PreserveUnknownFields
// +nullable
type AnnotationsSpec map[KeyType]Annotations


// Annotations are annotations
type Annotations map[string]string

It should be translated to something like BTreeMap<String, BTreeMap<String, String>> .

@clux clux added the bug Something isn't working label Mar 5, 2024
@chenyuanrun
Copy link
Author

This is the minimal crd to reproduce this issue:

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: v0.14.0
  name: cephclusters.ceph.rook.io
spec:
  group: ceph.rook.io
  names:
    kind: CephCluster
    listKind: CephClusterList
    plural: cephclusters
    singular: cephcluster
  scope: Namespaced
  versions:
    - name: v1
      schema:
        openAPIV3Schema:
          properties:
            apiVersion:
              type: string
            kind:
              type: string
            metadata:
              type: object
            spec:
              properties:
                annotations:
                  additionalProperties:
                    additionalProperties:
                      type: string
                    type: object
                  nullable: true
                  type: object
                  x-kubernetes-preserve-unknown-fields: true
                cephConfig:
                  additionalProperties:
                    additionalProperties:
                      type: string
                    type: object
                  nullable: true
                  type: object
                cephVersion:
                  nullable: true
                  properties:
                    allowUnsupported:
                      type: boolean
                    image:
                      type: string
                    imagePullPolicy:
                      enum:
                        - IfNotPresent
                        - Always
                        - Never
                        - ""
                      type: string
                  type: object
                cleanupPolicy:
                  nullable: true
                  properties:
                    allowUninstallWithVolumes:
                      type: boolean
                    confirmation:
                      nullable: true
                      pattern: ^$|^yes-really-destroy-data$
                      type: string
                    sanitizeDisks:
                      nullable: true
                      properties:
                        dataSource:
                          enum:
                            - zero
                            - random
                          type: string
                        iteration:
                          format: int32
                          type: integer
                        method:
                          enum:
                            - complete
                            - quick
                          type: string
                      type: object
                  type: object
                continueUpgradeAfterChecksEvenIfNotHealthy:
                  type: boolean
                crashCollector:
                  nullable: true
                  properties:
                    daysToRetain:
                      type: integer
                    disable:
                      type: boolean
                  type: object
              type: object

@chenyuanrun
Copy link
Author

I think this field:

annotations:
  additionalProperties:
    additionalProperties:
      type: string
    type: object
  nullable: true
  type: object
  x-kubernetes-preserve-unknown-fields: true

may be translated to:

#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct CephClusterAnnotations {
    #[serde(flatten)]
    additional_properties: BTreeMap<String, String>,
}

@chenyuanrun
Copy link
Author

Run kopium with RUST_LOG=trace:

[2024-03-06T00:13:45Z DEBUG kopium] schema: {
      "properties": {
        "apiVersion": {
          "type": "string"
        },
        "kind": {
          "type": "string"
        },
        "metadata": {
          "type": "object"
        },
        "spec": {
          "properties": {
            "annotations": {
              "additionalProperties": {
                "additionalProperties": {
                  "type": "string"
                },
                "type": "object"
              },
              "nullable": true,
              "type": "object",
              "x-kubernetes-preserve-unknown-fields": true
            },
            "cephConfig": {
              "additionalProperties": {
                "additionalProperties": {
                  "type": "string"
                },
                "type": "object"
              },
              "nullable": true,
              "type": "object"
            },
            "cephVersion": {
              "nullable": true,
              "properties": {
                "allowUnsupported": {
                  "type": "boolean"
                },
                "image": {
                  "type": "string"
                },
                "imagePullPolicy": {
                  "enum": [
                    "IfNotPresent",
                    "Always",
                    "Never",
                    ""
                  ],
                  "type": "string"
                }
              },
              "type": "object"
            },
            "cleanupPolicy": {
              "nullable": true,
              "properties": {
                "allowUninstallWithVolumes": {
                  "type": "boolean"
                },
                "confirmation": {
                  "nullable": true,
                  "pattern": "^$|^yes-really-destroy-data$",
                  "type": "string"
                },
                "sanitizeDisks": {
                  "nullable": true,
                  "properties": {
                    "dataSource": {
                      "enum": [
                        "zero",
                        "random"
                      ],
                      "type": "string"
                    },
                    "iteration": {
                      "format": "int32",
                      "type": "integer"
                    },
                    "method": {
                      "enum": [
                        "complete",
                        "quick"
                      ],
                      "type": "string"
                    }
                  },
                  "type": "object"
                }
              },
              "type": "object"
            },
            "continueUpgradeAfterChecksEvenIfNotHealthy": {
              "type": "boolean"
            },
            "crashCollector": {
              "nullable": true,
              "properties": {
                "daysToRetain": {
                  "type": "integer"
                },
                "disable": {
                  "type": "boolean"
                }
              },
              "type": "object"
            }
          },
          "type": "object"
        }
      }
    }
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] not recursing into ignored apiVersion
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] not recursing into ignored kind
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] not recursing into ignored metadata
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] Generating struct for Spec (under CephClusterSpec)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got additional: {"additionalProperties":{"type":"string"},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member annotations of type BTreeMap<String, CephClusterSpecAnnotations>
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got additional: {"additionalProperties":{"type":"string"},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member cephConfig of type BTreeMap<String, CephClusterSpecCephConfig>
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member cephVersion of type CephClusterSpecCephVersion
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member cleanupPolicy of type CephClusterSpecCleanupPolicy
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member continueUpgradeAfterChecksEvenIfNotHealthy of type bool
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member crashCollector of type CephClusterSpecCrashCollector
[2024-03-06T00:13:45Z WARN  kopium::analyzer] not generating type Annotations - using object map
[2024-03-06T00:13:45Z WARN  kopium::analyzer] not generating type CephConfig - using object map
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] Generating struct for CephVersion (under CephClusterSpecCephVersion)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member allowUnsupported of type bool
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member image of type String
[2024-03-06T00:13:45Z TRACE kopium::analyzer] got enum string: {"nullable":true,"properties":{"allowUnsupported":{"type":"boolean"},"image":{"type":"string"},"imagePullPolicy":{"enum":["IfNotPresent","Always","Never",""],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member imagePullPolicy of type CephClusterSpecCephVersionImagePullPolicy
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into allowUnsupported ('boolean' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into image ('string' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] analyzing enum {"nullable":true,"properties":{"allowUnsupported":{"type":"boolean"},"image":{"type":"string"},"imagePullPolicy":{"enum":["IfNotPresent","Always","Never",""],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("IfNotPresent"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member IfNotPresent
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("Always"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member Always
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("Never"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member Never
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String(""))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member 
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] Generating struct for CleanupPolicy (under CephClusterSpecCleanupPolicy)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member allowUninstallWithVolumes of type bool
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member confirmation of type String
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member sanitizeDisks of type CephClusterSpecCleanupPolicySanitizeDisks
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into allowUninstallWithVolumes ('boolean' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into confirmation ('string' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] Generating struct for SanitizeDisks (under CephClusterSpecCleanupPolicySanitizeDisks)
[2024-03-06T00:13:45Z TRACE kopium::analyzer] got enum string: {"nullable":true,"properties":{"dataSource":{"enum":["zero","random"],"type":"string"},"iteration":{"format":"int32","type":"integer"},"method":{"enum":["complete","quick"],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member dataSource of type CephClusterSpecCleanupPolicySanitizeDisksDataSource
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member iteration of type i32
[2024-03-06T00:13:45Z TRACE kopium::analyzer] got enum string: {"nullable":true,"properties":{"dataSource":{"enum":["zero","random"],"type":"string"},"iteration":{"format":"int32","type":"integer"},"method":{"enum":["complete","quick"],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member method of type CephClusterSpecCleanupPolicySanitizeDisksMethod
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] analyzing enum {"nullable":true,"properties":{"dataSource":{"enum":["zero","random"],"type":"string"},"iteration":{"format":"int32","type":"integer"},"method":{"enum":["complete","quick"],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("zero"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member zero
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("random"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member random
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into iteration ('integer' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] analyzing enum {"nullable":true,"properties":{"dataSource":{"enum":["zero","random"],"type":"string"},"iteration":{"format":"int32","type":"integer"},"method":{"enum":["complete","quick"],"type":"string"}},"type":"object"}
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("complete"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member complete
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] got enum JSON(String("quick"))
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with enum member quick
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into continueUpgradeAfterChecksEvenIfNotHealthy ('boolean' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] Generating struct for CrashCollector (under CephClusterSpecCrashCollector)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member daysToRetain of type i64
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] with optional member disable of type bool
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into daysToRetain ('integer' is not a container)
[2024-03-06T00:13:45Z DEBUG kopium::analyzer] ..not recursing into disable ('boolean' is not a container)

@chenyuanrun
Copy link
Author

I will manually patch the generated files with

// Patched by xxx until https://github.com/kube-rs/kopium/issues/202 is fixed.
// Do not edit it manually.

#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema)]
pub struct CephClusterAnnotations {
    #[serde(flatten)]
    additional_properties: BTreeMap<String, String>,
}

#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema)]
pub struct CephClusterCephConfig {
    #[serde(flatten)]
    additional_properties: BTreeMap<String, String>,
}

#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema)]
pub struct CephClusterLabels {
    #[serde(flatten)]
    additional_properties: BTreeMap<String, String>,
}

before this issue is fixed.

clux added a commit that referenced this issue Mar 16, 2024
@clux clux linked a pull request Mar 16, 2024 that will close this issue
@clux
Copy link
Member

clux commented Mar 16, 2024

Thanks for this bug report!

This seems like it will be a bug yes. The part of the analyzer where this ends up is probably a bit too primitive. Have added some premature failing tests in #212 based on your yaml snippets.

Do you have examples of actual yaml containing these nested annotations/labels structs? I am unsure whether to do something with flatten in the general kopium case or whether to nest maps.

@clux clux linked a pull request Mar 16, 2024 that will close this issue
@chenyuanrun
Copy link
Author

@clux Sorry for my late reply. There is a example in rook repo: https://github.com/rook/rook/blob/02a552ce00c4f61703e09e5e3ad8aaca14f724ce/deploy/examples/cluster-test.yaml#L49

It seems to be something like dict[str, dict[str, str]] .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants