kr8s-org / kr8s

A batteries-included Python client library for Kubernetes that feels familiar for folks who already know how to use kubectl
https://kr8s.org
BSD 3-Clause "New" or "Revised" License
804 stars 43 forks source link

Infer namespace from the marshaled object when reading from files and creating new classes #404

Open leelavg opened 3 months ago

leelavg commented 3 months ago

Which project are you reporting a bug for?

kr8s

What happened?

Irrespective of the resource being marshaled into spec for custom objects, namespace is always set to True while creating new classes. It might be better to infer the scope from the spec.

Pls do note that this might be a double edged sword, as there could be resources in the manifest w/o any namespace and during apply we could also supply namespace so inferring from the spec may not always be correct.

For creation of CRs at cluster scoped supplying namespace only results in warning but we are creating new_class out of it and so I believe surfacing an option during marshaling resources on how to infer namespace might be good enough.

First mentioned in #400

Anything else?

No response

jacobtomlinson commented 3 months ago

Yup totally agree, objects_from_spec(..., allow_unknown_type=True) is broken for non-namespaced objects that don't have a type registered.

I'm nervous about the idea of inferring things based on whether the metadata.namespace field is populated because it's very common to omit that field and pick up the namespace from the current context automatically.

I think the right thing to do would be to make an API call before registering a new object and explicitly look up whether an object should be namespaced or not.

leelavg commented 3 months ago

an API call

  • to? to api-resources?

another question around ns but different context, during the lifetime of a program, if a CRD is deleted and recreated at different scope (like Namespaced to Cluster) the walk func at https://github.com/kr8s-org/kr8s/blob/main/kr8s/_objects.py#L1556 doesn't guarantee returning correctly scopes class.

This is quite restricted to my usecase and is of lesser priority to you, so any pointers that I can take a look into?

jacobtomlinson commented 3 months ago

Yep that's right. We need to get the list of API resources which will tell us if it is namespaced.

during the lifetime of a program, if a CRD is deleted and recreated at different scope

This seems like a wild edge case. In theory resources should never have their scope changed like this. I don't think it's reasonable to expect kr8s to handle this.

leelavg commented 3 months ago

We need to get the list of API resources which will tell us if it is namespaced.

  • ack, that might be going through a cache which could fail a recently created CRD, however there might not be many users/usecases creating CRD as part of the program. So, that should be good enough.

    I don't think it's reasonable to expect kr8s to handle this.

  • agreed, ack.
jacobtomlinson commented 3 months ago

cache which could fail a recently created CRD

Yep very true. #256 covers this.

leelavg commented 3 months ago

With #432, it might be possible to call (async)_lookup_kind to set the namespace in new_class while reading from file isn't it? That could also fix this issue afaiu.

jacobtomlinson commented 3 months ago

I'm not sure I would want to do this in new_class as that would result in API calls every time we want to make a new class. However it might be a good thing to do in object_from_spec().