list_event_for_all_namespaces_with_http_info raises ValueError due to None value for event_time if there are events

[haxsaw]

What happened (please include outputs or screenshots):
If you try to fetch events when there aren't any, the above mentioned method returns just fine. But if there are events, then an exception is raised in the internal setter for the 'event_time' property as the value being read in from K8s is None:

        if self.local_vars_configuration.client_side_validation and event_time is None:  # noqa: E501
            raise ValueError("Invalid value for `event_time`, must not be `None`")  # noqa: E501

or, as it happens:

  File ".../python3.8/site-packages/kubernetes/client/api_client.py", line 303, in __deserialize
    return self.__deserialize_model(data, klass)
  File ".../lib/python3.8/site-packages/kubernetes/client/api_client.py", line 641, in __deserialize_model
    instance = klass(**kwargs)
  File ".../lib/python3.8/site-packages/kubernetes/client/models/events_v1_event.py", line 112, in __init__
    self.event_time = event_time
  File ".../lib/python3.8/site-packages/kubernetes/client/models/events_v1_event.py", line 291, in event_time
    raise ValueError("Invalid value for `event_time`, must not be `None`")  # noqa: E501
ValueError: Invalid value for `event_time`, must not be `None`

It may be the case that turning off client_side_validation will fix this, however there seem to be problems in this regard as well. In issue #1353 there was a mention of a breaking change (in release 1.12?) that involved setting up default config where client_side_validation can supposedly be turned off, but setting default configs don't seem to have any effect. Here's the code inside of the __init__() of EventsV1Api (local_vars_configuration is a parameter):

        if local_vars_configuration is None:
            local_vars_configuration = Configuration()
        self.local_vars_configuration = local_vars_configuration

So if you don't pass in a value of local_vars_configuration, you get a brand-new Configuration object in which client_side_validation is True. One would have thought that the assignment to local_vars_configuration should have been:

           local_vars_configuration = Configuration.get_default_copy()

which would have kind of lined up with the notes for release 12.0.1. More curiously, if you have a look into the __init__() for Configuration itself, you find this comment before setting client_side_validation:

        # Disable client side validation
        self.client_side_validation = True

So the comment implies that there shouldn't be any client side validation by default, but the code does the opposite.

This is kind of feeling like a regression.

What you expected to happen:
I was expecting that default behaviour wouldn't raise an exception for fetching regular event data from K8s (or in my case, k3s). My tests don't cover every resource type that could be fetched, so there may be other similar fields lurking within the library.

How to reproduce it (as minimally and precisely as possible):

from kubernetes.client import EventsV1Api
from kubernetes.client import CoreV1Api
from kubernetes import config


def tryit():
    config.load_kube_config(config_file="/etc/rancher/k3s/k3s.yaml")  # the config for k3s
    # you need to first carry out some action that will result in an Event being reported, so we'll make a simple Pod
    p = {'apiVersion': 'v1',
         'kind': 'Pod',
         'metadata': {'labels': {'app': 'myapp'}, 'name': 'myapp-pod'},
         'spec': {'containers': [{'command': ['sh',
                                              '-c',
                                              'echo Hello Kubernetes! && sleep 3600'],
                                  'image': 'busybox',
                                  'name': 'myapp-container'}]}}
    api = CoreV1Api()
    t0 = api.create_namespaced_pod_with_http_info(namespace='default', body=p)
    ev = EventsV1Api()
    t1 = ev.list_event_for_all_namespaces_with_http_info(async_req=False)  # this craters


if __name__ == "__main__":
    tryit()

Anything else we need to know?:

In my dev environment I use k3s, which may be the source for the empty event_time (versions below). Nonetheless, there's clearly something wrong in how default Configurations are being handled.

As this is impacting objects that are being built inside of the K8s Python client itself, it isn't clear that there's a feasible workaround that could involve the user supplying the config object since it appears the default config isn't used and there's no way to pass in a config into list_event_for_all_namespaces_with_http_info().

Environment:

• Kubernetes version (kubectl version): Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}
  Server Version: version.Info{Major:"1", Minor:"20", GitVersion:"v1.20.5+k3s1", GitCommit:"355fff3017b06cde44dbd879408a3a6826fa7125", GitTreeState:"clean", BuildDate:"2021-03-31T06:21:52Z", GoVersion:"go1.15.10", Compiler:"gc", Platform:"linux/amd64"}
• OS (e.g., MacOS 10.13.6): Ubuntu 20.04.3 LTS
• Python version (python --version): 3.810
• Python client version (pip list | grep kubernetes): 19.15.0

[roycaihw]

/assign

[roycaihw]

This is likely similar to kubernetes-client/gen#52. EventTime is marked as required in the openapi spec but kube-apiserver doesn't always set/validate it. The fix is to mark the field as optional in https://github.com/kubernetes/kubernetes/blob/ae550b62da15ca5fe4983c79aaa6b2a39e3e711a/staging/src/k8s.io/api/events/v1/types.go#L41-L42

/unassign
/help

[k8s-ci-robot]

@roycaihw:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

This is likely similar to kubernetes-client/gen#52. EventTime is marked as required in the openapi spec but kube-apiserver doesn't always set/validate it. The fix is to mark the field as optional in https://github.com/kubernetes/kubernetes/blob/ae550b62da15ca5fe4983c79aaa6b2a39e3e711a/staging/src/k8s.io/api/events/v1/types.go#L41-L42

/unassign
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[haxsaw]

If this gets marked as optional will this then propagate to the swagger? And when does that happen; will there be a patch release? How does that address the issue I found of default configs not being applied when no configuration object is supplied to a constructor? Isn't that incorrect? Or else how are we to use the default config?

[roycaihw]

If this gets marked as optional will this then propagate to the swagger?

Yes, the Go type is the source of truth for the swagger in k/k

And when does that happen; will there be a patch release?

Let's fix the issue at HEAD first. Then we can do a patch in this client

So the comment implies that there shouldn't be any client side validation by default, but the code does the opposite

It was a typo in the upstream generator: OpenAPITools/openapi-generator#6850

default configs not being applied when no configuration object is supplied to a constructor

Not sure I follow. The default config is being used here (and used by EventsV1Api here). You can set the default config using Configuration.set_default(c) as https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1201 mentioned

[benholtzmansmith]

I am also encountering this issue. One note on this comment by @roycaihw :

Not sure I follow. The default config is being used here (and used by EventsV1Api here). You can set the default config using Configuration.set_default(c) as https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1201 mentioned

@roycaihw the default config is not being used on the models. For example:

python/kubernetes/client/models/v1beta1_event.py

Line 78 in 33c5ea9

local_vars_configuration = Configuration()

python/kubernetes/client/models/events_v1_event.py

Line 78 in 33c5ea9

local_vars_configuration = Configuration()

When this code is changed to local_vars_configuration = Configuration.get_default_copy() then it is possible to bypass client_side_validation by setting the default configuration's client_side_validation to False

[roycaihw]

@benholtzmansmith Ah that makes sense. @haxsaw Sorry I thought the models would use the config from the API client and misread your comment.

Unable to disable client side validation is a regression. It has been fixed in upstream code generator: OpenAPITools/openapi-generator#8500. We should upgrade the generator version we use to pick up the fix.

[k8s-triage-robot]

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

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

/lifecycle stale