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

v1_daemon_set_convertToJSON is returning NULL cJSON object #193

Open
hirishh opened this issue Jun 6, 2023 · 13 comments
Open

v1_daemon_set_convertToJSON is returning NULL cJSON object #193

hirishh opened this issue Jun 6, 2023 · 13 comments

Comments

@hirishh
Copy link
Contributor

hirishh commented Jun 6, 2023

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
Copy link
Member

ityuhui commented Jun 6, 2023

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

@ityuhui
Copy link
Member

ityuhui commented Jun 7, 2023

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
Copy link
Contributor Author

hirishh commented Jun 7, 2023

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
Copy link
Contributor Author

hirishh commented Jun 7, 2023

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
Copy link
Member

ityuhui commented Jun 7, 2023

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
Copy link
Contributor Author

hirishh commented Jun 12, 2023

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

@ityuhui
Copy link
Member

ityuhui commented Jun 13, 2023

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

@k8s-triage-robot
Copy link

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 22, 2024
@ityuhui
Copy link
Member

ityuhui commented Jan 23, 2024

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 23, 2024
@k8s-triage-robot
Copy link

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 22, 2024
@hirishh
Copy link
Contributor Author

hirishh commented Apr 23, 2024

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
Copy link
Member

ityuhui commented Apr 23, 2024

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

@ityuhui
Copy link
Member

ityuhui commented Apr 23, 2024

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants