Create schemas for users in granted databases (#3940)

* Create schemas for users in granted databases

To help developers set up and connect quickly, the operator
can now create schemas for `spec.users` without using an
init SQL script.

This is a gated feature: to turn on set the FeatureGate
`AutoCreateUserSchema=true`.

If turned on, a cluster can be annotated with
`postgres-operator.crunchydata.com/autoCreateUserSchema=true`.

If the feature is turned on and the cluster is annotated, PGO
will create a schema named after the user in every database
where that user has permissions.

(PG note: creating a schema with the same name as the user
means that the PG `search_path` should not need to be updated,
since `search_path` defaults to `"$user", public`.)

As with our usual pattern, the operator does not remove/delete
PG objects (users, databases) that are removed from the spec.

NOTE: There are several schema names that would be dangerous to the
cluster's operation; for instance, if you had pgbouncer enabled
(which would create a `pgbouncer` schema) it would be dangerous to
create a user named `pgbouncer` and use this feature to create
a schema for that user. We have a blacklist for such reserved names,
which result in the skipping being logged for now.

Issues: [PGO-1333]

Author: benjaminjb

internal/controller/postgrescluster/postgres.go

@@ -534,7 +534,7 @@ func (r *Reconciler) reconcilePostgresUsersInPostgreSQL(
 	}
 
 	write := func(ctx context.Context, exec postgres.Executor) error {
-		return postgres.WriteUsersInPostgreSQL(ctx, exec, specUsers, verifiers)
+		return postgres.WriteUsersInPostgreSQL(ctx, cluster, exec, specUsers, verifiers)
 	}
 
 	revision, err := safeHash32(func(hasher io.Writer) error {

internal/naming/annotations.go

@@ -70,4 +70,9 @@ const (
 	// bridge cluster, the user must add this annotation to the CR to allow the CR to take control of
 	// the Bridge Cluster. The Value assigned to the annotation must be the ID of existing cluster.
 	CrunchyBridgeClusterAdoptionAnnotation = annotationPrefix + "adopt-bridge-cluster"
+
+	// AutoCreateUserSchemaAnnotation is an annotation used to allow users to control whether the cluster
+	// has schemas automatically created for the users defined in `spec.users` for all of the databases
+	// listed for that user.
+	AutoCreateUserSchemaAnnotation = annotationPrefix + "autoCreateUserSchema"
 )

internal/postgres/users.go

@@ -24,9 +24,17 @@ import (
 	pg_query "github.com/pganalyze/pg_query_go/v5"
 
 	"github.com/crunchydata/postgres-operator/internal/logging"
+	"github.com/crunchydata/postgres-operator/internal/naming"
+	"github.com/crunchydata/postgres-operator/internal/util"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
+var RESERVED_SCHEMA_NAMES = map[string]bool{
+	"public":    true, // This is here for documentation; Postgres will reject a role named `public` as reserved
+	"pgbouncer": true,
+	"monitor":   true,
+}
+
 func sanitizeAlterRoleOptions(options string) string {
 	const AlterRolePrefix = `ALTER ROLE "any" WITH `
 
@@ -61,7 +69,7 @@ func sanitizeAlterRoleOptions(options string) string {
 // grants them access to their specified databases. The databases must already
 // exist.
 func WriteUsersInPostgreSQL(
-	ctx context.Context, exec Executor,
+	ctx context.Context, cluster *v1beta1.PostgresCluster, exec Executor,
 	users []v1beta1.PostgresUserSpec, verifiers map[string]string,
 ) error {
 	log := logging.FromContext(ctx)
@@ -162,5 +170,83 @@ SELECT pg_catalog.format('GRANT ALL PRIVILEGES ON DATABASE %I TO %I',
 
 	log.V(1).Info("wrote PostgreSQL users", "stdout", stdout, "stderr", stderr)
 
+	// The operator will attemtp to write schemas for the users in the spec if
+	// 	* the feature gate is enabled and
+	// 	* the cluster is annotated.
+	if util.DefaultMutableFeatureGate.Enabled(util.AutoCreateUserSchema) {
+		autoCreateUserSchemaAnnotationValue, annotationExists := cluster.Annotations[naming.AutoCreateUserSchemaAnnotation]
+		if annotationExists && strings.EqualFold(autoCreateUserSchemaAnnotationValue, "true") {
+			log.V(1).Info("Writing schemas for users.")
+			err = WriteUsersSchemasInPostgreSQL(ctx, exec, users)
+		}
+	}
+
+	return err
+}
+
+// WriteUsersSchemasInPostgreSQL will create a schema for each user in each database that user has access to
+func WriteUsersSchemasInPostgreSQL(ctx context.Context, exec Executor,
+	users []v1beta1.PostgresUserSpec) error {
+
+	log := logging.FromContext(ctx)
+
+	var err error
+	var stdout string
+	var stderr string
+
+	for i := range users {
+		spec := users[i]
+
+		// We skip if the user has the name of a reserved schema
+		if RESERVED_SCHEMA_NAMES[string(spec.Name)] {
+			log.V(1).Info("Skipping schema creation for user with reserved name",
+				"name", string(spec.Name))
+			continue
+		}
+
+		// We skip if the user has no databases
+		if len(spec.Databases) == 0 {
+			continue
+		}
+
+		var sql bytes.Buffer
+
+		// Prevent unexpected dereferences by emptying "search_path". The "pg_catalog"
+		// schema is still searched, and only temporary objects can be created.
+		// - https://www.postgresql.org/docs/current/runtime-config-client.html#GUC-SEARCH-PATH
+		_, _ = sql.WriteString(`SET search_path TO '';`)
+
+		_, _ = sql.WriteString(`SELECT * FROM json_array_elements_text(:'databases');`)
+
+		databases, _ := json.Marshal(spec.Databases)
+
+		stdout, stderr, err = exec.ExecInDatabasesFromQuery(ctx,
+			sql.String(),
+			strings.Join([]string{
+				// Quiet NOTICE messages from IF EXISTS statements.
+				// - https://www.postgresql.org/docs/current/runtime-config-client.html
+				`SET client_min_messages = WARNING;`,
+
+				// Creates a schema named after and owned by the user
+				// - https://www.postgresql.org/docs/current/ddl-schemas.html
+				// - https://www.postgresql.org/docs/current/sql-createschema.html
+
+				// We create a schema named after the user because
+				// the PG search_path does not need to be updated,
+				// since search_path defaults to "$user", public.
+				// - https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH
+				`CREATE SCHEMA IF NOT EXISTS :"username" AUTHORIZATION :"username";`,
+			}, "\n"),
+			map[string]string{
+				"databases": string(databases),
+				"username":  string(spec.Name),
+
+				"ON_ERROR_STOP": "on", // Abort when any one statement fails.
+				"QUIET":         "on", // Do not print successful commands to stdout.
+			},
+		)
+
+		log.V(1).Info("wrote PostgreSQL schemas", "stdout", stdout, "stderr", stderr)
+	}
 	return err
 }

internal/postgres/users_test.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -59,7 +60,8 @@ func TestWriteUsersInPostgreSQL(t *testing.T) {
 			return expected
 		}
 
-		assert.Equal(t, expected, WriteUsersInPostgreSQL(ctx, exec, nil, nil))
+		cluster := new(v1beta1.PostgresCluster)
+		assert.Equal(t, expected, WriteUsersInPostgreSQL(ctx, cluster, exec, nil, nil))
 	})
 
 	t.Run("Empty", func(t *testing.T) {
@@ -104,17 +106,19 @@ COMMIT;`))
 			return nil
 		}
 
-		assert.NilError(t, WriteUsersInPostgreSQL(ctx, exec, nil, nil))
+		cluster := new(v1beta1.PostgresCluster)
+		assert.NilError(t, WriteUsersInPostgreSQL(ctx, cluster, exec, nil, nil))
 		assert.Equal(t, calls, 1)
 
-		assert.NilError(t, WriteUsersInPostgreSQL(ctx, exec, []v1beta1.PostgresUserSpec{}, nil))
+		assert.NilError(t, WriteUsersInPostgreSQL(ctx, cluster, exec, []v1beta1.PostgresUserSpec{}, nil))
 		assert.Equal(t, calls, 2)
 
-		assert.NilError(t, WriteUsersInPostgreSQL(ctx, exec, nil, map[string]string{}))
+		assert.NilError(t, WriteUsersInPostgreSQL(ctx, cluster, exec, nil, map[string]string{}))
 		assert.Equal(t, calls, 3)
 	})
 
 	t.Run("OptionalFields", func(t *testing.T) {
+		cluster := new(v1beta1.PostgresCluster)
 		calls := 0
 		exec := func(
 			_ context.Context, stdin io.Reader, _, _ io.Writer, command ...string,
@@ -134,7 +138,7 @@ COMMIT;`))
 			return nil
 		}
 
-		assert.NilError(t, WriteUsersInPostgreSQL(ctx, exec,
+		assert.NilError(t, WriteUsersInPostgreSQL(ctx, cluster, exec,
 			[]v1beta1.PostgresUserSpec{
 				{
 					Name:      "user-no-options",
@@ -162,6 +166,7 @@ COMMIT;`))
 
 	t.Run("PostgresSuperuser", func(t *testing.T) {
 		calls := 0
+		cluster := new(v1beta1.PostgresCluster)
 		exec := func(
 			_ context.Context, stdin io.Reader, _, _ io.Writer, command ...string,
 		) error {
@@ -177,7 +182,7 @@ COMMIT;`))
 			return nil
 		}
 
-		assert.NilError(t, WriteUsersInPostgreSQL(ctx, exec,
+		assert.NilError(t, WriteUsersInPostgreSQL(ctx, cluster, exec,
 			[]v1beta1.PostgresUserSpec{
 				{
 					Name:      "postgres",
@@ -192,3 +197,52 @@ COMMIT;`))
 		assert.Equal(t, calls, 1)
 	})
 }
+
+func TestWriteUsersSchemasInPostgreSQL(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("Mixed users", func(t *testing.T) {
+		calls := 0
+		exec := func(
+			_ context.Context, stdin io.Reader, _, _ io.Writer, command ...string,
+		) error {
+			calls++
+
+			b, err := io.ReadAll(stdin)
+			assert.NilError(t, err)
+
+			// The command strings will contain either of two possibilities, depending on the user called.
+			commands := strings.Join(command, ",")
+			re := regexp.MustCompile("--set=databases=\\[\"db1\"\\],--set=username=user-single-db|--set=databases=\\[\"db1\",\"db2\"\\],--set=username=user-multi-db")
+			assert.Assert(t, cmp.Regexp(re, commands))
+
+			assert.Assert(t, cmp.Contains(string(b), `CREATE SCHEMA IF NOT EXISTS :"username" AUTHORIZATION :"username";`))
+			return nil
+		}
+
+		assert.NilError(t, WriteUsersSchemasInPostgreSQL(ctx, exec,
+			[]v1beta1.PostgresUserSpec{
+				{
+					Name:      "user-single-db",
+					Databases: []v1beta1.PostgresIdentifier{"db1"},
+				},
+				{
+					Name: "user-no-databases",
+				},
+				{
+					Name:      "user-multi-dbs",
+					Databases: []v1beta1.PostgresIdentifier{"db1", "db2"},
+				},
+				{
+					Name:      "public",
+					Databases: []v1beta1.PostgresIdentifier{"db3"},
+				},
+			},
+		))
+		// The spec.users has four elements, but two will be skipped:
+		// 	* the user with the reserved name `public`
+		// 	* the user with 0 databases
+		assert.Equal(t, calls, 2)
+	})
+
+}

internal/util/features.go

@@ -35,6 +35,9 @@ const (
 	// Enables support of appending custom queries to default PGMonitor queries
 	AppendCustomQueries featuregate.Feature = "AppendCustomQueries"
 	//
+	// Enables automatic creation of user schema
+	AutoCreateUserSchema featuregate.Feature = "AutoCreateUserSchema"
+	//
 	// Enables support of auto-grow volumes
 	AutoGrowVolumes featuregate.Feature = "AutoGrowVolumes"
 	//
@@ -58,12 +61,13 @@ const (
 //
 // - https://releases.k8s.io/v1.20.0/pkg/features/kube_features.go#L729-732
 var pgoFeatures = map[featuregate.Feature]featuregate.FeatureSpec{
-	AppendCustomQueries: {Default: false, PreRelease: featuregate.Alpha},
-	AutoGrowVolumes:     {Default: false, PreRelease: featuregate.Alpha},
-	BridgeIdentifiers:   {Default: false, PreRelease: featuregate.Alpha},
-	InstanceSidecars:    {Default: false, PreRelease: featuregate.Alpha},
-	PGBouncerSidecars:   {Default: false, PreRelease: featuregate.Alpha},
-	TablespaceVolumes:   {Default: false, PreRelease: featuregate.Alpha},
+	AppendCustomQueries:  {Default: false, PreRelease: featuregate.Alpha},
+	AutoCreateUserSchema: {Default: false, PreRelease: featuregate.Alpha},
+	AutoGrowVolumes:      {Default: false, PreRelease: featuregate.Alpha},
+	BridgeIdentifiers:    {Default: false, PreRelease: featuregate.Alpha},
+	InstanceSidecars:     {Default: false, PreRelease: featuregate.Alpha},
+	PGBouncerSidecars:    {Default: false, PreRelease: featuregate.Alpha},
+	TablespaceVolumes:    {Default: false, PreRelease: featuregate.Alpha},
 }
 
 // DefaultMutableFeatureGate is a mutable, shared global FeatureGate.