Make ImageVolumeSource.Reference required via OpenAPI

OpenAPI validation does not count toward the CRD XValidation CEL budget.

Author: cbandy

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

@@ -2620,7 +2620,9 @@ spec:
                           type: array
                           x-kubernetes-list-type: set
                         image:
-                          description: Details for adding an image volume
+                          description: |-
+                            Reference to an image or OCI artifact.
+                            More info: https://kubernetes.io/docs/concepts/storage/volumes#image
                           properties:
                             pullPolicy:
                               description: |-
@@ -2629,6 +2631,11 @@ spec:
                                 Never: the kubelet never pulls the reference and only uses a local image or artifact. Container creation will fail if the reference isn't present.
                                 IfNotPresent: the kubelet pulls if the reference isn't already present on disk. Container creation will fail if the reference isn't present and the pull fails.
                                 Defaults to Always if :latest tag is specified, or IfNotPresent otherwise.
+                              enum:
+                              - Always
+                              - Never
+                              - IfNotPresent
+                              maxLength: 12
                               type: string
                             reference:
                               description: |-
@@ -2638,7 +2645,10 @@ spec:
                                 More info: https://kubernetes.io/docs/concepts/containers/images
                                 This field is optional to allow higher level config management to default or override
                                 container images in workload controllers like Deployments and StatefulSets.
+                              minLength: 1
                               type: string
+                          required:
+                          - reference
                           type: object
                         name:
                           description: |-
@@ -2659,11 +2669,8 @@ spec:
                       x-kubernetes-validations:
                       - message: you must set only one of image or claimName
                         rule: has(self.claimName) != has(self.image)
-                      - message: readOnly cannot be set false when using an ImageVolumeSource
+                      - message: image volumes must be readOnly
                         rule: '!has(self.image) || !has(self.readOnly) || self.readOnly'
-                      - message: if using an ImageVolumeSource, you must set a reference
-                        rule: '!has(self.image) || (self.?image.reference.hasValue()
-                          && self.image.reference.size() > 0)'
                     maxItems: 10
                     type: array
                     x-kubernetes-list-map-keys:

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

@@ -3,13 +3,26 @@
 # SPDX-License-Identifier: Apache-2.0
 #
 # This [jq] filter modifies a Kubernetes CustomResourceDefinition.
+# Use the controller-gen "+kubebuilder:title" marker to identify schemas that need special manipulation.
 #
 # [jq]: https://jqlang.org
 
 # merge recursively combines a stream of objects.
 # https://jqlang.org/manual#multiplication-division-modulo
 def merge(stream): reduce stream as $i ({}; . * $i);
 
+# https://pkg.go.dev/k8s.io/api/core/v1#ImageVolumeSource
+reduce paths(try .title == "$corev1.ImageVolumeSource") as $path (.;
+  getpath($path) as $schema |
+  setpath($path; $schema * {
+    required: (["reference"] + ($schema.required // []) | sort),
+    properties: {
+      pullPolicy: { enum: ["Always", "Never", "IfNotPresent"] },
+      reference: { minLength: 1 }
+    }
+  } | del(.title))
+) |
+
 # Kubernetes assumes the evaluation cost of an enum value is very large: https://issue.k8s.io/119511
 # Look at every schema that has a populated "enum" property.
 reduce paths(try .enum | length > 0) as $path (.;

internal/crd/post-process.jq

@@ -14,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/yaml"
 
+	"github.com/crunchydata/postgres-operator/internal/testing/cmp"
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
 	v1 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
@@ -110,6 +111,7 @@ func TestPostgresUserInterfaceAcrossVersions(t *testing.T) {
 func TestAdditionalVolumes(t *testing.T) {
 	ctx := context.Background()
 	cc := require.KubernetesAtLeast(t, "1.30")
+	dryrun := client.NewDryRunClient(cc)
 	t.Parallel()
 
 	namespace := require.Namespace(t, cc)
@@ -154,8 +156,13 @@ func TestAdditionalVolumes(t *testing.T) {
 					}]
 				}
 			}]`, "spec", "instances")
-		err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
+
+		err := dryrun.Create(ctx, tmp.DeepCopy())
 		assert.Assert(t, apierrors.IsInvalid(err))
+
+		details := require.StatusErrorDetails(t, err)
+		assert.Assert(t, cmp.Len(details.Causes, 1))
+		assert.Equal(t, details.Causes[0].Field, "spec.instances[0].volumes.additional[0]")
 		assert.ErrorContains(t, err, "you must set only one of image or claimName")
 	})
 
@@ -178,9 +185,14 @@ func TestAdditionalVolumes(t *testing.T) {
 					}]
 				}
 			}]`, "spec", "instances")
-		err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
+
+		err := dryrun.Create(ctx, tmp.DeepCopy())
 		assert.Assert(t, apierrors.IsInvalid(err))
-		assert.ErrorContains(t, err, "readOnly cannot be set false when using an ImageVolumeSource")
+
+		details := require.StatusErrorDetails(t, err)
+		assert.Assert(t, cmp.Len(details.Causes, 1))
+		assert.Equal(t, details.Causes[0].Field, "spec.instances[0].volumes.additional[0]")
+		assert.ErrorContains(t, err, "image volumes must be readOnly")
 	})
 
 	t.Run("Reference must be set when using image volume", func(t *testing.T) {
@@ -201,9 +213,15 @@ func TestAdditionalVolumes(t *testing.T) {
 					}]
 				}
 			}]`, "spec", "instances")
-		err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
+
+		err := dryrun.Create(ctx, tmp.DeepCopy())
 		assert.Assert(t, apierrors.IsInvalid(err))
-		assert.ErrorContains(t, err, "if using an ImageVolumeSource, you must set a reference")
+
+		details := require.StatusErrorDetails(t, err)
+		assert.Assert(t, cmp.Len(details.Causes, 2))
+		assert.Assert(t, cmp.Equal(details.Causes[0].Field, "spec.instances[0].volumes.additional[0].image.reference"))
+		assert.Assert(t, cmp.Equal(details.Causes[0].Type, "FieldValueRequired"))
+		assert.ErrorContains(t, err, "Required")
 	})
 
 	t.Run("Reference cannot be an empty string when using image volume", func(t *testing.T) {
@@ -225,9 +243,15 @@ func TestAdditionalVolumes(t *testing.T) {
 					}]
 				}
 			}]`, "spec", "instances")
-		err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll)
+
+		err := dryrun.Create(ctx, tmp.DeepCopy())
 		assert.Assert(t, apierrors.IsInvalid(err))
-		assert.ErrorContains(t, err, "if using an ImageVolumeSource, you must set a reference")
+
+		details := require.StatusErrorDetails(t, err)
+		assert.Assert(t, cmp.Len(details.Causes, 1))
+		assert.Assert(t, cmp.Equal(details.Causes[0].Field, "spec.instances[0].volumes.additional[0].image.reference"))
+		assert.Assert(t, cmp.Equal(details.Causes[0].Type, "FieldValueInvalid"))
+		assert.ErrorContains(t, err, "at least 1 chars long")
 	})
 
 	t.Run("ReadOnly can be omitted or set true when using image volume", func(t *testing.T) {
@@ -265,6 +289,6 @@ func TestAdditionalVolumes(t *testing.T) {
 					}]
 				}
 			}]`, "spec", "instances")
-		assert.NilError(t, cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll))
+		assert.NilError(t, dryrun.Create(ctx, tmp.DeepCopy()))
 	})
 }