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

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

Open
leelavg opened this issue Jun 11, 2024 · 7 comments
Labels
bug Something isn't working kr8s

Comments

@leelavg
Copy link
Contributor

leelavg commented Jun 11, 2024

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

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

leelavg commented Jun 11, 2024

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

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

leelavg commented Jun 11, 2024

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

cache which could fail a recently created CRD

Yep very true. #256 covers this.

@jacobtomlinson jacobtomlinson changed the title Refer namespace from the marshaled object when reading from files and creating new classes Infer namespace from the marshaled object when reading from files and creating new classes Jun 28, 2024
@leelavg
Copy link
Contributor Author

leelavg commented Jul 5, 2024

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

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().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kr8s
Projects
None yet
Development

No branches or pull requests

2 participants