Skip to content

Commit

Permalink
Change PostgresIdentifier to a type alias
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cbandy committed Jan 3, 2025
1 parent 4d73723 commit 79ea670
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 31 deletions.
17 changes: 7 additions & 10 deletions internal/controller/postgrescluster/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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...)
}
}

Expand Down Expand Up @@ -379,18 +377,17 @@ 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},
}}
}
}

// 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{}
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/postgrescluster/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/pgadmin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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],
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/pgadmin/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions internal/postgres/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,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],
})
}
}
Expand Down Expand Up @@ -190,9 +190,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
}

Expand Down Expand Up @@ -231,7 +231,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.
Expand Down
10 changes: 5 additions & 5 deletions internal/postgres/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ COMMIT;`))
[]v1beta1.PostgresUserSpec{
{
Name: "user-no-options",
Databases: []v1beta1.PostgresIdentifier{"db1"},
Databases: []string{"db1"},
},
{
Name: "user-no-databases",
Expand Down Expand Up @@ -175,7 +175,7 @@ COMMIT;`))
[]v1beta1.PostgresUserSpec{
{
Name: "postgres",
Databases: []v1beta1.PostgresIdentifier{"all", "ignored"},
Databases: []string{"all", "ignored"},
Options: "NOLOGIN CONNECTION LIMIT 0",
},
},
Expand Down Expand Up @@ -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"},
},
},
))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ 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
//
// ---
// +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
Expand All @@ -23,6 +23,7 @@ type PostgresPasswordSpec struct {
//
// +kubebuilder:default=ASCII
// +kubebuilder:validation:Enum={ASCII,AlphaNumeric}
// +required
Type string `json:"type"`
}

Expand All @@ -33,27 +34,29 @@ const (
)

type PostgresUserSpec struct {

// 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. 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.
//
// +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"`

// 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"
Expand All @@ -62,6 +65,7 @@ type PostgresUserSpec struct {
Options string `json:"options,omitempty"`

// Properties of the password generated for this user.
// ---
// +optional
Password *PostgresPasswordSpec `json:"password,omitempty"`
}

0 comments on commit 79ea670

Please sign in to comment.