Static deserialization issues

[shawkins]

Is your task related to a problem? Please describe

To eventually remove the static type registration from the KubernetesDeserializer we can't have static entry points that rely on this implicit class resolution.

Describe the solution you'd like

Deprecate and provide alternatives to any static Serialization.unmarshall routines that don't supply the type.

Describe alternatives you've considered

No response

Additional context

No response

[shawkins]

Specifically deprecate

UNMATCHED_FIELD_TYPE_MODULE
jsonMapper()
yamlMapper()
unmarshal(InputStream is)
unmarshal(InputStream is, Map<String, String> parameters)
unmarshal(String str)

If we want the replacement to be available without completely building a client, then we'll need to add some method on the KubernetesClientBuilder. Otherwise with the logic in #3966 it would be easier to obtain the deserializer from the client:

var serdes = client.serialization(); // type and method name tbd
serdes.unmarshal("something yaml");

Under the covers we can use a HandlerInstantiator to associate an appropriate KubernetesDeserializer.

[shawkins]

At a minimum we would introduce here an api for accessing what are now static concerns of serialization:

public interface KubernetesSerialization {

  <T> T unmarshal(InputStream is);

  <T> T unmarshal(InputStream is, Map<String, String> parameters);

  <T> T unmarshal(String str);

  <T> T clone(T resource);

  void setLogUnmatchedFieldWarnings(boolean logWarnings);

  void setRestrictUnmatchedFieldToTemplates(boolean restrictToTemplates);

  default void registerCustomKind(String kind, Class<? extends KubernetesResource> clazz) {
    registerCustomKind(null, kind, clazz);
  }

  void registerCustomKind(String apiVersion, String kind, Class<? extends KubernetesResource> clazz);

  // optionally we could include patchutils methods

  String withoutRuntimeState(KubernetesResource object, Format format, boolean omitStatus);

  String jsonDiff(KubernetesResource current, KubernetesResource updated, boolean omitStatus);

}

Assuming that we want users to obtain this without creating a client, there would likely need to be another factory: KubernetesSerialization.Factory or similar. It would also be obtainable off of the client: client.getKubernetesSerialization()

On the public interface it doesn't seem advisable to directly expose the mapper instances. Those would be available for internal use on the implementation class.

@manusa @rohanKanojia does this seem reasonable?

[shawkins]

After talking with @manusa the usage of registerCustomKind is more limited for the client than I thought. We probably want to avoid using that internally or promoting that for usage by users.

This may require users to deal more with generic resources, which we'd like to offer a KubernetesResource.adapt(class) call.

This would also affect the Serialization.clone method.

[shawkins]

The next steps here is to remove all internal usage of KubernetesDeserializer.registerCustomKind - even if we have a non-static KubernetesDeserializer, that kind of side implicit side-effect is undesirable. The expectation moving forward will be that the class is jandex discoverable. In some situations if it is not, rather than seeing the typed result it would instead be a generickubenetesresource. The places affected by this change will be:

• direct untyped user calls to Serialization
• client.load - if the item is at a top level, the api server will be consulted to create the ResourceHandler. If not found, then an exception will be thrown.
• transitive references to the type via KubernetesResource, or HasMetadata - under a template for example

All other kubernetesclient calls will function as expected because they are typed.

The only issue with KubernetesResource.adapt is the handling of mutability since the real type is not a view, but a converted instance. It would be on the user to after making modifications, set the modified item back:

MyResource resource = something.getResource().adapt(MyResource.class) ;
// do something
something.setResource(resource);

[stale]

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!

[shawkins]

@manusa some next steps:

• refine the KubernetesSerialization interface and where that would be associated with the client.
• ideally allow for the configuration/setting of the objectmappers to the instance associated with the client via the KubernetesClientBuilder - will that help quarkus native scenarios if we use (likely via a copy) their objectmapper?
• decide if we want to continue to allow KubernetesDeserializer.registerCustomKind. If we do not, then I don't think we'll have any additional state that we're associating with the deserializer - all classes must be known via the spi. If we do, then we'll have to make it part of the interface

[shawkins]

Marking as resolved by #4662 - we'll make further adjustments to the KubernetesSerialization from there.