FIXUP: review feedback

Author: cbandy

config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml

@@ -1955,12 +1955,11 @@ spec:
                           and unit: `3d`, `4 weeks`, `12 hr`, etc.
                         format: duration
                         minLength: 1
-                        pattern: ^(PT)?(0|[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$
+                        pattern: ^(PT)?[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?)+$
                         type: string
                         x-kubernetes-validations:
-                        - message: must be greater than one hour
-                          rule: self == duration("0") || (self >= duration("1h") &&
-                            self <= duration("8760h"))
+                        - message: must be at least one hour
+                          rule: duration("1h") <= self && self <= duration("8760h")
                     type: object
                   resources:
                     description: Resources holds the resource requirements for the

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

@@ -11488,12 +11488,11 @@ spec:
                           and unit: `3d`, `4 weeks`, `12 hr`, etc.
                         format: duration
                         minLength: 1
-                        pattern: ^(PT)?(0|[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$
+                        pattern: ^(PT)?[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?)+$
                         type: string
                         x-kubernetes-validations:
-                        - message: must be greater than one hour
-                          rule: self == duration("0") || (self >= duration("1h") &&
-                            self <= duration("8760h"))
+                        - message: must be at least one hour
+                          rule: duration("1h") <= self && self <= duration("8760h")
                     type: object
                   resources:
                     description: Resources holds the resource requirements for the

go.mod

@@ -27,6 +27,7 @@ require (
 	k8s.io/apimachinery v0.31.0
 	k8s.io/client-go v0.31.0
 	k8s.io/component-base v0.31.0
+	k8s.io/kube-openapi v0.0.0-20240521193020-835d969ad83a
 	sigs.k8s.io/controller-runtime v0.19.3
 	sigs.k8s.io/yaml v1.4.0
 )
@@ -120,7 +121,6 @@ require (
 	k8s.io/apiextensions-apiserver v0.31.0 // indirect
 	k8s.io/apiserver v0.31.0 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect
-	k8s.io/kube-openapi v0.0.0-20240521193020-835d969ad83a // indirect
 	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

internal/testing/validation/pgadmin_test.go

@@ -60,8 +60,6 @@ func TestPGAdminInstrumentation(t *testing.T) {
 				"28 weeks",
 				"90 DAY",
 				"1 hr",
-				"0 days",
-				"0",
 				"PT1D",
 			} {
 				u, err := runtime.ToUnstructuredObject(pgadmin)
@@ -72,5 +70,21 @@ func TestPGAdminInstrumentation(t *testing.T) {
 				assert.NilError(t, cc.Create(ctx, u, client.DryRunAll), tt)
 			}
 		})
+
+		t.Run("Invalid", func(t *testing.T) {
+			for _, tt := range []string{
+				"0 days",
+				"0",
+			} {
+				u, err := runtime.ToUnstructuredObject(pgadmin)
+				assert.NilError(t, err)
+				assert.NilError(t, unstructured.SetNestedField(u.Object,
+					tt, "spec", "instrumentation", "logs", "retentionPeriod"))
+
+				err = cc.Create(ctx, u, client.DryRunAll)
+				assert.Assert(t, apierrors.IsInvalid(err), tt)
+				assert.ErrorContains(t, err, "retentionPeriod")
+			}
+		})
 	})
 }

pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go

@@ -58,12 +58,12 @@ type InstrumentationLogsSpec struct {
 	// ---
 	// Kubernetes ensures the value is in the "duration" format, but go ahead
 	// and loosely validate the format to show some acceptable units.
-	// +kubebuilder:validation:Pattern=`^(PT)?(0|[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$`
+	// +kubebuilder:validation:Pattern=`^(PT)?[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?)+$`
 	//
 	// `controller-gen` needs to know "Type=string" to allow a "Pattern".
 	// +kubebuilder:validation:Type=string
 	//
-	// +kubebuilder:validation:XValidation:rule=`self == duration("0") || (self >= duration("1h") && self <= duration("8760h"))`,message="must be greater than one hour"
+	// +kubebuilder:validation:XValidation:rule=`duration("1h") <= self && self <= duration("8760h")`,message="must be at least one hour"
 	// +optional
 	RetentionPeriod *Duration `json:"retentionPeriod,omitempty"`
 }

pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go

@@ -60,7 +60,7 @@ func NewDuration(s string) (*Duration, error) {
 }
 
 // AsDuration returns d as a [metav1.Duration].
-func (d Duration) AsDuration() metav1.Duration {
+func (d *Duration) AsDuration() metav1.Duration {
 	return d.parsed
 }