crd2pulumi icon indicating copy to clipboard operation
crd2pulumi copied to clipboard

[nodejs] Type for generated metadata field incorrect

Open ringods opened this issue 3 years ago • 4 comments

Expected behavior

Every Kubernetes object has a metadata field. When generating the NodeJS code for a CRD, I expect the type of the metadata field to be similar to the type of the core Kubernetes classes. E.g. I copied this from kubernetes.core.v1.Service:

    public readonly metadata!: pulumi.Output<outputs.meta.v1.ObjectMeta>;

Current behavior

The metadata field in my generated is this:

    public readonly metadata!: pulumi.Output<ObjectMeta | undefined>;

Since the metadata field always exists on a Kubernetes object, the | undefined part is not correct.

Steps to reproduce

  1. Take a GKE cluster.
  2. kubectl get crd backendconfigs.cloud.google.com -o yaml > backendconfigs.yaml
  3. crd2pulumi --nodejsName backendconfig --nodejsPath ./backendconfig backendconfigs.yaml
  4. Create an instance: const cfg = new backendconfig.cloud.v1.BackendConfig(...)
  5. Try to retrieve the name: cfg.metadata.name
  6. You get an error: Property 'name' does not exist on type 'Output<ObjectMeta | undefined>'.

Context (Environment)

I had to remove the | undefined part manually from the generated field. A lot more of the generated fields have the same | undefined section in the type definition, so there might be problems there as well.

When following the code path, I ended up here in the core Pulumi codegen packages:

https://github.com/pulumi/pulumi/blob/1f16423ede4c9b6266f2df5aa4ef2aa8c79ae54f/pkg/codegen/nodejs/gen.go#L263-L265

Affected feature

ringods avatar Mar 24 '21 07:03 ringods

Update: comparing the type definitions of between my generated code and the k8s Service class made me remove the undefined part as well from:

  • apiVersion
  • kind
  • spec
  • status

ringods avatar Mar 24 '21 07:03 ringods

I took a look at the schema for the k8s provider, and I think crd2pulumi may not be setting the requiredOutputs language option for nodejs. Here's what the schema should look like for a Service resource:

"kubernetes:core/v1:Service": {
    "description": "Service is a named abstraction of software service (for example, mysql) consisting of local port (for example 3306) that the proxy listens on, and the selector that determines which pods will answer requests sent through the proxy.\n\nThis resource waits until its status is ready before registering success\nfor create/update, and populating output properties from the current state of the resource.\nThe following conditions are used to determine whether the resource creation has\nsucceeded or failed:\n\n1. Service object exists.\n2. Related Endpoint objects are created. Each time we get an update, wait 10 seconds\n   for any stragglers.\n3. The endpoints objects target some number of living objects (unless the Service is\n   an \"empty headless\" Service [1] or a Service with '.spec.type: ExternalName').\n4. External IP address is allocated (if Service has '.spec.type: LoadBalancer').\n\nKnown limitations: \nServices targeting ReplicaSets (and, by extension, Deployments,\nStatefulSets, etc.) with '.spec.replicas' set to 0 are not handled, and will time\nout. To work around this limitation, set 'pulumi.com/skipAwait: \"true\"' on\n'.metadata.annotations' for the Service. Work to handle this case is in progress [2].\n\n[1] https://kubernetes.io/docs/concepts/services-networking/service/#headless-services\n[2] https://github.com/pulumi/pulumi-kubernetes/pull/703\n\nIf the Service has not reached a Ready state after 10 minutes, it will\ntime out and mark the resource update as Failed. You can override the default timeout value\nby setting the 'customTimeouts' option on the resource.",
    "properties": {
        "apiVersion": {
            "type": "string",
            "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
            "const": "v1"
        },
        "kind": {
            "type": "string",
            "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
            "const": "Service"
        },
        "metadata": {
            "$ref": "#/types/kubernetes:meta/v1:ObjectMeta",
            "description": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata"
        },
        "spec": {
            "$ref": "#/types/kubernetes:core/v1:ServiceSpec",
            "description": "Spec defines the behavior of a service. https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status"
        },
        "status": {
            "$ref": "#/types/kubernetes:core/v1:ServiceStatus",
            "description": "Most recently observed status of the service. Populated by the system. Read-only. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status"
        }
    },
    "type": "object",
    "language": {
        "nodejs": {
            "requiredOutputs": [
                "apiVersion",
                "kind",
                "metadata",
                "spec",
                "status"
            ]
        }
    }
},

https://github.com/pulumi/crd2pulumi/blob/e64639829235cae038139d0c02589e9b3b835f17/gen/schema.go#L152-L181

lblackstone avatar Mar 24 '21 18:03 lblackstone

@lblackstone I don't understand what you are trying to explain here. For the record: I don't have problems with the core Kubernetes Service class. I have problems with the properties in the generated BackendConfig class, generated from the Google Cloud specific BackendConfig CRD.

ringods avatar Mar 25 '21 07:03 ringods

Sorry, I should have made it clearer that my comment was for whomever works on the fix for this issue. I'm fairly certain this a bug in the schema generated internally by crd2pulumi.

lblackstone avatar Mar 25 '21 14:03 lblackstone

Added to epic https://github.com/pulumi/home/issues/3431

cleverguy25 avatar Aug 09 '24 23:08 cleverguy25