From cdd28ca1d3d6fe8520f0492136458faae8359ef7 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 25 Jul 2025 13:19:27 -0500 Subject: [PATCH] Share PostgresCluster validation tests across API versions Tests are shared by manipulating unstructured values rather structs. This exposed a bug in HBA LDAP rule validation that was masked by "omitempty" and "omitzero" tags on struct fields. --- ...ator.crunchydata.com_postgresclusters.yaml | 8 +- internal/testing/require/encoding.go | 22 + internal/testing/require/encoding_test.go | 42 ++ .../postgres_authentication_test.go | 272 ++++++++++ .../postgrescluster/postgres_config_test.go | 238 +++++++++ .../postgrescluster/postgres_users_test.go | 168 ++++++ .../validation/postgrescluster_test.go | 499 ------------------ .../v1beta1/postgres_types.go | 6 +- 8 files changed, 749 insertions(+), 506 deletions(-) create mode 100644 internal/testing/validation/postgrescluster/postgres_authentication_test.go create mode 100644 internal/testing/validation/postgrescluster/postgres_config_test.go create mode 100644 internal/testing/validation/postgrescluster/postgres_users_test.go diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index 9eae0d3736..f0c3c6aace 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -136,8 +136,8 @@ spec: "ldapsearchattribute", or "ldapsearchfilter" options with "ldapprefix" or "ldapsuffix" options rule: has(self.hba) || self.method != "ldap" || !has(self.options) - || [["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].exists_one(a, - a.exists(k, k in self.options)) + || 2 > size([["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].filter(a, + a.exists(k, k in self.options))) - message: the "radius" method requires "radiusservers" and "radiussecrets" options rule: has(self.hba) || self.method != "radius" || (has(self.options) @@ -18767,8 +18767,8 @@ spec: "ldapsearchattribute", or "ldapsearchfilter" options with "ldapprefix" or "ldapsuffix" options rule: has(self.hba) || self.method != "ldap" || !has(self.options) - || [["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].exists_one(a, - a.exists(k, k in self.options)) + || 2 > size([["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].filter(a, + a.exists(k, k in self.options))) - message: the "radius" method requires "radiusservers" and "radiussecrets" options rule: has(self.hba) || self.method != "radius" || (has(self.options) diff --git a/internal/testing/require/encoding.go b/internal/testing/require/encoding.go index a99f7a42f1..8016c1921a 100644 --- a/internal/testing/require/encoding.go +++ b/internal/testing/require/encoding.go @@ -9,6 +9,7 @@ import ( "testing" "gotest.tools/v3/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/json" "sigs.k8s.io/yaml" ) @@ -37,3 +38,24 @@ func UnmarshalInto[Data ~string | ~[]byte, Destination *T, T any]( assert.NilError(t, err) assert.NilError(t, errors.Join(strict...)) } + +// UnmarshalIntoField parses input as YAML (or JSON) the same way as the Kubernetes API Server. +// The result goes into a (nested) field of output. It calls t.Fatal when something fails. +func UnmarshalIntoField[Data ~string | ~[]byte]( + t testing.TB, output *unstructured.Unstructured, input Data, fields ...string, +) { + t.Helper() + + if len(fields) == 0 { + t.Fatal("BUG: called without a destination") + } + + if output.Object == nil { + output.Object = map[string]any{} + } + + var value any + UnmarshalInto(t, &value, []byte(input)) + + assert.NilError(t, unstructured.SetNestedField(output.Object, value, fields...)) +} diff --git a/internal/testing/require/encoding_test.go b/internal/testing/require/encoding_test.go index e4f53611eb..cbdf93963c 100644 --- a/internal/testing/require/encoding_test.go +++ b/internal/testing/require/encoding_test.go @@ -8,10 +8,14 @@ import ( "reflect" "testing" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/crunchydata/postgres-operator/internal/testing/require" ) func TestUnmarshalInto(t *testing.T) { + t.Parallel() + for _, tt := range []struct { input string expected any @@ -39,3 +43,41 @@ func TestUnmarshalInto(t *testing.T) { } } } + +func TestUnmarshalIntoField(t *testing.T) { + t.Parallel() + + var u unstructured.Unstructured + + t.Run("NestedString", func(t *testing.T) { + u.Object = nil + require.UnmarshalIntoField(t, &u, `asdf`, "spec", "nested", "field") + + if !reflect.DeepEqual(u.Object, map[string]any{ + "spec": map[string]any{ + "nested": map[string]any{ + "field": "asdf", + }, + }, + }) { + t.Fatalf("got %[1]T(%#[1]v)", u.Object) + } + }) + + t.Run("Numeric", func(t *testing.T) { + u.Object = nil + require.UnmarshalIntoField(t, &u, `99`, "one") + require.UnmarshalIntoField(t, &u, `5.7`, "two") + + // Kubernetes distinguishes between integral and fractional numbers. + if !reflect.DeepEqual(u.Object, map[string]any{ + "one": int64(99), + "two": float64(5.7), + }) { + t.Fatalf("got %[1]T(%#[1]v)", u.Object) + } + }) + + // Correctly fails with: BUG: called without a destination + // require.UnmarshalIntoField(t, &u, `true`) +} diff --git a/internal/testing/validation/postgrescluster/postgres_authentication_test.go b/internal/testing/validation/postgrescluster/postgres_authentication_test.go new file mode 100644 index 0000000000..8ae80f719a --- /dev/null +++ b/internal/testing/validation/postgrescluster/postgres_authentication_test.go @@ -0,0 +1,272 @@ +// Copyright 2021 - 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "fmt" + "testing" + + "gotest.tools/v3/assert" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "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" +) + +func TestPostgresAuthenticationV1beta1(t *testing.T) { + ctx := t.Context() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-authentication-rules" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1beta1") + + testPostgresAuthenticationCommon(t, cc, u) +} + +func TestPostgresAuthenticationV1(t *testing.T) { + ctx := t.Context() + cc := require.KubernetesAtLeast(t, "1.30") + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-authentication-rules" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1") + + testPostgresAuthenticationCommon(t, cc, u) +} + +func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstructured.Unstructured) { + ctx := t.Context() + + t.Run("OneTopLevel", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: host, hba: anything }, + { users: [alice, bob], hba: anything }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 2)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot be combined")) + } + }) + + t.Run("NoInclude", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { hba: 'include "/etc/passwd"' }, + { hba: ' include_dir /tmp' }, + { hba: 'include_if_exists postgresql.auto.conf' }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].hba", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot include")) + } + }) + + t.Run("NoStructuredTrust", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: local, method: trust }, + { connection: hostssl, method: trust }, + { connection: hostgssenc, method: trust }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].method", i)) + assert.Assert(t, cmp.Contains(cause.Message, "unsafe")) + } + }) + + t.Run("LDAP", func(t *testing.T) { + t.Run("Required", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: ldap }, + { connection: hostssl, method: ldap, options: {} }, + { connection: hostssl, method: ldap, options: { ldapbinddn: any } }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) + assert.Assert(t, cmp.Contains(cause.Message, `"ldap" method requires`)) + } + + // These are valid. + + unstructured.RemoveNestedField(cluster.Object, "spec", "authentication") + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: ldap, options: { ldapbasedn: any } }, + { connection: hostssl, method: ldap, options: { ldapprefix: any } }, + { connection: hostssl, method: ldap, options: { ldapsuffix: any } }, + ], + }`, "spec", "authentication") + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + + t.Run("Mixed", func(t *testing.T) { + // Some options cannot be combined with others. + + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: ldap, options: { ldapbinddn: any, ldapprefix: other } }, + { connection: hostssl, method: ldap, options: { ldapbasedn: any, ldapsuffix: other } }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 2)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) + assert.Assert(t, cmp.Regexp(`cannot use .+? options with .+? options`, cause.Message)) + } + + // These combinations are allowed. + + unstructured.RemoveNestedField(cluster.Object, "spec", "authentication") + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: ldap, options: { ldapprefix: one, ldapsuffix: two } }, + { connection: hostssl, method: ldap, options: { ldapbasedn: one, ldapbinddn: two } }, + { connection: hostssl, method: ldap, options: { + ldapbasedn: one, ldapsearchattribute: two, ldapsearchfilter: three, + } }, + ], + }`, "spec", "authentication") + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + }) + + t.Run("RADIUS", func(t *testing.T) { + t.Run("Required", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: radius }, + { connection: hostssl, method: radius, options: {} }, + { connection: hostssl, method: radius, options: { radiusidentifiers: any } }, + { connection: hostssl, method: radius, options: { radiusservers: any } }, + { connection: hostssl, method: radius, options: { radiussecrets: any } }, + ], + }`, "spec", "authentication") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 5)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) + assert.Assert(t, cmp.Contains(cause.Message, `"radius" method requires`)) + } + + // These are valid. + + unstructured.RemoveNestedField(cluster.Object, "spec", "authentication") + require.UnmarshalIntoField(t, cluster, `{ + rules: [ + { connection: hostssl, method: radius, options: { radiusservers: one, radiussecrets: two } }, + { connection: hostssl, method: radius, options: { + radiusservers: one, radiussecrets: two, radiusports: three, + } }, + ], + }`, "spec", "authentication") + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + }) +} diff --git a/internal/testing/validation/postgrescluster/postgres_config_test.go b/internal/testing/validation/postgrescluster/postgres_config_test.go new file mode 100644 index 0000000000..a55d8de03d --- /dev/null +++ b/internal/testing/validation/postgrescluster/postgres_config_test.go @@ -0,0 +1,238 @@ +// Copyright 2021 - 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "testing" + + "gotest.tools/v3/assert" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "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" +) + +func TestPostgresConfigParametersV1beta1(t *testing.T) { + ctx := t.Context() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-config-parameters" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1beta1") + + testPostgresConfigParametersCommon(t, cc, u) +} + +func TestPostgresConfigParametersV1(t *testing.T) { + ctx := t.Context() + cc := require.KubernetesAtLeast(t, "1.30") + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-config-parameters" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1") + + testPostgresConfigParametersCommon(t, cc, u) +} + +func testPostgresConfigParametersCommon(t *testing.T, cc client.Client, base unstructured.Unstructured) { + ctx := t.Context() + + t.Run("Allowed", func(t *testing.T) { + for _, tt := range []struct { + key string + value any + }{ + {"archive_timeout", 100}, + {"archive_timeout", "20s"}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal(tt.value)), + "spec", "config", "parameters", tt.key) + + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + } + }) + + t.Run("Disallowed", func(t *testing.T) { + for _, tt := range []struct { + key string + value any + }{ + {key: "cluster_name", value: "asdf"}, + {key: "config_file", value: "asdf"}, + {key: "data_directory", value: ""}, + {key: "external_pid_file", value: ""}, + {key: "hba_file", value: "one"}, + {key: "hot_standby", value: "off"}, + {key: "ident_file", value: "two"}, + {key: "listen_addresses", value: ""}, + {key: "log_file_mode", value: ""}, + {key: "logging_collector", value: "off"}, + {key: "port", value: 5}, + {key: "wal_log_hints", value: "off"}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal(tt.value)), + "spec", "config", "parameters", tt.key) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 1)) + + // TODO(k8s-1.30) TODO(validation): Move the parameter name from the message to the field path. + assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") + assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.key)) + }) + } + }) + + t.Run("NoConnections", func(t *testing.T) { + for _, tt := range []struct { + key string + value any + }{ + {key: "ssl", value: "off"}, + {key: "ssl_ca_file", value: ""}, + {key: "unix_socket_directories", value: "one"}, + {key: "unix_socket_group", value: "two"}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal(tt.value)), + "spec", "config", "parameters", tt.key) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) + + t.Run("NoWriteAheadLog", func(t *testing.T) { + for _, tt := range []struct { + key string + value any + }{ + {key: "archive_mode", value: "off"}, + {key: "archive_command", value: "true"}, + {key: "restore_command", value: "true"}, + {key: "recovery_target", value: "immediate"}, + {key: "recovery_target_name", value: "doot"}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal(tt.value)), + "spec", "config", "parameters", tt.key) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) + + t.Run("wal_level", func(t *testing.T) { + t.Run("Valid", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + `logical`, "spec", "config", "parameters", "wal_level") + + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) + + t.Run("Invalid", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + `minimal`, "spec", "config", "parameters", "wal_level") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, `"replica" or higher`) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 1)) + assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") + assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "wal_level")) + }) + }) + + t.Run("NoReplication", func(t *testing.T) { + for _, tt := range []struct { + key string + value any + }{ + {key: "synchronous_standby_names", value: ""}, + {key: "primary_conninfo", value: ""}, + {key: "primary_slot_name", value: ""}, + {key: "recovery_min_apply_delay", value: ""}, + } { + t.Run(tt.key, func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal(tt.value)), + "spec", "config", "parameters", tt.key) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + }) + } + }) +} diff --git a/internal/testing/validation/postgrescluster/postgres_users_test.go b/internal/testing/validation/postgrescluster/postgres_users_test.go new file mode 100644 index 0000000000..4bb6ca52a4 --- /dev/null +++ b/internal/testing/validation/postgrescluster/postgres_users_test.go @@ -0,0 +1,168 @@ +// Copyright 2021 - 2025 Crunchy Data Solutions, Inc. +// +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "fmt" + "testing" + + "gotest.tools/v3/assert" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "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" +) + +func TestPostgresUserOptionsV1beta1(t *testing.T) { + ctx := t.Context() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-user-options" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1beta1") + + testPostgresUserOptionsCommon(t, cc, u) +} + +func TestPostgresUserOptionsV1(t *testing.T) { + ctx := t.Context() + cc := require.KubernetesAtLeast(t, "1.30") + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1.NewPostgresCluster() + + // required fields + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-user-options" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + var u unstructured.Unstructured + require.UnmarshalInto(t, &u, require.Value(yaml.Marshal(base))) + assert.Equal(t, u.GetAPIVersion(), "postgres-operator.crunchydata.com/v1") + + testPostgresUserOptionsCommon(t, cc, u) +} + +func testPostgresUserOptionsCommon(t *testing.T, cc client.Client, base unstructured.Unstructured) { + ctx := t.Context() + + // See [internal/controller/postgrescluster.TestValidatePostgresUsers] + + t.Run("NoComments", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal([]v1beta1.PostgresUserSpec{ + {Name: "dashes", Options: "ANY -- comment"}, + {Name: "block-open", Options: "/* asdf"}, + {Name: "block-close", Options: " qw */ rt"}, + })), + "spec", "users") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "cannot contain comments") + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot contain comments")) + } + }) + + t.Run("NoPassword", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal([]v1beta1.PostgresUserSpec{ + {Name: "uppercase", Options: "SUPERUSER PASSWORD ''"}, + {Name: "lowercase", Options: "password 'asdf'"}, + })), + "spec", "users") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "cannot assign password") + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 2)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot assign password")) + } + }) + + t.Run("NoTerminators", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal([]v1beta1.PostgresUserSpec{ + {Name: "semicolon", Options: "some ;where"}, + })), + "spec", "users") + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + assert.ErrorContains(t, err, "should match") + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 1)) + assert.Equal(t, status.Details.Causes[0].Field, "spec.users[0].options") + }) + + t.Run("Valid", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalIntoField(t, cluster, + require.Value(yaml.Marshal([]v1beta1.PostgresUserSpec{ + {Name: "normal", Options: "CREATEDB valid until '2006-01-02'"}, + {Name: "very-full", Options: "NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 5"}, + })), + "spec", "users") + + assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) + }) +} diff --git a/internal/testing/validation/postgrescluster_test.go b/internal/testing/validation/postgrescluster_test.go index 53c4ad7c4b..a4c052ee8f 100644 --- a/internal/testing/validation/postgrescluster_test.go +++ b/internal/testing/validation/postgrescluster_test.go @@ -6,515 +6,16 @@ package validation import ( "context" - "fmt" "testing" "gotest.tools/v3/assert" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/util/intstr" "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" ) -func TestPostgresAuthenticationRules(t *testing.T) { - ctx := context.Background() - cc := require.Kubernetes(t) - t.Parallel() - - namespace := require.Namespace(t, cc) - base := v1beta1.NewPostgresCluster() - - // Start with a bunch of required fields. - require.UnmarshalInto(t, &base.Spec, `{ - postgresVersion: 16, - instances: [{ - dataVolumeClaimSpec: { - accessModes: [ReadWriteOnce], - resources: { requests: { storage: 1Mi } }, - }, - }], - }`) - - base.Namespace = namespace.Name - base.Name = "postgres-authentication-rules" - - assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), - "expected this base cluster to be valid") - - t.Run("OneTopLevel", func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: host, hba: anything }, - { users: [alice, bob], hba: anything }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 2)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i)) - assert.Assert(t, cmp.Contains(cause.Message, "cannot be combined")) - } - }) - - t.Run("NoInclude", func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { hba: 'include "/etc/passwd"' }, - { hba: ' include_dir /tmp' }, - { hba: 'include_if_exists postgresql.auto.conf' }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 3)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].hba", i)) - assert.Assert(t, cmp.Contains(cause.Message, "cannot include")) - } - }) - - t.Run("NoStructuredTrust", func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: local, method: trust }, - { connection: hostssl, method: trust }, - { connection: hostgssenc, method: trust }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 3)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].method", i)) - assert.Assert(t, cmp.Contains(cause.Message, "unsafe")) - } - }) - - t.Run("LDAP", func(t *testing.T) { - t.Run("Required", func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: ldap }, - { connection: hostssl, method: ldap, options: {} }, - { connection: hostssl, method: ldap, options: { ldapbinddn: any } }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 3)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) - assert.Assert(t, cmp.Contains(cause.Message, `"ldap" method requires`)) - } - - // These are valid. - - cluster.Spec.Authentication = nil - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: ldap, options: { ldapbasedn: any } }, - { connection: hostssl, method: ldap, options: { ldapprefix: any } }, - { connection: hostssl, method: ldap, options: { ldapsuffix: any } }, - ], - }`) - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) - - t.Run("Mixed", func(t *testing.T) { - // Some options cannot be combined with others. - - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: ldap, options: { ldapbinddn: any, ldapprefix: other } }, - { connection: hostssl, method: ldap, options: { ldapbasedn: any, ldapsuffix: other } }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 2)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) - assert.Assert(t, cmp.Regexp(`cannot use .+? options with .+? options`, cause.Message)) - } - - // These combinations are allowed. - - cluster.Spec.Authentication = nil - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: ldap, options: { ldapprefix: one, ldapsuffix: two } }, - { connection: hostssl, method: ldap, options: { ldapbasedn: one, ldapbinddn: two } }, - { connection: hostssl, method: ldap, options: { - ldapbasedn: one, ldapsearchattribute: two, ldapsearchfilter: three, - } }, - ], - }`) - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) - }) - - t.Run("RADIUS", func(t *testing.T) { - t.Run("Required", func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: radius }, - { connection: hostssl, method: radius, options: {} }, - { connection: hostssl, method: radius, options: { radiusidentifiers: any } }, - { connection: hostssl, method: radius, options: { radiusservers: any } }, - { connection: hostssl, method: radius, options: { radiussecrets: any } }, - ], - }`) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 5)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause) - assert.Assert(t, cmp.Contains(cause.Message, `"radius" method requires`)) - } - - // These are valid. - - cluster.Spec.Authentication = nil - require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ - rules: [ - { connection: hostssl, method: radius, options: { radiusservers: one, radiussecrets: two } }, - { connection: hostssl, method: radius, options: { - radiusservers: one, radiussecrets: two, radiusports: three, - } }, - ], - }`) - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) - }) -} - -func TestPostgresConfigParameters(t *testing.T) { - ctx := context.Background() - cc := require.Kubernetes(t) - t.Parallel() - - namespace := require.Namespace(t, cc) - base := v1beta1.NewPostgresCluster() - - // Start with a bunch of required fields. - require.UnmarshalInto(t, &base.Spec, `{ - postgresVersion: 16, - instances: [{ - dataVolumeClaimSpec: { - accessModes: [ReadWriteOnce], - resources: { requests: { storage: 1Mi } }, - }, - }], - }`) - - base.Namespace = namespace.Name - base.Name = "postgres-config-parameters" - - assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), - "expected this base cluster to be valid") - - t.Run("Allowed", func(t *testing.T) { - for _, tt := range []struct { - key string - value any - }{ - {"archive_timeout", 100}, - {"archive_timeout", "20s"}, - } { - t.Run(tt.key, func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Config, - require.Value(yaml.Marshal(map[string]any{ - "parameters": map[string]any{tt.key: tt.value}, - }))) - - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) - } - }) - - t.Run("Disallowed", func(t *testing.T) { - for _, tt := range []struct { - key string - value any - }{ - {key: "cluster_name", value: "asdf"}, - {key: "config_file", value: "asdf"}, - {key: "data_directory", value: ""}, - {key: "external_pid_file", value: ""}, - {key: "hba_file", value: "one"}, - {key: "hot_standby", value: "off"}, - {key: "ident_file", value: "two"}, - {key: "listen_addresses", value: ""}, - {key: "log_file_mode", value: ""}, - {key: "logging_collector", value: "off"}, - {key: "port", value: 5}, - {key: "wal_log_hints", value: "off"}, - } { - t.Run(tt.key, func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Config, - require.Value(yaml.Marshal(map[string]any{ - "parameters": map[string]any{tt.key: tt.value}, - }))) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 1)) - - // TODO(k8s-1.30) TODO(validation): Move the parameter name from the message to the field path. - assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") - assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.key)) - }) - } - }) - - t.Run("NoConnections", func(t *testing.T) { - for _, tt := range []struct { - key string - value any - }{ - {key: "ssl", value: "off"}, - {key: "ssl_ca_file", value: ""}, - {key: "unix_socket_directories", value: "one"}, - {key: "unix_socket_group", value: "two"}, - } { - t.Run(tt.key, func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Config, - require.Value(yaml.Marshal(map[string]any{ - "parameters": map[string]any{tt.key: tt.value}, - }))) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - }) - } - }) - - t.Run("NoWriteAheadLog", func(t *testing.T) { - for _, tt := range []struct { - key string - value any - }{ - {key: "archive_mode", value: "off"}, - {key: "archive_command", value: "true"}, - {key: "restore_command", value: "true"}, - {key: "recovery_target", value: "immediate"}, - {key: "recovery_target_name", value: "doot"}, - } { - t.Run(tt.key, func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Config, - require.Value(yaml.Marshal(map[string]any{ - "parameters": map[string]any{tt.key: tt.value}, - }))) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - }) - } - }) - - t.Run("wal_level", func(t *testing.T) { - t.Run("Valid", func(t *testing.T) { - cluster := base.DeepCopy() - - cluster.Spec.Config = &v1beta1.PostgresConfigSpec{ - Parameters: map[string]intstr.IntOrString{ - "wal_level": intstr.FromString("logical"), - }, - } - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) - - t.Run("Invalid", func(t *testing.T) { - cluster := base.DeepCopy() - - cluster.Spec.Config = &v1beta1.PostgresConfigSpec{ - Parameters: map[string]intstr.IntOrString{ - "wal_level": intstr.FromString("minimal"), - }, - } - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - assert.ErrorContains(t, err, `"replica" or higher`) - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 1)) - assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters") - assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "wal_level")) - }) - }) - - t.Run("NoReplication", func(t *testing.T) { - for _, tt := range []struct { - key string - value any - }{ - {key: "synchronous_standby_names", value: ""}, - {key: "primary_conninfo", value: ""}, - {key: "primary_slot_name", value: ""}, - {key: "recovery_min_apply_delay", value: ""}, - } { - t.Run(tt.key, func(t *testing.T) { - cluster := base.DeepCopy() - require.UnmarshalInto(t, &cluster.Spec.Config, - require.Value(yaml.Marshal(map[string]any{ - "parameters": map[string]any{tt.key: tt.value}, - }))) - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - }) - } - }) -} - -func TestPostgresUserOptions(t *testing.T) { - ctx := context.Background() - cc := require.Kubernetes(t) - t.Parallel() - - namespace := require.Namespace(t, cc) - base := v1beta1.NewPostgresCluster() - - // Start with a bunch of required fields. - require.UnmarshalInto(t, &base.Spec, `{ - postgresVersion: 16, - instances: [{ - dataVolumeClaimSpec: { - accessModes: [ReadWriteOnce], - resources: { requests: { storage: 1Mi } }, - }, - }], - }`) - - base.Namespace = namespace.Name - base.Name = "postgres-user-options" - - assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), - "expected this base cluster to be valid") - - // See [internal/controller/postgrescluster.TestValidatePostgresUsers] - - t.Run("NoComments", func(t *testing.T) { - cluster := base.DeepCopy() - cluster.Spec.Users = []v1beta1.PostgresUserSpec{ - {Name: "dashes", Options: "ANY -- comment"}, - {Name: "block-open", Options: "/* asdf"}, - {Name: "block-close", Options: " qw */ rt"}, - } - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - assert.ErrorContains(t, err, "cannot contain comments") - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 3)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i)) - assert.Assert(t, cmp.Contains(cause.Message, "cannot contain comments")) - } - }) - - t.Run("NoPassword", func(t *testing.T) { - cluster := base.DeepCopy() - cluster.Spec.Users = []v1beta1.PostgresUserSpec{ - {Name: "uppercase", Options: "SUPERUSER PASSWORD ''"}, - {Name: "lowercase", Options: "password 'asdf'"}, - } - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - assert.ErrorContains(t, err, "cannot assign password") - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 2)) - - for i, cause := range status.Details.Causes { - assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i)) - assert.Assert(t, cmp.Contains(cause.Message, "cannot assign password")) - } - }) - - t.Run("NoTerminators", func(t *testing.T) { - cluster := base.DeepCopy() - cluster.Spec.Users = []v1beta1.PostgresUserSpec{ - {Name: "semicolon", Options: "some ;where"}, - } - - err := cc.Create(ctx, cluster, client.DryRunAll) - assert.Assert(t, apierrors.IsInvalid(err)) - assert.ErrorContains(t, err, "should match") - - status := require.StatusError(t, err) - assert.Assert(t, status.Details != nil) - assert.Assert(t, cmp.Len(status.Details.Causes, 1)) - assert.Equal(t, status.Details.Causes[0].Field, "spec.users[0].options") - }) - - t.Run("Valid", func(t *testing.T) { - cluster := base.DeepCopy() - cluster.Spec.Users = []v1beta1.PostgresUserSpec{ - {Name: "normal", Options: "CREATEDB valid until '2006-01-02'"}, - {Name: "very-full", Options: "NOSUPERUSER NOCREATEDB NOCREATEROLE NOINHERIT NOLOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 5"}, - } - - assert.NilError(t, cc.Create(ctx, cluster, client.DryRunAll)) - }) -} - func TestPostgresUserInterfaceAcrossVersions(t *testing.T) { ctx := context.Background() cc := require.Kubernetes(t) diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go index 47f7382671..06658065b6 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go @@ -141,12 +141,12 @@ type PostgresHBARule struct { // // https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_10_0;f=src/backend/libpq/hba.c#l1501 // https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_17_0;f=src/backend/libpq/hba.c#l1886 -// +kubebuilder:validation:XValidation:rule=`has(self.hba) || self.method != "ldap" || (has(self.options) && ["ldapbasedn","ldapprefix","ldapsuffix"].exists(k, k in self.options))`,message=`the "ldap" method requires an "ldapbasedn", "ldapprefix", or "ldapsuffix" option` -// +kubebuilder:validation:XValidation:rule=`has(self.hba) || self.method != "ldap" || !has(self.options) || [["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].exists_one(a, a.exists(k, k in self.options))`,message=`cannot use "ldapbasedn", "ldapbinddn", "ldapbindpasswd", "ldapsearchattribute", or "ldapsearchfilter" options with "ldapprefix" or "ldapsuffix" options` +// +kubebuilder:validation:XValidation:message=`the "ldap" method requires an "ldapbasedn", "ldapprefix", or "ldapsuffix" option`,rule=`has(self.hba) || self.method != "ldap" || (has(self.options) && ["ldapbasedn","ldapprefix","ldapsuffix"].exists(k, k in self.options))` +// +kubebuilder:validation:XValidation:message=`cannot use "ldapbasedn", "ldapbinddn", "ldapbindpasswd", "ldapsearchattribute", or "ldapsearchfilter" options with "ldapprefix" or "ldapsuffix" options`,rule=`has(self.hba) || self.method != "ldap" || !has(self.options) || 2 > size([["ldapprefix","ldapsuffix"], ["ldapbasedn","ldapbinddn","ldapbindpasswd","ldapsearchattribute","ldapsearchfilter"]].filter(a, a.exists(k, k in self.options)))` // // https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_10_0;f=src/backend/libpq/hba.c#l1539 // https://git.postgresql.org/gitweb/?p=postgresql.git;hb=refs/tags/REL_17_0;f=src/backend/libpq/hba.c#l1945 -// +kubebuilder:validation:XValidation:rule=`has(self.hba) || self.method != "radius" || (has(self.options) && ["radiusservers","radiussecrets"].all(k, k in self.options))`,message=`the "radius" method requires "radiusservers" and "radiussecrets" options` +// +kubebuilder:validation:XValidation:message=`the "radius" method requires "radiusservers" and "radiussecrets" options`,rule=`has(self.hba) || self.method != "radius" || (has(self.options) && ["radiusservers","radiussecrets"].all(k, k in self.options))` // // +structType=atomic type PostgresHBARuleSpec struct {