Bug 2050332: explicit Pattern to validate Duration fields

Due to a difference in how apiserver validates `Format=duration` vs how
apimachinery unmarshals `metav1.Duration`, using that format can allow
users to blow up some of our controllers if they provide certain values
for Duration fields (specifically: containing `d` or `w` units).
Upstream issues have been opened against both sides of this discrepancy:
- kubernetes/apimachinery#131
- kubernetes/apiextensions-apiserver#56

In the meantime, work around the problem by using an explicit regex
instead of the built in format validator.

Closes 2050332

apis/hive/v1/clusterclaim_types.go

@@ -24,9 +24,13 @@ type ClusterClaimSpec struct {
  // Lifetime is the maximum lifetime of the claim after it is assigned a cluster. If the claim still exists
  // when the lifetime has elapsed, the claim will be deleted by Hive.
  // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
  // +kubebuilder:validation:Type=string
- // +kubebuilder:validation:Format=duration
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  Lifetime *metav1.Duration `json:"lifetime,omitempty"`
 }
 

apis/hive/v1/clusterdeployment_types.go

@@ -175,9 +175,13 @@ type ClusterDeploymentSpec struct {
  // given duration. The time that a cluster has been running is the time since the cluster was installed or the
  // time since the cluster last came out of hibernation.
  // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
  // +kubebuilder:validation:Type=string
- // +kubebuilder:validation:Format=duration
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  HibernateAfter *metav1.Duration `json:"hibernateAfter,omitempty"`
 
  // InstallAttemptsLimit is the maximum number of times Hive will attempt to install the cluster.

apis/hive/v1/clusterpool_types.go

@@ -68,9 +68,13 @@ type ClusterPoolSpec struct {
  // that a cluster has been running is the time since the cluster was installed or the time since the cluster last came
  // out of hibernation.
  // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
  // +kubebuilder:validation:Type=string
- // +kubebuilder:validation:Format=duration
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  HibernateAfter *metav1.Duration `json:"hibernateAfter,omitempty"`
 
  // InstallAttemptsLimit is the maximum number of times Hive will attempt to install the cluster.
@@ -95,26 +99,41 @@ type HibernationConfig struct {
  // hibernation (e.g. at the behest of runningCount, or in preparation for being claimed). If this time is
  // exceeded, the ClusterDeployment will be considered Broken and we will replace it. The default (unspecified
  // or zero) means no timeout -- we will allow the ClusterDeployment to continue trying to resume "forever".
+ // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
+ // +kubebuilder:validation:Type=string
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  ResumeTimeout metav1.Duration `json:"resumeTimeout"`
 }
 
 // ClusterPoolClaimLifetime defines the lifetimes for claims for the cluster pool.
 type ClusterPoolClaimLifetime struct {
  // Default is the default lifetime of the claim when no lifetime is set on the claim itself.
  // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
  // +kubebuilder:validation:Type=string
- // +kubebuilder:validation:Format=duration
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  Default *metav1.Duration `json:"default,omitempty"`
 
  // Maximum is the maximum lifetime of the claim after it is assigned a cluster. If the claim still exists
  // when the lifetime has elapsed, the claim will be deleted by Hive.
  // The lifetime of a claim is the mimimum of the lifetimes set by the cluster pool and the claim itself.
  // This is a Duration value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ // Note: due to discrepancies in validation vs parsing, we use a Pattern instead of `Format=duration`. See
+ // https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ // https:/kubernetes/apimachinery/issues/131
+ // https:/kubernetes/apiextensions-apiserver/issues/56
  // +optional
  // +kubebuilder:validation:Type=string
- // +kubebuilder:validation:Format=duration
+ // +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
  Maximum *metav1.Duration `json:"maximum,omitempty"`
 }
 

config/crds/hive.openshift.io_clusterclaims.yaml

@@ -55,11 +55,14 @@ spec:
  which to claim a cluster.
  type: string
  lifetime:
- description: Lifetime is the maximum lifetime of the claim after it
- is assigned a cluster. If the claim still exists when the lifetime
+ description: 'Lifetime is the maximum lifetime of the claim after
+ it is assigned a cluster. If the claim still exists when the lifetime
  has elapsed, the claim will be deleted by Hive. This is a Duration
  value; see https://pkg.go.dev/time#ParseDuration for accepted formats.
- format: duration
+ Note: due to discrepancies in validation vs parsing, we use a Pattern
+ instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  namespace:
  description: Namespace is the namespace containing the ClusterDeployment

config/crds/hive.openshift.io_clusterdeployments.yaml

@@ -248,13 +248,15 @@ spec:
  type: object
  type: object
  hibernateAfter:
- description: HibernateAfter will transition a cluster to hibernating
+ description: 'HibernateAfter will transition a cluster to hibernating
  power state after it has been running for the given duration. The
  time that a cluster has been running is the time since the cluster
  was installed or the time since the cluster last came out of hibernation.
  This is a Duration value; see https://pkg.go.dev/time#ParseDuration
- for accepted formats.
- format: duration
+ for accepted formats. Note: due to discrepancies in validation vs
+ parsing, we use a Pattern instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  ingress:
  description: Ingress allows defining desired clusteringress/shards

config/crds/hive.openshift.io_clusterpools.yaml

@@ -70,43 +70,57 @@ spec:
  cluster pool.
  properties:
  default:
- description: Default is the default lifetime of the claim when
+ description: 'Default is the default lifetime of the claim when
  no lifetime is set on the claim itself. This is a Duration value;
  see https://pkg.go.dev/time#ParseDuration for accepted formats.
- format: duration
+ Note: due to discrepancies in validation vs parsing, we use
+ a Pattern instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  maximum:
- description: Maximum is the maximum lifetime of the claim after
+ description: 'Maximum is the maximum lifetime of the claim after
  it is assigned a cluster. If the claim still exists when the
  lifetime has elapsed, the claim will be deleted by Hive. The
  lifetime of a claim is the mimimum of the lifetimes set by the
  cluster pool and the claim itself. This is a Duration value;
  see https://pkg.go.dev/time#ParseDuration for accepted formats.
- format: duration
+ Note: due to discrepancies in validation vs parsing, we use
+ a Pattern instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  type: object
  hibernateAfter:
- description: HibernateAfter will be applied to new ClusterDeployments
+ description: 'HibernateAfter will be applied to new ClusterDeployments
  created for the pool. HibernateAfter will transition clusters in
  the clusterpool to hibernating power state after it has been running
  for the given duration. The time that a cluster has been running
  is the time since the cluster was installed or the time since the
  cluster last came out of hibernation. This is a Duration value;
  see https://pkg.go.dev/time#ParseDuration for accepted formats.
- format: duration
+ Note: due to discrepancies in validation vs parsing, we use a Pattern
+ instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  hibernationConfig:
  description: HibernationConfig configures the hibernation/resume behavior
  of ClusterDeployments owned by the ClusterPool.
  properties:
  resumeTimeout:
- description: ResumeTimeout is the maximum amount of time we will
+ description: 'ResumeTimeout is the maximum amount of time we will
  wait for an unclaimed ClusterDeployment to resume from hibernation
  (e.g. at the behest of runningCount, or in preparation for being
  claimed). If this time is exceeded, the ClusterDeployment will
  be considered Broken and we will replace it. The default (unspecified
  or zero) means no timeout -- we will allow the ClusterDeployment
- to continue trying to resume "forever".
+ to continue trying to resume "forever". This is a Duration value;
+ see https://pkg.go.dev/time#ParseDuration for accepted formats.
+ Note: due to discrepancies in validation vs parsing, we use
+ a Pattern instead of `Format=duration`. See https://bugzilla.redhat.com/show_bug.cgi?id=2050332
+ https:/kubernetes/apimachinery/issues/131 https:/kubernetes/apiextensions-apiserver/issues/56'
+ pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
  type: string
  type: object
  imageSetRef: