From daa6b4631a4433c76e879adbda462a51b2efe390 Mon Sep 17 00:00:00 2001 From: benjaminjb Date: Wed, 10 Sep 2025 15:11:09 -0500 Subject: [PATCH 1/6] Add validation for pgbouncer logfile This PR adds validation for v1 pgBouncer so that the logfile must either be in /tmp or one of the attached additional volumes. This PR also adds validation for v1beta1 that restricts the number of global config settings and requires a '.log' suffix on the logfile. (This may be reverted in the future as we are learning towards validation in v1 and documentation in v1beta1.) This PR also fixes an error in the OTEL setup around a custom logfile. Issues: [PGO-2565] --- ...ator.crunchydata.com_postgresclusters.yaml | 22 ++- internal/pgbouncer/reconcile.go | 2 +- internal/pgbouncer/reconcile_test.go | 73 ++++++++ internal/testing/validation/pgbouncer_test.go | 168 ++++++++++++++++++ .../v1/pgbouncer_types.go | 15 ++ .../v1/postgrescluster_types.go | 2 +- .../v1/zz_generated.deepcopy.go | 18 +- .../v1beta1/pgbouncer_types.go | 7 + 8 files changed, 298 insertions(+), 9 deletions(-) create mode 100644 internal/testing/validation/pgbouncer_test.go create mode 100644 pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index b7915891c..0152ba941 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -14472,10 +14472,12 @@ spec: global: additionalProperties: type: string - description: |- - Settings that apply to the entire PgBouncer process. - More info: https://www.pgbouncer.org/config.html + maxProperties: 50 type: object + x-kubernetes-map-type: granular + x-kubernetes-validations: + - message: logfile config must end with '.log' + rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' users: additionalProperties: type: string @@ -16505,6 +16507,12 @@ spec: x-kubernetes-list-type: map type: object type: object + x-kubernetes-validations: + - message: config.global.logfile destination is restricted to + '/tmp/logs/pgbouncer/' or an existing additional volume + rule: self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") + || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, + f.startsWith("/volumes/" + v.name)))).orValue(true) required: - pgBouncer type: object @@ -33505,10 +33513,12 @@ spec: global: additionalProperties: type: string - description: |- - Settings that apply to the entire PgBouncer process. - More info: https://www.pgbouncer.org/config.html + maxProperties: 50 type: object + x-kubernetes-map-type: granular + x-kubernetes-validations: + - message: logfile config must end with '.log' + rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' users: additionalProperties: type: string diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 613f0dbae..304749d3b 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -223,7 +223,7 @@ func Pod( if collector.OpenTelemetryLogsOrMetricsEnabled(ctx, inCluster) { collector.AddToPod(ctx, inCluster.Spec.Instrumentation, inCluster.Spec.ImagePullPolicy, inConfigMap, template, []corev1.VolumeMount{configVolumeMount}, string(inSecret.Data["pgbouncer-password"]), - []string{naming.PGBouncerLogPath}, true, true) + []string{logPath}, true, true) } } diff --git a/internal/pgbouncer/reconcile_test.go b/internal/pgbouncer/reconcile_test.go index fe0287931..ba964e1c8 100644 --- a/internal/pgbouncer/reconcile_test.go +++ b/internal/pgbouncer/reconcile_test.go @@ -142,6 +142,10 @@ func TestPod(t *testing.T) { t.Parallel() features := feature.NewGate() + assert.NilError(t, features.SetFromMap(map[string]bool{ + feature.OpenTelemetryLogs: true, + feature.OpenTelemetryMetrics: true, + })) ctx := feature.NewContext(context.Background(), features) cluster := new(v1beta1.PostgresCluster) @@ -463,6 +467,59 @@ containers: - mountPath: /etc/pgbouncer name: pgbouncer-config readOnly: true +- command: + - bash + - -ceu + - -- + - "monitor() {\n\nmkdir -p '/tmp/receiver' && { chmod 0775 '/tmp/receiver' || :; + }\nOTEL_PIDFILE=/tmp/otel.pid\n\nstart_otel_collector() {\n\techo \"Starting OTel + Collector\"\n\t/otelcol-contrib --config /etc/otel-collector/config.yaml &\n\techo + $! > $OTEL_PIDFILE\n}\nstart_otel_collector\n\nexec {fd}<> <(:||:)\nwhile read + -r -t 5 -u \"${fd}\" ||:; do\n\tlogrotate -s /tmp/logrotate.status /etc/logrotate.d/logrotate.conf\n\tif + [[ \"${directory}\" -nt \"/proc/self/fd/${fd}\" ]] && kill -HUP $(head -1 ${OTEL_PIDFILE?});\n\tthen\n\t\techo + \"OTel configuration changed...\"\n\t\texec {fd}>&- && exec {fd}<> <(:||:)\n\t\tstat + --format='Loaded configuration dated %y' \"${directory}\"\n\tfi\n\tif [[ ! -e + /proc/$(head -1 ${OTEL_PIDFILE?}) ]] ; then\n\t\tstart_otel_collector\n\tfi\ndone\n}; + export directory=\"$1\"; export -f monitor; exec -a \"$0\" bash -ceu monitor" + - collector + - /etc/otel-collector + env: + - name: K8S_POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: K8S_POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: PGPASSWORD + imagePullPolicy: Always + name: collector + ports: + - containerPort: 9187 + name: otel-metrics + protocol: TCP + resources: {} + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + privileged: false + readOnlyRootFilesystem: true + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/pgbouncer + name: pgbouncer-config + readOnly: true + - mountPath: /etc/otel-collector + name: collector-config + readOnly: true + - mountPath: /etc/logrotate.d + name: logrotate-config + readOnly: true volumes: - name: pgbouncer-config projected: @@ -490,6 +547,20 @@ volumes: items: - key: ca.crt path: ~postgres-operator/backend-ca.crt +- name: logrotate-config + projected: + sources: + - configMap: + items: + - key: logrotate.conf + path: logrotate.conf +- name: collector-config + projected: + sources: + - configMap: + items: + - key: collector.yaml + path: config.yaml `)) }) @@ -498,6 +569,8 @@ volumes: "logfile": "/volumes/required/mylog.log", } logfile = "/volumes/required/mylog.log" + // Reset the instrumentation from the previous test + cluster.Spec.Instrumentation = nil call() diff --git a/internal/testing/validation/pgbouncer_test.go b/internal/testing/validation/pgbouncer_test.go new file mode 100644 index 000000000..3e6345a65 --- /dev/null +++ b/internal/testing/validation/pgbouncer_test.go @@ -0,0 +1,168 @@ +// Copyright 2021 - 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "context" + "testing" + + "gotest.tools/v3/assert" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/crunchydata/postgres-operator/internal/testing/require" + v1 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1" +) + +func TestV1PGBouncerLogging(t *testing.T) { + ctx := context.Background() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + + base := v1.NewPostgresCluster() + base.Namespace = namespace.Name + base.Name = "pgbouncer-logging" + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base to be valid") + + t.Run("Can set logging on tmp with .log", func(t *testing.T) { + tmp := base.DeepCopy() + + require.UnmarshalInto(t, &tmp.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/tmp/logs/pgbouncer/log.log" + } + } + } + }`) + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this option to be valid") + }) + + t.Run("Cannot set logging on tmp without .log", func(t *testing.T) { + tmp := base.DeepCopy() + + require.UnmarshalInto(t, &tmp.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/tmp/logs/pgbouncer/log.txt" + } + } + } + }`) + + err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "logfile config must end with '.log'") + }) + + t.Run("Cannot set logging on tmp without correct subdir", func(t *testing.T) { + tmp := base.DeepCopy() + + require.UnmarshalInto(t, &tmp.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/tmp/logs/log.log" + } + } + } + }`) + + err := cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume") + + require.UnmarshalInto(t, &tmp.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/tmp/pgbouncer/log.log" + } + } + } + }`) + + err = cc.Create(ctx, tmp.DeepCopy(), client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume") + }) + + t.Run("Cannot set logging on volumes that don't exist", func(t *testing.T) { + vol := base.DeepCopy() + + require.UnmarshalInto(t, &vol.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/volumes/logging/log.log" + } + } + } + }`) + + err := cc.Create(ctx, vol.DeepCopy(), client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume") + }) + + t.Run("Cannot set logging elsewhere", func(t *testing.T) { + vol := base.DeepCopy() + + require.UnmarshalInto(t, &vol.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/var/log.log" + } + } + } + }`) + + err := cc.Create(ctx, vol.DeepCopy(), client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume") + }) + + t.Run("Can set logging on volumes that exist", func(t *testing.T) { + vol := base.DeepCopy() + + require.UnmarshalInto(t, &vol.Spec.Proxy, `{ + pgBouncer: { + config: { + global: { + logfile: "/volumes/logging/log.log" + } + }, + volumes: { + additional: [ + { + name: logging, + claimName: required-1 + }] + } + } + }`) + + assert.NilError(t, cc.Create(ctx, vol.DeepCopy(), client.DryRunAll), + "expected this option to be valid") + }) +} diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go new file mode 100644 index 000000000..48f71f017 --- /dev/null +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go @@ -0,0 +1,15 @@ +// Copyright 2021 - 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package v1 + +import ( + "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" +) + +// PGBouncerPodSpec defines the desired state of a PgBouncer connection pooler. +// +kubebuilder:validation:XValidation:rule=`self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, f.startsWith("/volumes/" + v.name)))).orValue(true)`,message=`config.global.logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume` +type PGBouncerPodSpec struct { + v1beta1.PGBouncerPodSpec `json:",inline"` +} diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go index fc409ee0c..d021e2db3 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/postgrescluster_types.go @@ -602,7 +602,7 @@ type PostgresInstanceSetStatus struct { type PostgresProxySpec struct { // Defines a PgBouncer proxy and connection pooler. - PGBouncer *v1beta1.PGBouncerPodSpec `json:"pgBouncer"` + PGBouncer *PGBouncerPodSpec `json:"pgBouncer"` } // Default sets the defaults for any proxies that are set. diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go index 20e65aee2..6f16d3a3e 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/zz_generated.deepcopy.go @@ -182,6 +182,22 @@ func (in *MonitoringStatus) DeepCopy() *MonitoringStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PGBouncerPodSpec) DeepCopyInto(out *PGBouncerPodSpec) { + *out = *in + in.PGBouncerPodSpec.DeepCopyInto(&out.PGBouncerPodSpec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PGBouncerPodSpec. +func (in *PGBouncerPodSpec) DeepCopy() *PGBouncerPodSpec { + if in == nil { + return nil + } + out := new(PGBouncerPodSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresCluster) DeepCopyInto(out *PostgresCluster) { *out = *in @@ -594,7 +610,7 @@ func (in *PostgresProxySpec) DeepCopyInto(out *PostgresProxySpec) { *out = *in if in.PGBouncer != nil { in, out := &in.PGBouncer, &out.PGBouncer - *out = new(v1beta1.PGBouncerPodSpec) + *out = new(PGBouncerPodSpec) (*in).DeepCopyInto(*out) } } diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go index 49e713c17..89f41df60 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go @@ -26,7 +26,14 @@ type PGBouncerConfiguration struct { // Settings that apply to the entire PgBouncer process. // More info: https://www.pgbouncer.org/config.html + // --- + // # Logging + // +kubebuilder:validation:XValidation:rule=`!has(self.logfile) || self.logfile.endsWith('.log')`,message=`logfile config must end with '.log'` + // +kubebuilder:validation:MaxProperties=50 + // See also XValidation rule on v1 PostgresProxySpec + // +optional + // +mapType=granular Global map[string]string `json:"global,omitempty"` // PgBouncer database definitions. The key is the database requested by a From fe54a3699c96a5b7c70af5b935f7ab1f660d41e3 Mon Sep 17 00:00:00 2001 From: benjaminjb Date: Thu, 11 Sep 2025 11:13:45 -0500 Subject: [PATCH 2/6] Make changes for K8s version k8s 1.28 cannot handle the .? validation we wanted to use in CEL, so this PR uses a less complex version. --- ...-operator.crunchydata.com_postgresclusters.yaml | 11 ++++++----- .../v1/pgbouncer_types.go | 5 ++++- .../validation.md | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 0152ba941..eff53cd8d 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -16508,11 +16508,12 @@ spec: type: object type: object x-kubernetes-validations: - - message: config.global.logfile destination is restricted to - '/tmp/logs/pgbouncer/' or an existing additional volume - rule: self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") - || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, - f.startsWith("/volumes/" + v.name)))).orValue(true) + - message: logfile destination is restricted to '/tmp/logs/pgbouncer/' + or an existing additional volume + rule: '!has(self.config) || !has(self.config.global) || !has(self.config.global.logfile) + || self.config.global.logfile.startsWith(''/tmp/logs/pgbouncer/'') + || (has(self.volumes) && has(self.volumes.additional) && self.volumes.additional.exists(x, + self.config.global.logfile.startsWith("/volumes/"+x.name)))' required: - pgBouncer type: object diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go index 48f71f017..76ddad4d5 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go @@ -9,7 +9,10 @@ import ( ) // PGBouncerPodSpec defines the desired state of a PgBouncer connection pooler. -// +kubebuilder:validation:XValidation:rule=`self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, f.startsWith("/volumes/" + v.name)))).orValue(true)`,message=`config.global.logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume` +// +kubebuilder:validation:XValidation:rule=`!has(self.config) || !has(self.config.global) || !has(self.config.global.logfile) || self.config.global.logfile.startsWith('/tmp/logs/pgbouncer/') || (has(self.volumes) && has(self.volumes.additional) && self.volumes.additional.exists(x, self.config.global.logfile.startsWith("/volumes/"+x.name)))`,message=`logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume` +// --- +// TODO: the `.?` CEL syntax is unsupported in k8s 1.28, so we cannot use the optional field syntax +// of `self.?config.global.logfile` and `self.?volumes.additional` type PGBouncerPodSpec struct { v1beta1.PGBouncerPodSpec `json:",inline"` } diff --git a/pkg/apis/postgres-operator.crunchydata.com/validation.md b/pkg/apis/postgres-operator.crunchydata.com/validation.md index e1e383931..49a243d4c 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/validation.md +++ b/pkg/apis/postgres-operator.crunchydata.com/validation.md @@ -95,3 +95,17 @@ The `additionalProperties` property indicates that the keys are unknown; these f > When possible, use [OpenAPI properties](#FIXME) rather than CEL rules. > The former do not affect the CRD [validation budget](#FIXME). +## Optional field syntax + +CEL offers a safe traversal/retrieval through the use of `?` as an [optional field marker]. + +As an example, when attempting to retrieve `self.config.global.logfile`, in older (but still supported) +versions of k8s, we may need to incrementally check the path: `has(self.config) && has(self.config.global)...` + +With the optional field marker, we can use that field safely without checking the entire path: +`self.?config.global.logfile` will return a value or no value; and anything using that value will +likewise be considered optional. + +The optional field syntax is only available in K8s 1.29+. + +[optional field marker]: https://pkg.go.dev/github.com/google/cel-go/cel#hdr-Syntax_Changes-OptionalTypes. From a9f85def3ebd9727f2a47fa13d19760ccf48ca75 Mon Sep 17 00:00:00 2001 From: benjaminjb Date: Thu, 11 Sep 2025 12:39:18 -0500 Subject: [PATCH 3/6] Bump lowest tested K8s, adjust CEL 5.8+ has k8s 1.30 for its floor; OCP is 4.14 (k8s 1.27) but 4.16 (k8s 1.29) works with this CEL. --- .github/workflows/test.yaml | 4 ++-- ...res-operator.crunchydata.com_postgresclusters.yaml | 11 +++++------ .../v1/pgbouncer_types.go | 5 +---- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6b64e801c..d86fcf645 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -58,7 +58,7 @@ jobs: strategy: fail-fast: false matrix: - kubernetes: [v1.33, v1.28] + kubernetes: [v1.33, v1.30] steps: - uses: actions/checkout@v5 - uses: actions/setup-go@v5 @@ -92,7 +92,7 @@ jobs: strategy: fail-fast: false matrix: - kubernetes: [v1.33, v1.28] + kubernetes: [v1.33, v1.30] steps: - uses: actions/checkout@v5 - uses: actions/setup-go@v5 diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index eff53cd8d..0152ba941 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -16508,12 +16508,11 @@ spec: type: object type: object x-kubernetes-validations: - - message: logfile destination is restricted to '/tmp/logs/pgbouncer/' - or an existing additional volume - rule: '!has(self.config) || !has(self.config.global) || !has(self.config.global.logfile) - || self.config.global.logfile.startsWith(''/tmp/logs/pgbouncer/'') - || (has(self.volumes) && has(self.volumes.additional) && self.volumes.additional.exists(x, - self.config.global.logfile.startsWith("/volumes/"+x.name)))' + - message: config.global.logfile destination is restricted to + '/tmp/logs/pgbouncer/' or an existing additional volume + rule: self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") + || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, + f.startsWith("/volumes/" + v.name)))).orValue(true) required: - pgBouncer type: object diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go index 76ddad4d5..48f71f017 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1/pgbouncer_types.go @@ -9,10 +9,7 @@ import ( ) // PGBouncerPodSpec defines the desired state of a PgBouncer connection pooler. -// +kubebuilder:validation:XValidation:rule=`!has(self.config) || !has(self.config.global) || !has(self.config.global.logfile) || self.config.global.logfile.startsWith('/tmp/logs/pgbouncer/') || (has(self.volumes) && has(self.volumes.additional) && self.volumes.additional.exists(x, self.config.global.logfile.startsWith("/volumes/"+x.name)))`,message=`logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume` -// --- -// TODO: the `.?` CEL syntax is unsupported in k8s 1.28, so we cannot use the optional field syntax -// of `self.?config.global.logfile` and `self.?volumes.additional` +// +kubebuilder:validation:XValidation:rule=`self.?config.global.logfile.optMap(f, f.startsWith("/tmp/logs/pgbouncer/") || (self.?volumes.additional.hasValue() && self.volumes.additional.exists(v, f.startsWith("/volumes/" + v.name)))).orValue(true)`,message=`config.global.logfile destination is restricted to '/tmp/logs/pgbouncer/' or an existing additional volume` type PGBouncerPodSpec struct { v1beta1.PGBouncerPodSpec `json:",inline"` } From 746c90671fc17e6e5e2ff2ccb438b215562376d2 Mon Sep 17 00:00:00 2001 From: Benjamin Blattberg Date: Thu, 11 Sep 2025 16:31:31 -0500 Subject: [PATCH 4/6] Update pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go Co-authored-by: Chris Bandy --- .../postgres-operator.crunchydata.com_postgresclusters.yaml | 6 ++++++ .../v1beta1/pgbouncer_types.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 0152ba941..098a0493c 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -14472,6 +14472,9 @@ spec: global: additionalProperties: type: string + description: |- + Settings that apply to the entire PgBouncer process. + More info: https://www.pgbouncer.org/config.html maxProperties: 50 type: object x-kubernetes-map-type: granular @@ -33513,6 +33516,9 @@ spec: global: additionalProperties: type: string + description: |- + Settings that apply to the entire PgBouncer process. + More info: https://www.pgbouncer.org/config.html maxProperties: 50 type: object x-kubernetes-map-type: granular diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go index 89f41df60..9632ecae1 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go @@ -31,7 +31,7 @@ type PGBouncerConfiguration struct { // +kubebuilder:validation:XValidation:rule=`!has(self.logfile) || self.logfile.endsWith('.log')`,message=`logfile config must end with '.log'` // +kubebuilder:validation:MaxProperties=50 // See also XValidation rule on v1 PostgresProxySpec - + // // +optional // +mapType=granular Global map[string]string `json:"global,omitempty"` From 1ea3e45d2d95f5d1bd99ac8303e4c3f6c96aa6f6 Mon Sep 17 00:00:00 2001 From: benjaminjb Date: Fri, 12 Sep 2025 12:17:54 -0500 Subject: [PATCH 5/6] Change global validation to optional marker --- .../postgres-operator.crunchydata.com_postgresclusters.yaml | 4 ++-- .../v1beta1/pgbouncer_types.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 098a0493c..bf1fa6e45 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -14480,7 +14480,7 @@ spec: x-kubernetes-map-type: granular x-kubernetes-validations: - message: logfile config must end with '.log' - rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' + rule: self.?logfile.orValue("").endsWith('.log') users: additionalProperties: type: string @@ -33524,7 +33524,7 @@ spec: x-kubernetes-map-type: granular x-kubernetes-validations: - message: logfile config must end with '.log' - rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' + rule: self.?logfile.orValue("").endsWith('.log') users: additionalProperties: type: string diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go index 9632ecae1..ba70a14d6 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go @@ -28,7 +28,7 @@ type PGBouncerConfiguration struct { // More info: https://www.pgbouncer.org/config.html // --- // # Logging - // +kubebuilder:validation:XValidation:rule=`!has(self.logfile) || self.logfile.endsWith('.log')`,message=`logfile config must end with '.log'` + // +kubebuilder:validation:XValidation:rule=`self.?logfile.orValue("").endsWith('.log')`,message=`logfile config must end with '.log'` // +kubebuilder:validation:MaxProperties=50 // See also XValidation rule on v1 PostgresProxySpec // From 1141e0f28b12ef683dbf9e6acd5e0b1cea75e77e Mon Sep 17 00:00:00 2001 From: benjaminjb Date: Fri, 12 Sep 2025 13:14:41 -0500 Subject: [PATCH 6/6] Change pgbouncer CEL rule This path is just one step, so use the 'has' macro --- .../postgres-operator.crunchydata.com_postgresclusters.yaml | 4 ++-- .../v1beta1/pgbouncer_types.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index bf1fa6e45..098a0493c 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -14480,7 +14480,7 @@ spec: x-kubernetes-map-type: granular x-kubernetes-validations: - message: logfile config must end with '.log' - rule: self.?logfile.orValue("").endsWith('.log') + rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' users: additionalProperties: type: string @@ -33524,7 +33524,7 @@ spec: x-kubernetes-map-type: granular x-kubernetes-validations: - message: logfile config must end with '.log' - rule: self.?logfile.orValue("").endsWith('.log') + rule: '!has(self.logfile) || self.logfile.endsWith(''.log'')' users: additionalProperties: type: string diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go index ba70a14d6..9632ecae1 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/pgbouncer_types.go @@ -28,7 +28,7 @@ type PGBouncerConfiguration struct { // More info: https://www.pgbouncer.org/config.html // --- // # Logging - // +kubebuilder:validation:XValidation:rule=`self.?logfile.orValue("").endsWith('.log')`,message=`logfile config must end with '.log'` + // +kubebuilder:validation:XValidation:rule=`!has(self.logfile) || self.logfile.endsWith('.log')`,message=`logfile config must end with '.log'` // +kubebuilder:validation:MaxProperties=50 // See also XValidation rule on v1 PostgresProxySpec //