From ef881c3c90f9d1a656f93682146534cb6d1e1443 Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Mon, 10 Feb 2025 14:21:38 -0600 Subject: [PATCH 1/2] Bump controller-gen to v0.17.2 --- Makefile | 2 +- ...stgres-operator.crunchydata.com_crunchybridgeclusters.yaml | 2 +- .../crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml | 2 +- .../bases/postgres-operator.crunchydata.com_pgupgrades.yaml | 2 +- .../postgres-operator.crunchydata.com_postgresclusters.yaml | 2 +- .../postgres-operator.crunchydata.com/v1beta1/shared_types.go | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 92b8057ebc..a4bf44629b 100644 --- a/Makefile +++ b/Makefile @@ -308,7 +308,7 @@ endef CONTROLLER ?= hack/tools/controller-gen tools: tools/controller-gen tools/controller-gen: - $(call go-get-tool,$(CONTROLLER),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.16.5) + $(call go-get-tool,$(CONTROLLER),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.2) ENVTEST ?= hack/tools/setup-envtest tools: tools/setup-envtest diff --git a/config/crd/bases/postgres-operator.crunchydata.com_crunchybridgeclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_crunchybridgeclusters.yaml index 6938d25da0..080683f01b 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_crunchybridgeclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_crunchybridgeclusters.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.16.5 + controller-gen.kubebuilder.io/version: v0.17.2 name: crunchybridgeclusters.postgres-operator.crunchydata.com spec: group: postgres-operator.crunchydata.com diff --git a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml index 9b322b1365..8bea9559f7 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.16.5 + controller-gen.kubebuilder.io/version: v0.17.2 name: pgadmins.postgres-operator.crunchydata.com spec: group: postgres-operator.crunchydata.com diff --git a/config/crd/bases/postgres-operator.crunchydata.com_pgupgrades.yaml b/config/crd/bases/postgres-operator.crunchydata.com_pgupgrades.yaml index 39b7bdfefd..d4c9f95bad 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_pgupgrades.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_pgupgrades.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.16.5 + controller-gen.kubebuilder.io/version: v0.17.2 name: pgupgrades.postgres-operator.crunchydata.com spec: group: postgres-operator.crunchydata.com diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index edae909760..cc7dc9d847 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.16.5 + controller-gen.kubebuilder.io/version: v0.17.2 name: postgresclusters.postgres-operator.crunchydata.com spec: group: postgres-operator.crunchydata.com diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go index 79de9ae5f3..baf429f513 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go @@ -64,7 +64,7 @@ type ServiceSpec struct { // // +optional // +kubebuilder:validation:Enum={Cluster,Local} - InternalTrafficPolicy *corev1.ServiceInternalTrafficPolicyType `json:"internalTrafficPolicy,omitempty"` + InternalTrafficPolicy *corev1.ServiceInternalTrafficPolicy `json:"internalTrafficPolicy,omitempty"` // More info: https://kubernetes.io/docs/concepts/services-networking/service/#traffic-policies // --- @@ -75,7 +75,7 @@ type ServiceSpec struct { // // +optional // +kubebuilder:validation:Enum={Cluster,Local} - ExternalTrafficPolicy *corev1.ServiceExternalTrafficPolicyType `json:"externalTrafficPolicy,omitempty"` + ExternalTrafficPolicy *corev1.ServiceExternalTrafficPolicy `json:"externalTrafficPolicy,omitempty"` } // Sidecar defines the configuration of a sidecar container From 0b4fbd33ee3080f0e8217d12993bbe9e0033b80c Mon Sep 17 00:00:00 2001 From: Chris Bandy Date: Fri, 3 Jan 2025 13:39:24 -0600 Subject: [PATCH 2/2] Change PostgresIdentifier to a type alias The type existed to avoid schema repetition with controller-gen, but recent versions can do that using aliases. This eliminates the need for some conversions. --- ...ator.crunchydata.com_postgresclusters.yaml | 3 --- .../controller/postgrescluster/postgres.go | 17 ++++++------- .../postgrescluster/postgres_test.go | 6 ++--- internal/pgadmin/users.go | 2 +- internal/pgadmin/users_test.go | 2 +- internal/postgres/users.go | 8 +++--- internal/postgres/users_test.go | 10 ++++---- .../v1beta1/postgres_types.go | 25 ++++++++++++------- 8 files changed, 37 insertions(+), 36 deletions(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index cc7dc9d847..aa551cba21 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -17087,9 +17087,6 @@ spec: database from this list does NOT revoke access. This field is ignored for the "postgres" user. items: - description: |- - PostgreSQL identifiers are limited in length but may contain any character. - More info: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS maxLength: 63 minLength: 1 type: string diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index c0660b9707..0806445586 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -43,7 +43,7 @@ import ( func (r *Reconciler) generatePostgresUserSecret( cluster *v1beta1.PostgresCluster, spec *v1beta1.PostgresUserSpec, existing *corev1.Secret, ) (*corev1.Secret, error) { - username := string(spec.Name) + username := spec.Name intent := &corev1.Secret{ObjectMeta: naming.PostgresUserSecret(cluster, username)} intent.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Secret")) initialize.Map(&intent.Data) @@ -100,7 +100,7 @@ func (r *Reconciler) generatePostgresUserSecret( // When a database has been specified, include it and a connection URI. // - https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING if len(spec.Databases) > 0 { - database := string(spec.Databases[0]) + database := spec.Databases[0] intent.Data["dbname"] = []byte(database) intent.Data["uri"] = []byte((&url.URL{ @@ -133,7 +133,7 @@ func (r *Reconciler) generatePostgresUserSecret( intent.Data["pgbouncer-port"] = []byte(port) if len(spec.Databases) > 0 { - database := string(spec.Databases[0]) + database := spec.Databases[0] intent.Data["pgbouncer-uri"] = []byte((&url.URL{ Scheme: "postgresql", @@ -216,9 +216,7 @@ func (r *Reconciler) reconcilePostgresDatabases( } } else { for _, user := range cluster.Spec.Users { - for _, database := range user.Databases { - databases.Insert(string(database)) - } + databases.Insert(user.Databases...) } } @@ -379,10 +377,9 @@ func (r *Reconciler) reconcilePostgresUserSecrets( r.Recorder.Event(cluster, corev1.EventTypeWarning, "InvalidUser", allErrors.ToAggregate().Error()) } else { - identifier := v1beta1.PostgresIdentifier(cluster.Name) specUsers = []v1beta1.PostgresUserSpec{{ - Name: identifier, - Databases: []v1beta1.PostgresIdentifier{identifier}, + Name: cluster.Name, + Databases: []string{cluster.Name}, }} } } @@ -390,7 +387,7 @@ func (r *Reconciler) reconcilePostgresUserSecrets( // Index user specifications by PostgreSQL user name. userSpecs := make(map[string]*v1beta1.PostgresUserSpec, len(specUsers)) for i := range specUsers { - userSpecs[string(specUsers[i].Name)] = &specUsers[i] + userSpecs[specUsers[i].Name] = &specUsers[i] } secrets := &corev1.SecretList{} diff --git a/internal/controller/postgrescluster/postgres_test.go b/internal/controller/postgrescluster/postgres_test.go index 5395b6f95f..a6966fc802 100644 --- a/internal/controller/postgrescluster/postgres_test.go +++ b/internal/controller/postgrescluster/postgres_test.go @@ -163,7 +163,7 @@ func TestGeneratePostgresUserSecret(t *testing.T) { } // Present when specified. - spec.Databases = []v1beta1.PostgresIdentifier{"db1"} + spec.Databases = []string{"db1"} secret, err = reconciler.generatePostgresUserSecret(cluster, &spec, nil) assert.NilError(t, err) @@ -180,7 +180,7 @@ func TestGeneratePostgresUserSecret(t *testing.T) { } // Only the first in the list. - spec.Databases = []v1beta1.PostgresIdentifier{"first", "asdf"} + spec.Databases = []string{"first", "asdf"} secret, err = reconciler.generatePostgresUserSecret(cluster, &spec, nil) assert.NilError(t, err) @@ -214,7 +214,7 @@ func TestGeneratePostgresUserSecret(t *testing.T) { // Includes a URI when possible. spec := *spec - spec.Databases = []v1beta1.PostgresIdentifier{"yes", "no"} + spec.Databases = []string{"yes", "no"} secret, err = reconciler.generatePostgresUserSecret(cluster, &spec, nil) assert.NilError(t, err) diff --git a/internal/pgadmin/users.go b/internal/pgadmin/users.go index 6c93fcd5d2..ef51978e8f 100644 --- a/internal/pgadmin/users.go +++ b/internal/pgadmin/users.go @@ -239,7 +239,7 @@ with create_app().app_context():`, if err == nil { err = encoder.Encode(map[string]interface{}{ "username": spec.Name, - "password": passwords[string(spec.Name)], + "password": passwords[spec.Name], }) } } diff --git a/internal/pgadmin/users_test.go b/internal/pgadmin/users_test.go index 17bec23204..4dba70f81a 100644 --- a/internal/pgadmin/users_test.go +++ b/internal/pgadmin/users_test.go @@ -235,7 +235,7 @@ with create_app().app_context(): []v1beta1.PostgresUserSpec{ { Name: "user-no-options", - Databases: []v1beta1.PostgresIdentifier{"db1"}, + Databases: []string{"db1"}, }, { Name: "user-no-databases", diff --git a/internal/postgres/users.go b/internal/postgres/users.go index b16be66152..0caa09cb42 100644 --- a/internal/postgres/users.go +++ b/internal/postgres/users.go @@ -106,7 +106,7 @@ CREATE TEMPORARY TABLE input (id serial, data json); "databases": databases, "options": options, "username": spec.Name, - "verifier": verifiers[string(spec.Name)], + "verifier": verifiers[spec.Name], }) } } @@ -194,9 +194,9 @@ func WriteUsersSchemasInPostgreSQL(ctx context.Context, exec Executor, spec := users[i] // We skip if the user has the name of a reserved schema - if RESERVED_SCHEMA_NAMES[string(spec.Name)] { + if RESERVED_SCHEMA_NAMES[spec.Name] { log.V(1).Info("Skipping schema creation for user with reserved name", - "name", string(spec.Name)) + "name", spec.Name) continue } @@ -239,7 +239,7 @@ func WriteUsersSchemasInPostgreSQL(ctx context.Context, exec Executor, }, "\n"), map[string]string{ "databases": string(databases), - "username": string(spec.Name), + "username": spec.Name, "ON_ERROR_STOP": "on", // Abort when any one statement fails. "QUIET": "on", // Do not print successful commands to stdout. diff --git a/internal/postgres/users_test.go b/internal/postgres/users_test.go index 57587a3b11..313a9f0134 100644 --- a/internal/postgres/users_test.go +++ b/internal/postgres/users_test.go @@ -131,7 +131,7 @@ COMMIT;`)) []v1beta1.PostgresUserSpec{ { Name: "user-no-options", - Databases: []v1beta1.PostgresIdentifier{"db1"}, + Databases: []string{"db1"}, }, { Name: "user-no-databases", @@ -175,7 +175,7 @@ COMMIT;`)) []v1beta1.PostgresUserSpec{ { Name: "postgres", - Databases: []v1beta1.PostgresIdentifier{"all", "ignored"}, + Databases: []string{"all", "ignored"}, Options: "NOLOGIN CONNECTION LIMIT 0", }, }, @@ -213,18 +213,18 @@ func TestWriteUsersSchemasInPostgreSQL(t *testing.T) { []v1beta1.PostgresUserSpec{ { Name: "user-single-db", - Databases: []v1beta1.PostgresIdentifier{"db1"}, + Databases: []string{"db1"}, }, { Name: "user-no-databases", }, { Name: "user-multi-dbs", - Databases: []v1beta1.PostgresIdentifier{"db1", "db2"}, + Databases: []string{"db1", "db2"}, }, { Name: "public", - Databases: []v1beta1.PostgresIdentifier{"db3"}, + Databases: []string{"db3"}, }, }, )) 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 cb69481664..0ed90d4a3e 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go @@ -4,12 +4,12 @@ package v1beta1 +// --- // PostgreSQL identifiers are limited in length but may contain any character. -// More info: https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS -// +// - https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 -type PostgresIdentifier string +type PostgresIdentifier = string type PostgresPasswordSpec struct { // Type of password to generate. Defaults to ASCII. Valid options are ASCII @@ -23,6 +23,7 @@ type PostgresPasswordSpec struct { // // +kubebuilder:default=ASCII // +kubebuilder:validation:Enum={ASCII,AlphaNumeric} + // +required Type string `json:"type"` } @@ -33,20 +34,24 @@ const ( ) type PostgresUserSpec struct { - - // This value goes into the name of a corev1.Secret and a label value, so - // it must match both IsDNS1123Subdomain and IsValidLabelValue. The pattern - // below is IsDNS1123Subdomain without any dots, U+002E. - // The name of this PostgreSQL user. The value may contain only lowercase // letters, numbers, and hyphen so that it fits into Kubernetes metadata. + // --- + // This value goes into the name of a corev1.Secret and a label value, so + // it must match both IsDNS1123Subdomain and IsValidLabelValue. + // - https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation#IsDNS1123Subdomain + // - https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation#IsValidLabelValue + // + // This is IsDNS1123Subdomain without any dots, U+002E: // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` - // +kubebuilder:validation:Type=string + // + // +required Name PostgresIdentifier `json:"name"` // Databases to which this user can connect and create objects. Removing a // database from this list does NOT revoke access. This field is ignored for // the "postgres" user. + // --- // +listType=set // +optional Databases []PostgresIdentifier `json:"databases,omitempty"` @@ -54,6 +59,7 @@ type PostgresUserSpec struct { // ALTER ROLE options except for PASSWORD. This field is ignored for the // "postgres" user. // More info: https://www.postgresql.org/docs/current/role-attributes.html + // --- // +kubebuilder:validation:MaxLength=200 // +kubebuilder:validation:Pattern=`^[^;]*$` // +kubebuilder:validation:XValidation:rule=`!self.matches("(?i:PASSWORD)")`,message="cannot assign password" @@ -62,6 +68,7 @@ type PostgresUserSpec struct { Options string `json:"options,omitempty"` // Properties of the password generated for this user. + // --- // +optional Password *PostgresPasswordSpec `json:"password,omitempty"` }