istio Timestamp type should be mapped to String type in java

[yimoucao]

Describe the bug

in istio generators, there's such code block that converts istio Timestamp to Long

manualTypeMap := map[reflect.Type]string{
		reflect.TypeOf(types.BoolValue{}):   "java.lang.Boolean",
		reflect.TypeOf(types.DoubleValue{}): "java.lang.Double",
		reflect.TypeOf(types.Duration{}):    "java.lang.String",
		reflect.TypeOf(types.Timestamp{}):   "java.lang.Long",
		reflect.TypeOf(types.Int32Value{}):  "java.lang.Integer",
		reflect.TypeOf(types.UInt32Value{}): "java.lang.Integer",
	}

according to istio doc, it will be marshaled to string in RFC-3339 string and one example is lastTransitionTime on istio offical site

strings can't be deserialized into java Long type

Fabric8 Kubernetes Client version

6.0.0

Steps to reproduce

copy yaml

  - lastProbeTime: null
    lastTransitionTime: "2019-12-26T22:06:34Z"
    message: "61/122 complete"
    reason: "stillPropagating"
    status: "False"
    type: Reconciled

and do deserialization with type IstioCondition

Expected behavior

successful deserialization

Runtime

other (please specify in additional context)

Kubernetes API Server version

other (please specify in additional context)

Environment

macOS

Fabric8 Kubernetes Client Logs

enterprise-customized k8s

Additional context

No response

[manusa]

Hi @yimoucao, Do you want to contribute a fix for this?

[yimoucao]

Hi @yimoucao, Do you want to contribute a fix for this?

@manusa yes I'd love to. so you confirmed it is indeed a bug? I am no expert in istio and don't know what was considered to map it to Long by original author

[manusa]

If the provided code reproduces an issue, i.e. the provided snippet (or any other resource retrieved from the cluster) doesn't get deserialized properly, this should be treated as a bug and fixed.

The right approach would be to deserialize the field as a Java Timestamp-compatible type via a Jackson converter/deserializer module, instead of a regular Java String type (which is not very useful for users, especially if they intend to perform operations on the value of the field).

However, most of the timestamps we deal with in the client models are currently being deserialized as Strings (See ObjectMeta#creationTimestamp)

Since this will involve a breaking change, maybe we can already switch this to use a Java Timestamp-compatible (java.time) type + the Jackson JavaTimeModule.

[yimoucao]

@manusa I can fix it using Strings fairly quick. If we use a Java Timestamp type, will it require serializer/deserializer change? I'm assuming something has to be done like registering JavaTimeModule and configuring feature flags. To accommodate RFC-3339, extra logic I think has to be written. In sum to fix with a right approach, it's worthwhile to open a new issue and fix all the timestamps not just this Istio one

[manusa]

Despite not being the most Java friendly way (OffsetDateTime, or other Java Time API types would be much better suited), we're currently mapping Tiemestamps to Strings: https://github.com/fabric8io/kubernetes-client/blob/711482cf7b75081693883d1fc538e65dd8f8043b/kubernetes-model-generator/kubernetes-model-core/src/generated/java/io/fabric8/kubernetes/api/model/ObjectMeta.java#L58-L59

https://github.com/kubernetes/apimachinery/blob/478dd6ed1ec9b4cc3459dcd4f1d772013ac0103c/pkg/apis/meta/v1/types.go#L179-L188

So for consistency reasons, we should stick with mapping Timestamps to Strings.