Merge pull request #1799 from splunk/feature/database-controllers-base-refactor-2

  fix(postgres): address PR review — API validation, type cleanup, controller correctness

Author: mploski

api/v4/postgrescluster_types.go

@@ -44,7 +44,7 @@ type ManagedRole struct {
 // Validation rules ensure immutability of Class, and that Storage and PostgresVersion can only be set once and cannot be removed or downgraded.
 // +kubebuilder:validation:XValidation:rule="!has(oldSelf.postgresVersion) || (has(self.postgresVersion) && int(self.postgresVersion.split('.')[0]) >= int(oldSelf.postgresVersion.split('.')[0]))",messageExpression="!has(self.postgresVersion) ? 'postgresVersion cannot be removed once set (was: ' + oldSelf.postgresVersion + ')' : 'postgresVersion major version cannot be downgraded (from: ' + oldSelf.postgresVersion + ', to: ' + self.postgresVersion + ')'"
 // +kubebuilder:validation:XValidation:rule="!has(oldSelf.storage) || (has(self.storage) && quantity(self.storage).compareTo(quantity(oldSelf.storage)) >= 0)",messageExpression="!has(self.storage) ? 'storage cannot be removed once set (was: ' + string(oldSelf.storage) + ')' : 'storage size cannot be decreased (from: ' + string(oldSelf.storage) + ', to: ' + string(self.storage) + ')'"
-// +kubebuilder:validation:XValidation:rule="!has(self.connectionPoolerConfig)",message="connectionPoolerConfig cannot be overridden on PostgresCluster"
+
 type PostgresClusterSpec struct {
 	// This field is IMMUTABLE after creation.
 	// +kubebuilder:validation:Required
@@ -92,14 +92,9 @@ type PostgresClusterSpec struct {
 
 	// ConnectionPoolerEnabled controls whether PgBouncer connection pooling is deployed for this cluster.
 	// When set, takes precedence over the class-level connectionPoolerEnabled value.
-	// +kubebuilder:default=false
 	// +optional
 	ConnectionPoolerEnabled *bool `json:"connectionPoolerEnabled,omitempty"`
 
-	// Only takes effect when connection pooling is enabled.
-	// +optional
-	ConnectionPoolerConfig *ConnectionPoolerConfig `json:"connectionPoolerConfig,omitempty"`
-
 	// ManagedRoles contains PostgreSQL roles that should be created in the cluster.
 	// This field supports Server-Side Apply with per-role granularity, allowing
 	// multiple PostgresDatabase controllers to manage different roles independently.
@@ -185,6 +180,7 @@ type ConnectionPoolerStatus struct {
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
 // PostgresCluster is the Schema for the postgresclusters API.
+// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) <= 50",message="name must be 50 characters or fewer to accommodate derived resource names"
 type PostgresCluster struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

api/v4/postgresclusterclass_types.go

@@ -24,8 +24,10 @@ import (
 
 // +kubebuilder:validation:XValidation:rule="!has(self.cnpg) || self.provisioner == 'postgresql.cnpg.io'",message="cnpg config can only be set when provisioner is postgresql.cnpg.io"
 // +kubebuilder:validation:XValidation:rule="!has(self.config) || !has(self.config.connectionPoolerEnabled) || !self.config.connectionPoolerEnabled || (has(self.cnpg) && has(self.cnpg.connectionPooler))",message="cnpg.connectionPooler must be set when config.connectionPoolerEnabled is true"
+// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="PostgresClusterClass is immutable after creation"
 // PostgresClusterClassSpec defines the desired state of PostgresClusterClass.
 // PostgresClusterClass is immutable after creation - it serves as a template for Cluster CRs.
+
 type PostgresClusterClassSpec struct {
 	// Provisioner identifies which database provisioner to use.
 	// Currently supported: "postgresql.cnpg.io" (CloudNativePG)
@@ -174,9 +176,9 @@ type PostgresClusterClassStatus struct {
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:scope=Cluster
 // +kubebuilder:printcolumn:name="Provisioner",type=string,JSONPath=`.spec.provisioner`
-// +kubebuilder:printcolumn:name="Instances",type=integer,JSONPath=`.spec.postgresClusterConfig.instances`
-// +kubebuilder:printcolumn:name="Storage",type=string,JSONPath=`.spec.postgresClusterConfig.storage`
-// +kubebuilder:printcolumn:name="Version",type=string,JSONPath=`.spec.postgresClusterConfig.postgresVersion`
+// +kubebuilder:printcolumn:name="Instances",type=integer,JSONPath=`.spec.config.instances`
+// +kubebuilder:printcolumn:name="Storage",type=string,JSONPath=`.spec.config.storage`
+// +kubebuilder:printcolumn:name="Version",type=string,JSONPath=`.spec.config.postgresVersion`
 // +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 

api/v4/postgresdatabase_types.go

@@ -23,6 +23,7 @@ import (
 
 // PostgresDatabaseSpec defines the desired state of PostgresDatabase.
 // +kubebuilder:validation:XValidation:rule="self.clusterRef == oldSelf.clusterRef",message="clusterRef is immutable"
+// +kubebuilder:validation:XValidation:rule="self.clusterRef.name != ''",message="clusterRef.name must not be empty"
 type PostgresDatabaseSpec struct {
 	// Reference to Postgres Cluster managed by postgresCluster controller
 	// +kubebuilder:validation:Required
@@ -37,7 +38,9 @@ type PostgresDatabaseSpec struct {
 
 type DatabaseDefinition struct {
 	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=30
+	// +kubebuilder:validation:Pattern=`^[a-z_][a-z0-9_]*$`
 	Name       string   `json:"name"`
 	Extensions []string `json:"extensions,omitempty"`
 	// +kubebuilder:validation:Enum=Delete;Retain
@@ -51,7 +54,7 @@ type DatabaseInfo struct {
 	DatabaseRef        *corev1.LocalObjectReference `json:"databaseRef,omitempty"`
 	AdminUserSecretRef *corev1.SecretKeySelector    `json:"adminUserSecretRef,omitempty"`
 	RWUserSecretRef    *corev1.SecretKeySelector    `json:"rwUserSecretRef,omitempty"`
-	ConfigMapRef       *corev1.LocalObjectReference `json:"configMap,omitempty"`
+	ConfigMapRef       *corev1.LocalObjectReference `json:"configMapRef,omitempty"`
 }
 
 // PostgresDatabaseStatus defines the observed state of PostgresDatabase.
@@ -74,6 +77,7 @@ type PostgresDatabaseStatus struct {
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 
 // PostgresDatabase is the Schema for the postgresdatabases API.
+// +kubebuilder:validation:XValidation:rule="size(self.metadata.name) <= 50",message="name must be 50 characters or fewer to accommodate derived resource names"
 type PostgresDatabase struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

api/v4/zz_generated.deepcopy.go

@@ -18,13 +18,13 @@ spec:
     - jsonPath: .spec.provisioner
       name: Provisioner
       type: string
-    - jsonPath: .spec.postgresClusterConfig.instances
+    - jsonPath: .spec.config.instances
       name: Instances
       type: integer
-    - jsonPath: .spec.postgresClusterConfig.storage
+    - jsonPath: .spec.config.storage
       name: Storage
       type: string
-    - jsonPath: .spec.postgresClusterConfig.postgresVersion
+    - jsonPath: .spec.config.postgresVersion
       name: Version
       type: string
     - jsonPath: .status.phase
@@ -58,9 +58,6 @@ spec:
           metadata:
             type: object
           spec:
-            description: |-
-              PostgresClusterClassSpec defines the desired state of PostgresClusterClass.
-              PostgresClusterClass is immutable after creation - it serves as a template for Cluster CRs.
             properties:
               cnpg:
                 description: |-
@@ -251,6 +248,8 @@ spec:
                 is true
               rule: '!has(self.config) || !has(self.config.connectionPoolerEnabled)
                 || !self.config.connectionPoolerEnabled || (has(self.cnpg) && has(self.cnpg.connectionPooler))'
+            - message: PostgresClusterClass is immutable after creation
+              rule: self == oldSelf
           status:
             description: PostgresClusterClassStatus defines the observed state of
               PostgresClusterClass.

config/crd/bases/enterprise.splunk.com_postgresclusterclasses.yaml

@@ -47,9 +47,6 @@ spec:
           metadata:
             type: object
           spec:
-            description: |-
-              PostgresClusterSpec defines the desired state of PostgresCluster.
-              Validation rules ensure immutability of Class, and that Storage and PostgresVersion can only be set once and cannot be removed or downgraded.
             properties:
               class:
                 description: This field is IMMUTABLE after creation.
@@ -66,37 +63,7 @@ spec:
                 - Delete
                 - Retain
                 type: string
-              connectionPoolerConfig:
-                description: Only takes effect when connection pooling is enabled.
-                properties:
-                  config:
-                    additionalProperties:
-                      type: string
-                    description: |-
-                      Config contains PgBouncer configuration parameters.
-                      Passed directly to CNPG Pooler spec.pgbouncer.parameters.
-                      See: https://cloudnative-pg.io/docs/1.28/connection_pooling/#pgbouncer-configuration-options
-                    type: object
-                  instances:
-                    default: 3
-                    description: |-
-                      Instances is the number of PgBouncer pod replicas.
-                      Higher values provide better availability and load distribution.
-                    format: int32
-                    maximum: 10
-                    minimum: 1
-                    type: integer
-                  mode:
-                    default: transaction
-                    description: Mode defines the connection pooling strategy.
-                    enum:
-                    - session
-                    - transaction
-                    - statement
-                    type: string
-                type: object
               connectionPoolerEnabled:
-                default: false
                 description: |-
                   ConnectionPoolerEnabled controls whether PgBouncer connection pooling is deployed for this cluster.
                   When set, takes precedence over the class-level connectionPoolerEnabled value.
@@ -270,8 +237,6 @@ spec:
                 '' + string(self.storage) + '')'''
               rule: '!has(oldSelf.storage) || (has(self.storage) && quantity(self.storage).compareTo(quantity(oldSelf.storage))
                 >= 0)'
-            - message: connectionPoolerConfig cannot be overridden on PostgresCluster
-              rule: '!has(self.connectionPoolerConfig)'
           status:
             description: PostgresClusterStatus defines the observed state of PostgresCluster.
             properties:
@@ -463,6 +428,10 @@ spec:
                 type: object
             type: object
         type: object
+        x-kubernetes-validations:
+        - message: name must be 50 characters or fewer to accommodate derived resource
+            names
+          rule: size(self.metadata.name) <= 50
     served: true
     storage: true
     subresources:

config/crd/bases/enterprise.splunk.com_postgresclusters.yaml

@@ -79,6 +79,8 @@ spec:
                       type: array
                     name:
                       maxLength: 30
+                      minLength: 1
+                      pattern: ^[a-z_][a-z0-9_]*$
                       type: string
                   required:
                   - name
@@ -96,6 +98,8 @@ spec:
             x-kubernetes-validations:
             - message: clusterRef is immutable
               rule: self.clusterRef == oldSelf.clusterRef
+            - message: clusterRef.name must not be empty
+              rule: self.clusterRef.name != ''
           status:
             description: PostgresDatabaseStatus defines the observed state of PostgresDatabase.
             properties:
@@ -182,7 +186,7 @@ spec:
                       - key
                       type: object
                       x-kubernetes-map-type: atomic
-                    configMap:
+                    configMapRef:
                       description: |-
                         LocalObjectReference contains enough information to let you locate the
                         referenced object inside the same namespace.
@@ -253,6 +257,10 @@ spec:
                 type: string
             type: object
         type: object
+        x-kubernetes-validations:
+        - message: name must be 50 characters or fewer to accommodate derived resource
+            names
+          rule: size(self.metadata.name) <= 50
     served: true
     storage: true
     subresources:

config/crd/bases/enterprise.splunk.com_postgresdatabases.yaml

@@ -47,10 +47,34 @@ metadata:
 spec:
   preserveUnknownFields: false
 
----    
+---
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   name: searchheadclusters.enterprise.splunk.com
+spec:
+  preserveUnknownFields: false
+
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: postgresclusters.enterprise.splunk.com
+spec:
+  preserveUnknownFields: false
+
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: postgresclusterclasses.enterprise.splunk.com
+spec:
+  preserveUnknownFields: false
+
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: postgresdatabases.enterprise.splunk.com
 spec:
   preserveUnknownFields: false