kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

Use one resource annotation

Open shawkins opened this issue 3 years ago • 7 comments

Is your enhancement related to a problem? Please describe

We have separate annotations for things like group, kind, plural, singular, and version.

Describe the solution you'd like

For those that don't auto-generate their model, it would be better if all of the single valued annotations could easily be combined to a single annotation:

Resource(group="x", kind="y" ...)

You could even include builder=Something.class, and list=Something.class - if we think that the conventions may not hold at some point.

Describe alternatives you've considered

No response

Additional context

No response

shawkins avatar May 22 '22 19:05 shawkins

This would make the code look more organized, and reduce the code-base by a few hundred lines.

However, won't this break our current model backwards compatibility support (i.e. consuming a model from v5.9 in v5.12)?

manusa avatar Jun 06 '22 10:06 manusa

However, won't this break our current model backwards compatibility support (i.e. consuming a model from v5.9 in v5.12)?

You'd have to keep and deprecate the old style for a couple of releases.

shawkins avatar Jun 06 '22 19:06 shawkins

A full proposal would look like:

public @interface Resource {
  
  String group();
  
  String kind() default "";  // empty would default to class name
  
  String plural() default ""; // empty would default to plualize kind
  
  boolean namespaced() default true;
  
  String version();
}

This would also mean deprecating the Namespaced interface - it won't be needed once this annotation is required.

The only sticking point are how this meshes with the crd related annotations - in particular Version can accept storage and served for that purpose. The simplest thing is that you could just flatten these attributes into the Resource annotation as well. The advantages of doing this are:

  • is much more straight-forward for those starting with the object model
  • is more straight-forward for annotation scanning. As of now you need to scan for Version - as that's the only one that's effectively required
  • generated code will be a little simpler

On the other hand it will be a fair amount of work and introduce some confusion while both sets of annotations are supported.

On versions, an additional thought - for multi-version support from a single object model, we'd need to support something like:

client.resources(Something.class, "v2") - under the covers we'd create a ResourceDefinitionContext off of Something, but change the version to v2. I'm not sure if this is a common enough use case to add another entry method.

shawkins avatar Jul 15 '22 19:07 shawkins

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

stale[bot] avatar Oct 14 '22 00:10 stale[bot]

A full proposal would look like:

public @interface Resource {
  
  String group();
  
  String kind() default "";  // empty would default to class name
  
  String plural() default ""; // empty would default to plualize kind
  
  boolean namespaced() default true;
  
  String version();
}

This would also mean deprecating the Namespaced interface - it won't be needed once this annotation is required.

This proposal sounds good. However, I'd defer this changes to a future 7.x (next major) release where architectural breaking changes would fit better.

manusa avatar Nov 28 '22 08:11 manusa

This is something we might want to consider for version 7.0

manusa avatar Mar 07 '24 14:03 manusa

One thing to consider here is that it would be nice to be able to determine whether a resource is namespaced or not without the annotation (as is the case at the moment).

metacosm avatar Apr 10 '24 13:04 metacosm