search

kubernetes-client / c

Official C client library for Kubernetes
Apache License 2.0
148 stars 49 forks source link

v1_daemon_set_convertToJSON is returning NULL cJSON object #193

Open hirishh opened 1 year ago

hirishh commented 1 year ago

Hello,

I've debugged this and this function is returning null while parsing the status. I have a simple Daemonset with this status:

status:
  currentNumberScheduled: 1
  numberMisscheduled: 0
  desiredNumberScheduled: 1
  numberReady: 1
  observedGeneration: 1
  updatedNumberScheduled: 1
  numberAvailable: 1

The conversion is failing here: https://github.com/kubernetes-client/c/blob/master/kubernetes/model/v1_daemon_set_status.c#L113

Is there any reason why this code must check the value of the int and fail if it's zero? I don't understand the logic here.

Best,

hirishh

ityuhui commented 1 year ago

I think this is a defect that has never been raised before. Thank you for reporting this.

ityuhui commented 1 year ago

If you want to fix this issue, please note that the code is generated by openapi-generator from here: https://github.com/OpenAPITools/openapi-generator/blob/59ba00e1f3ddb1efa5ae064987cb4e5a6286e8d5/modules/openapi-generator/src/main/resources/C-libcurl/model-body.mustache#L357

hirishh commented 1 year ago

If you want to fix this issue, please note that the code is generated by openapi-generator from here: https://github.com/OpenAPITools/openapi-generator/blob/59ba00e1f3ddb1efa5ae064987cb4e5a6286e8d5/modules/openapi-generator/src/main/resources/C-libcurl/model-body.mustache#L357

Thank you for pointing me out. I tried to had a look. As far as I understand the problem is when a field is an int and also required, indeed the same issue is present for currentNumberScheduled, desiredNumberScheduled and numberReady that are required fields in the deamonset status object.

But I have zero experience with openapi-generator and I'm struggling with the syntax there (for example I don't understand the difference between {{#required}} and {{^required}}, maybe # -> is and ^-> is not) ?

hirishh commented 1 year ago

Maybe this is enough to fix the issue? (L333 - L344)

    {{#required}}
    {{^isEnum}}
    {{^isNumeric}}
    if (!{{{classname}}}->{{{name}}}) {
        goto fail;
    }
    {{/isNumeric}}
    {{/isEnum}}
    {{#isEnum}}
    if ({{projectName}}_{{classVarName}}_{{enumName}}_NULL == {{{classname}}}->{{{name}}}) {
        goto fail;
    }
    {{/isEnum}}
    {{/required}}
ityuhui commented 1 year ago

Your fix is correct. Now it can return a valid cJSON object. We can also add ^isBoolean here to fix boolean type (0).

And for the non-required property, we'd better to add the code too:

    {{^required}}  <-- L345
    {{^isEnum}}
    {{^isNumeric}}
    {{^isBoolean}}
    if({{{classname}}}->{{{name}}}) {
    {{/isBoolean}}
    {{/isNumeric}}
    {{/isEnum}}
    {{#isEnum}}
    if({{{classname}}}->{{{name}}} != {{projectName}}_{{classVarName}}_{{enumName}}_NULL) {
    {{/isEnum}}
    {{/required}}
    {{^required}}  <-- L550
    {{^isNumeric}}
    {{^isBoolean}}
    }
    {{/isBoolean}}
    {{/isNumeric}}
    {{/required}}
hirishh commented 1 year ago

Hello @ityuhui, how we should proceed with this issue?

ityuhui commented 1 year ago

I'll look at the reviewer's comments and talk with him.

k8s-triage-robot commented 10 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

ityuhui commented 10 months ago

/remove-lifecycle stale

k8s-triage-robot commented 7 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

hirishh commented 7 months ago

Hello @ityuhui, I saw you implemented the int* for the client part in the openapi-generator. Is there any plan to implement it also in the model? I do not want to seem pushy, it is not my intention :)

ityuhui commented 7 months ago

This task is on the top of my list. Maybe in a month I'll start.

ityuhui commented 7 months ago

/remove-lifecycle stale

k8s-triage-robot commented 4 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 3 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

ityuhui commented 3 months ago

/remove-lifecycle rotten

k8s-triage-robot commented 6 days ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale