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

[nodejs] Type for generated metadata field incorrect #30

Closed
ringods opened this issue Mar 24, 2021 · 6 comments · Fixed by #143
Closed

[nodejs] Type for generated metadata field incorrect #30

ringods opened this issue Mar 24, 2021 · 6 comments · Fixed by #143
Assignees
Labels
area/codegen Affects quality or correctness of generated code impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed

Comments

@ringods
Copy link
Member

ringods commented Mar 24, 2021

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 ringods added the kind/bug Some behavior is incorrect or out of spec label Mar 24, 2021
@ringods
Copy link
Member Author

ringods commented Mar 24, 2021

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

@lblackstone
Copy link
Member

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"
            ]
        }
    }
},

crd2pulumi/gen/schema.go

Lines 152 to 181 in e646398

func AddType(schema map[string]interface{}, name string, types map[string]pschema.ObjectTypeSpec) {
properties, foundProperties, _ := unstruct.NestedMap(schema, "properties")
description, _, _ := unstruct.NestedString(schema, "description")
schemaType, _, _ := unstruct.NestedString(schema, "type")
required, _, _ := unstruct.NestedStringSlice(schema, "required")
propertySpecs := map[string]pschema.PropertySpec{}
for propertyName := range properties {
propertySchema, _, _ := unstruct.NestedMap(properties, propertyName)
propertyDescription, _, _ := unstruct.NestedString(propertySchema, "description")
defaultValue, _, _ := unstruct.NestedFieldNoCopy(propertySchema, "default")
propertySpecs[propertyName] = pschema.PropertySpec{
TypeSpec: GetTypeSpec(propertySchema, name+strings.Title(propertyName), types),
Description: propertyDescription,
Default: defaultValue,
}
}
// If the type wasn't specified but we found properties, then we can infer that the type is an object
if foundProperties && schemaType == "" {
schemaType = Object
}
types[name] = pschema.ObjectTypeSpec{
Type: schemaType,
Properties: propertySpecs,
Required: required,
Description: description,
}
}

@lblackstone lblackstone added impact/usability Something that impacts users' ability to use the product easily and intuitively bug kind/bug Some behavior is incorrect or out of spec and removed kind/bug Some behavior is incorrect or out of spec bug labels Mar 24, 2021
@ringods
Copy link
Member Author

ringods commented Mar 25, 2021

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

@lblackstone
Copy link
Member

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.

@cleverguy25
Copy link

@mjeffryes mjeffryes removed this from the 0.108 milestone Aug 19, 2024
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Sep 13, 2024
@pulumi-bot
Copy link

This issue has been addressed in PR #143 and shipped in release v1.5.0.

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 impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants