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.

Author: cbandy

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)

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...))
+}

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`)
+}

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))
+})
+})
+}