allansun/kubernetes-php-client

$isRawData is set to true for Status object in version 1.14, should be false (or not present)

Closed this issue · 6 comments

piwi91 commented

After #6 I have a new issue. The rawData in the AbstractModel isn't parsed into the object properties anymore.

var_dump from an object:

object(Kubernetes\Model\Io\K8s\Apimachinery\Pkg\Apis\Meta\V1\Status)#729 (10) {
  ["apiVersion"]=>
  string(2) "v1"
  ["code"]=>
  NULL
  ["details"]=>
  NULL
  ["kind"]=>
  string(6) "Status"
  ["message"]=>
  NULL
  ["metadata"]=>
  NULL
  ["reason"]=>
  NULL
  ["status"]=>
  NULL
  ["isRawObject":protected]=>
  bool(true)
  ["rawData":protected]=>
  array(8) {
    ["kind"]=>
    string(6) "Status"
    ["apiVersion"]=>
    string(2) "v1"
    ["metadata"]=>
    array(0) {
    }
    ["status"]=>
    string(7) "Failure"
    ["message"]=>
    string(48) "the server could not find the requested resource"
    ["reason"]=>
    string(8) "NotFound"
    ["details"]=>
    array(0) {
    }
    ["code"]=>
    int(404)
  }
}

I expected:

object(Kubernetes\Model\Io\K8s\Apimachinery\Pkg\Apis\Meta\V1\Status)#729 (10) {
  ["apiVersion"]=>
  string(2) "v1"
  ["code"]=>
  int(404)
  ["details"]=>
  array(0) {
  }
  ["kind"]=>
  string(6) "Status"
  ["message"]=>
  string(48) "the server could not find the requested resource"
  ["metadata"]=>
  array(0) {
  }
  ["reason"]=>
  string(8) "NotFound"
  ["status"]=>
  string(7) "Failure"
  ["isRawObject":protected]=>
  bool(false)
}
piwi91 commented

I've found the issue. In the previously generated version (1.9.11), the $isRawData property was false and not present in the Status object as you can see here: https://github.com/allansun/kubernetes-php-client/blob/v1.9.11/src/Model/Io/K8s/Apimachinery/Pkg/Apis/Meta/V1/Status.php

In the version 1.14.0 the $isRawData is present in the Status object and set to true, it should be false though.

allansun commented

After accepting your PR shall i close this issue?

Thanks very much for your contribution

piwi91 commented

No, this is another issue in your code generator. As you can see in version 1.14.0 (https://github.com/allansun/kubernetes-php-client/blob/v1.14.0/src/Model/Io/K8s/Apimachinery/Pkg/Apis/Meta/V1/Status.php) the property itself is declared and set to true. This will give you trouble using the Status object because the rawData itself is never parsed into the object.

allansun commented

@piwi91 I see, however this seems to be caused by the swagger content changed by Kubernetes.

In my generator, I check if there was a 'type' field defined in a definition:

kubernetes-php-client/dev_src/Code/Model.php

Lines 58 to 60 in 4a2b5a0

if ($SchemaObject->type) {
$this->ClassGenerator->addProperty('isRawObject', true, PropertyGenerator::FLAG_PROTECTED);
}

This worked fine until v1.14.

As seen in version v1.13.5:

{
"io.k8s.apimachinery.pkg.apis.meta.v1.Status": {
    "description": "Status is a return value for calls that don't return other objects.",
    "properties": {
     "apiVersion": {
      "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/api-conventions.md#resources",
      "type": "string"
     },
     "code": {
      "description": "Suggested HTTP return code for this status, 0 if not set.",
      "type": "integer",
      "format": "int32"
     },
     "details": {
      "description": "Extended data associated with the reason.  Each reason may define its own extended details. This field is optional and the data returned is not guaranteed to conform to any schema except that defined by the reason type.",
      "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.StatusDetails"
     },
     "kind": {
      "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/api-conventions.md#types-kinds",
      "type": "string"
     },
     "message": {
      "description": "A human-readable description of the status of this operation.",
      "type": "string"
     },
     "metadata": {
      "description": "Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds",
      "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ListMeta"
     },
     "reason": {
      "description": "A machine-readable description of why this operation is in the \"Failure\" status. If this value is empty there is no information available. A Reason clarifies an HTTP status code but does not override it.",
      "type": "string"
     },
     "status": {
      "description": "Status of the operation. One of: \"Success\" or \"Failure\". More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status",
      "type": "string"
     }
    },
    "x-kubernetes-group-version-kind": [
     {
      "group": "",
      "kind": "Status",
      "version": "v1"
     }
    ]
   }
}

However since v1.14.0 it seems all definitions have a "type":"object" defined

{
"io.k8s.apimachinery.pkg.apis.meta.v1.Status": {
      "description": "Status is a return value for calls that don't return other objects.",
      "properties": {
        "apiVersion": {
          "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/api-conventions.md#resources",
          "type": "string"
        },
        "code": {
          "description": "Suggested HTTP return code for this status, 0 if not set.",
          "format": "int32",
          "type": "integer"
        },
        "details": {
          "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.StatusDetails",
          "description": "Extended data associated with the reason.  Each reason may define its own extended details. This field is optional and the data returned is not guaranteed to conform to any schema except that defined by the reason type."
        },
        "kind": {
          "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/api-conventions.md#types-kinds",
          "type": "string"
        },
        "message": {
          "description": "A human-readable description of the status of this operation.",
          "type": "string"
        },
        "metadata": {
          "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ListMeta",
          "description": "Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds"
        },
        "reason": {
          "description": "A machine-readable description of why this operation is in the \"Failure\" status. If this value is empty there is no information available. A Reason clarifies an HTTP status code but does not override it.",
          "type": "string"
        },
        "status": {
          "description": "Status of the operation. One of: \"Success\" or \"Failure\". More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status",
          "type": "string"
        }
      },
      "type": "object",
      "x-kubernetes-group-version-kind": [
        {
          "group": "",
          "kind": "Status",
          "version": "v1"
        }
      ]
    }
}

I don't know the reason for this change behind, but will look into it

allansun commented

@piwi91 Sorry about this long waited updated. It was such a simple change to the code and it should be working now. I have regenerated the code against version v1.14.1 and it seems to be working now. Do you want to have a try?

piwi91 commented

@allansun I'm not able to test your change at this moment so I can only review your code and this looks good to me :) Thx!