TTL and LimitMarkerTTL should have the same type

[ktarplee]

What motivated this proposal?

Currently we have the following in v1beta2.

type KeyValueSpec struct {
  ...
  TTL                      string             `json:"ttl,omitempty"`
  LimitMarkerTTL time.Duration `json:"limitMarkerTtl,omitempty"`
}

To set the fields the equivalent value one must use:

apiVersion: jetstream.nats.io/v1beta2
kind: KeyValue
metadata:
  name: my-kv
spec:
  bucket: my-kv
  ttl: 24h
  limitMarkerTtl: 86_400_000_000_000 # 24h as an integer (ns)

Both types are incorrect IMHO. I think they should both be changed to metav2.Duration. https://pkg.go.dev/k8s.io/apimachinery@v0.36.2/pkg/apis/meta/v1#Duration

What is the proposed change?

type KeyValueSpec struct {
  ...
  TTL                     metav1.Duration  `json:"ttl,omitempty"`
  LimitMarkerTTL metav1.Duration `json:"limitMarkerTtl,omitempty"`
}

Same would be true for ConsumerSpec.PinnedTTL, ConsumerSpec.InactiveThreshold, ObjectStoreSpec.TTL, StreamSpec.MaxAge, and StreamSpec.SubjectDeleteMarkerTTL.

Who benefits from this change?

Users would see an more consistent API. When/if NACK changes to automatically generating the CRDs then the extra methods that metav1.Duration provide will enhance the documentation.

What alternatives have you evaluated?

No response

[ktarplee]

This would be an API breaking change so if you want to do this maybe it should be a new version, such as v1beta3. I might be able to put together a MR if I get confirmation on the direction the maintainers want to go.

[Jarema]

thanks for filling in the issue.

There are quite few fields taht could benefit from this change, however, this one is the only one that is really breaking.

We could potentially work around it with IntOrString. Then it would be breaking only for people importing nack types directly.
Need to think about it a bit.