diff --git a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go index c8f2eaf3d..5271b3fec 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go @@ -24,10 +24,13 @@ import ( "testing" admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/api/resource" authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/pkg/splunk/enterprise/validation" @@ -571,3 +574,235 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { assert.Contains(t, resp.Result.Message, "unknown auth method") }) } + +func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { + s := runtime.NewScheme() + enterpriseApi.AddToScheme(s) + b := fake.NewClientBuilder().WithScheme(s) + for _, obj := range objects { + b = b.WithRuntimeObjects(obj) + } + return b +} + +func TestCrossResourceValidationIntegration(t *testing.T) { + prodClass := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), + }, + }, + } + + fakeClient := newFakeReader(prodClass).Build() + + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + Client: fakeClient, + }) + + tests := []struct { + name string + obj *enterpriseApi.PostgresCluster + wantAllowed bool + wantMessage string + }{ + { + name: "allowed - inherits all from class", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantAllowed: true, + }, + { + name: "rejected - class not found", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + }, + wantAllowed: false, + wantMessage: "PostgresClusterClass not found", + }, + { + name: "rejected - version below class floor", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("16"), + }, + }, + wantAllowed: false, + wantMessage: "postgresVersion cannot be lower than class default", + }, + { + name: "rejected - pooler enabled but class has no cnpg.connectionPooler", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + wantAllowed: false, + wantMessage: "connection pooler requires cnpg.connectionPooler configuration", + }, + { + name: "allowed - higher version", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("18"), + }, + }, + wantAllowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterAdmissionReview(t, "uid-xref-"+tt.name, admissionv1.Create, tt.obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + }) + } +} + +func TestCrossResourceValidationUpdateIntegration(t *testing.T) { + prodClass := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), + }, + }, + } + + fakeClient := newFakeReader(prodClass).Build() + + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + Client: fakeClient, + }) + + oldObj := &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17"), + }, + } + + tests := []struct { + name string + newObj *enterpriseApi.PostgresCluster + wantAllowed bool + wantMessage string + }{ + { + name: "allowed - upgrade version", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("18"), + }, + }, + wantAllowed: true, + }, + { + name: "rejected - downgrade version below class floor", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("16"), + }, + }, + wantAllowed: false, + wantMessage: "postgresVersion cannot be lower than class default", + }, + { + name: "rejected - enable pooler without cnpg config", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + wantAllowed: false, + wantMessage: "connection pooler requires cnpg.connectionPooler configuration", + }, + { + name: "allowed - no changes", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17"), + }, + }, + wantAllowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterAdmissionReview(t, "uid-xref-update-"+tt.name, admissionv1.Update, tt.newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + }) + } +} + +func TestCrossResourceValidationDisabledWithoutClient(t *testing.T) { + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + }) + + obj := &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + } + + ar := newPostgresClusterAdmissionReview(t, "uid-no-client", admissionv1.Create, obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.True(t, resp.Allowed, "without a client, cross-resource validation should be skipped") +} diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go index 7fc724c4f..02763f974 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go @@ -17,19 +17,25 @@ limitations under the License. package webhook import ( + "context" + "fmt" + "strconv" + + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" enterpriseApi "github.com/splunk/splunk-operator/api/v4" - hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" + core "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" ) // ValidatePostgresClusterCreate validates a PostgresCluster on CREATE. -func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.ErrorList { +func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { var allErrs field.ErrorList if len(obj.Spec.PgHBA) > 0 { pgHBAPath := field.NewPath("spec").Child("pgHBA") - for _, re := range hba.ValidateRules(obj.Spec.PgHBA) { + for _, re := range core.ValidateRules(obj.Spec.PgHBA) { allErrs = append(allErrs, field.Invalid( pgHBAPath.Index(re.Index), obj.Spec.PgHBA[re.Index], @@ -37,12 +43,93 @@ func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.Err } } + if reader != nil { + allErrs = append(allErrs, validateAgainstClass(obj, reader)...) + } + return allErrs } // ValidatePostgresClusterUpdate validates a PostgresCluster on UPDATE. -func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { - return ValidatePostgresClusterCreate(obj) +func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { + return ValidatePostgresClusterCreate(obj, reader) +} + +func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { + var allErrs field.ErrorList + + class := &enterpriseApi.PostgresClusterClass{} + if err := reader.Get(context.Background(), client.ObjectKey{Name: obj.Spec.Class}, class); err != nil { + classPath := field.NewPath("spec").Child("class") + if apierrors.IsNotFound(err) { + allErrs = append(allErrs, field.Invalid(classPath, obj.Spec.Class, + "referenced PostgresClusterClass not found")) + } else { + allErrs = append(allErrs, field.InternalError(classPath, + fmt.Errorf("failed to look up PostgresClusterClass %q: %w", obj.Spec.Class, err))) + } + return allErrs + } + + merged, err := core.GetMergedConfig(class, obj) + if err != nil { + specPath := field.NewPath("spec") + if merged == nil || merged.Spec.Instances == nil { + allErrs = append(allErrs, field.Required(specPath.Child("instances"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + if merged == nil || merged.Spec.PostgresVersion == nil { + allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + if merged == nil || merged.Spec.Storage == nil { + allErrs = append(allErrs, field.Required(specPath.Child("storage"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + return allErrs + } + + if classConfig := class.Spec.Config; classConfig != nil { + // Class version acts as a minimum floor for compliance; clusters may override higher but not lower. + if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { + clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion) + classMajor, classMinor := parseVersion(*classConfig.PostgresVersion) + if clusterMajor > 0 && classMajor > 0 { + versionTooLow := clusterMajor < classMajor || + (clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor) + if versionTooLow { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("postgresVersion"), + *obj.Spec.PostgresVersion, + "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + } + } + } + + poolerEnabled := (obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled) || + (obj.Spec.ConnectionPoolerEnabled == nil && classConfig.ConnectionPoolerEnabled != nil && *classConfig.ConnectionPoolerEnabled) + if poolerEnabled && (class.Spec.CNPG == nil || class.Spec.CNPG.ConnectionPooler == nil) { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("connectionPoolerEnabled"), + true, + "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass")) + } + } + + _ = merged + return allErrs +} + +func parseVersion(version string) (major, minor int) { + for i, ch := range version { + if ch == '.' { + major, _ = strconv.Atoi(version[:i]) + minor, _ = strconv.Atoi(version[i+1:]) + return major, minor + } + } + major, _ = strconv.Atoi(version) + return major, -1 } // GetPostgresClusterWarningsOnCreate returns warnings for PostgresCluster CREATE. diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go index 56ff34c9c..e678b5930 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go @@ -19,8 +19,15 @@ package webhook import ( "testing" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidatePostgresClusterCreate(t *testing.T) { @@ -118,7 +125,7 @@ func TestValidatePostgresClusterCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterCreate(tt.obj) + errs := ValidatePostgresClusterCreate(tt.obj, nil) assert.Len(t, errs, tt.wantErrCount, "unexpected error count") if tt.wantErrField != "" && len(errs) > 0 { assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") @@ -168,7 +175,7 @@ func TestValidatePostgresClusterUpdate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj) + errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj, nil) assert.Len(t, errs, tt.wantErrCount, "unexpected error count") }) } @@ -190,3 +197,336 @@ func TestGetPostgresClusterWarningsOnUpdate(t *testing.T) { } assert.Empty(t, GetPostgresClusterWarningsOnUpdate(obj, oldObj)) } + +func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { + s := runtime.NewScheme() + enterpriseApi.AddToScheme(s) + b := fake.NewClientBuilder().WithScheme(s) + for _, obj := range objects { + b = b.WithRuntimeObjects(obj) + } + return b +} + +func TestValidateAgainstClass(t *testing.T) { + classWithDefaults := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), + }, + }, + } + + classWithMinorVersion := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod-pinned"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17.2"), + }, + }, + } + + classWithPoolerEnabled := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "pooler-class"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(true), + }, + CNPG: &enterpriseApi.CNPGConfig{ + ConnectionPooler: &enterpriseApi.ConnectionPoolerConfig{}, + }, + }, + } + + tests := []struct { + name string + class *enterpriseApi.PostgresClusterClass + obj *enterpriseApi.PostgresCluster + wantErrCount int + wantErrField string + wantErrMsg string + }{ + { + name: "class not found", + class: nil, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + }, + wantErrCount: 1, + wantErrField: "spec.class", + wantErrMsg: "referenced PostgresClusterClass not found", + }, + { + name: "valid - no overrides", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantErrCount: 0, + }, + { + name: "valid - same postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - higher postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("18"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - lower postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("16"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17)", + }, + { + name: "valid - minor version ignored when class has major only", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17.2"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - lower minor ignored when class has major only", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17.0"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - cluster minor equal to class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptr.To("17.2"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - cluster minor higher than class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptr.To("17.5"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - cluster minor lower than class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptr.To("17.1"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17.2)", + }, + { + name: "invalid - cluster major lower even with higher minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptr.To("16.9"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17.2)", + }, + { + name: "valid - cluster major higher than class with minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptr.To("18"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - pooler enabled but class has no cnpg.connectionPooler", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + wantErrCount: 1, + wantErrField: "spec.connectionPoolerEnabled", + wantErrMsg: "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass", + }, + { + name: "valid - pooler disabled, class has no cnpg config", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptr.To(false), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - pooler enabled and class has cnpg.connectionPooler", + class: classWithPoolerEnabled, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "pooler-class", + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - pooler unset (inherits class)", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantErrCount: 0, + }, + { + name: "invalid - class enables pooler but missing cnpg config", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "pooler-no-cnpg"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "pooler-no-cnpg"}, + }, + wantErrCount: 1, + wantErrField: "spec.connectionPoolerEnabled", + wantErrMsg: "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass", + }, + { + name: "invalid - class has no config, cluster missing required fields", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "bare"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "bare"}, + }, + wantErrCount: 3, + wantErrField: "spec.instances", + wantErrMsg: "must be set in PostgresCluster or PostgresClusterClass", + }, + { + name: "invalid - class config missing storage, cluster doesn't provide it", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "no-storage"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + PostgresVersion: ptr.To("17"), + }, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "no-storage"}, + }, + wantErrCount: 1, + wantErrField: "spec.storage", + wantErrMsg: "must be set in PostgresCluster or PostgresClusterClass", + }, + { + name: "valid - cluster fills in what class is missing", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "minimal"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{}, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "minimal", + Instances: ptr.To(int32(1)), + PostgresVersion: ptr.To("17"), + Storage: ptr.To(resource.MustParse("10Gi")), + }, + }, + wantErrCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + builder := newFakeReader() + if tt.class != nil { + builder = newFakeReader(tt.class) + } + reader := builder.Build() + + errs := ValidatePostgresClusterCreate(tt.obj, reader) + require.Len(t, errs, tt.wantErrCount, "unexpected error count: %v", errs) + if tt.wantErrField != "" && len(errs) > 0 { + assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") + } + if tt.wantErrMsg != "" && len(errs) > 0 { + assert.Contains(t, errs[0].Detail, tt.wantErrMsg, "unexpected error message") + } + }) + } +} diff --git a/pkg/postgresql/cluster/core/cluster.go b/pkg/postgresql/cluster/core/cluster.go index a09e95780..0a1cadb49 100644 --- a/pkg/postgresql/cluster/core/cluster.go +++ b/pkg/postgresql/cluster/core/cluster.go @@ -142,7 +142,7 @@ func PostgresClusterService(ctx context.Context, rc *ReconcileContext, req ctrl. } // Merge PostgresClusterSpec on top of PostgresClusterClass defaults. - mergedConfig, err := getMergedConfig(clusterClass, postgresCluster) + mergedConfig, err := GetMergedConfig(clusterClass, postgresCluster) if err != nil { logger.Error(err, "Failed to merge PostgresCluster configuration") rc.emitWarning(postgresCluster, EventConfigMergeFailed, fmt.Sprintf("Failed to merge configuration: %v", err)) @@ -1380,9 +1380,9 @@ func isIntermediateState(state pgcConstants.State) bool { } } -// getMergedConfig overlays PostgresCluster spec on top of the class defaults. +// GetMergedConfig overlays PostgresCluster spec on top of the class defaults. // Class values are used only where the cluster spec is silent. -func getMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterprisev4.PostgresCluster) (*MergedConfig, error) { +func GetMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterprisev4.PostgresCluster) (*MergedConfig, error) { result := cluster.Spec.DeepCopy() // Config is optional on the class — apply defaults only when provided. @@ -1411,7 +1411,7 @@ func getMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterpri } if result.Instances == nil || result.PostgresVersion == nil || result.Storage == nil { - return nil, fmt.Errorf("invalid configuration for class %s: instances, postgresVersion and storage are required", class.Name) + return &MergedConfig{Spec: result, CNPG: class.Spec.CNPG}, fmt.Errorf("invalid configuration for class %s: instances, postgresVersion and storage are required", class.Name) } if result.PostgreSQLConfig == nil { result.PostgreSQLConfig = make(map[string]string) diff --git a/pkg/postgresql/cluster/core/cluster_unit_test.go b/pkg/postgresql/cluster/core/cluster_unit_test.go index 6df965cc6..deadf88cd 100644 --- a/pkg/postgresql/cluster/core/cluster_unit_test.go +++ b/pkg/postgresql/cluster/core/cluster_unit_test.go @@ -404,7 +404,7 @@ func TestGetMergedConfig(t *testing.T) { }, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) assert.Equal(t, int32(5), *cfg.Spec.Instances) @@ -419,7 +419,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) assert.Equal(t, int32(1), *cfg.Spec.Instances) @@ -437,7 +437,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - _, err := getMergedConfig(emptyClass, cluster) + _, err := GetMergedConfig(emptyClass, cluster) require.Error(t, err) }) @@ -447,7 +447,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) require.NotNil(t, cfg.CNPG) @@ -469,7 +469,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(classWithNoMaps, cluster) + cfg, err := GetMergedConfig(classWithNoMaps, cluster) require.NoError(t, err) assert.NotNil(t, cfg.Spec.PostgreSQLConfig) diff --git a/pkg/splunk/enterprise/validation/registry.go b/pkg/splunk/enterprise/validation/registry.go index 5eab98402..3d7e47897 100644 --- a/pkg/splunk/enterprise/validation/registry.go +++ b/pkg/splunk/enterprise/validation/registry.go @@ -18,6 +18,7 @@ package validation import ( "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" enterpriseApi "github.com/splunk/splunk-operator/api/v4" pgwebhook "github.com/splunk/splunk-operator/pkg/postgresql/cluster/adapter/webhook" @@ -195,8 +196,18 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterGVR: &GenericValidator[*enterpriseApi.PostgresCluster]{ - ValidateCreateFunc: pgwebhook.ValidatePostgresClusterCreate, - ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterUpdate, + ValidateCreateFunc: func(obj *enterpriseApi.PostgresCluster) field.ErrorList { + return pgwebhook.ValidatePostgresClusterCreate(obj, nil) + }, + ValidateUpdateFunc: func(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { + return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, nil) + }, + ValidateCreateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { + return pgwebhook.ValidatePostgresClusterCreate(obj, vc.Client) + }, + ValidateUpdateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { + return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, vc.Client) + }, WarningsOnCreateFunc: pgwebhook.GetPostgresClusterWarningsOnCreate, WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterWarningsOnUpdate, GroupKind: schema.GroupKind{