diff --git a/cmd/main.go b/cmd/main.go index 57e79c3c..951f0c0d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -132,8 +132,9 @@ func main() { Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("PostgreSQLDatabase"), - ManagerRoleName: config.ManagerRoleName, - HostCredentials: config.HostCredentials, + ManagerRoleName: config.ManagerRoleName, + SuperuserRoleName: config.SuperuserRoleName, + HostCredentials: config.HostCredentials, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PostgreSQLDatabase") os.Exit(1) @@ -190,9 +191,10 @@ func main() { os.Exit(1) } if err = (&controller.CustomRoleReconciler{ - Client: mgr.GetClient(), - Log: ctrl.Log.WithName("controllers").WithName("CustomRole"), - HostCredentials: config.HostCredentials, + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("CustomRole"), + SuperuserRoleName: config.SuperuserRoleName, + HostCredentials: config.HostCredentials, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "CustomRole") os.Exit(1) diff --git a/internal/config/config.go b/internal/config/config.go index ea448b98..14c1f474 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,7 @@ type ControllerConfiguration struct { AWS AwsConfig HostCredentials map[string]postgres.Credentials ManagerRoleName string + SuperuserRoleName string AllDatabasesReadEnabled bool AllDatabasesWriteEnabled bool ExtendedWriteEnabled bool @@ -51,6 +52,7 @@ func (c *ControllerConfiguration) RegisterFlags(flagSet *flag.FlagSet) { flagSet.Var(&HostCredentials{value: &c.HostCredentials}, "host-credentials", "Host and credential pairs in the form hostname=user:password. Use comma separated pairs for multiple hosts") flagSet.StringVar(&c.ManagerRoleName, "manager-role-name", "postgres_role_manager", "Name of the role which will be managing other roles") + flagSet.StringVar(&c.SuperuserRoleName, "superuser-role-name", "rds_superuser", "Name of the superuser role the connecting user must be a member of (defaults to RDS's rds_superuser; override for non-RDS deployments)") flagSet.StringVar(&c.UserRoles, "user-roles", "rds_iam", "List of roles granted to all users") flagSet.BoolVar(&c.AllDatabasesReadEnabled, "all-databases-enabled-read", false, "Enable usage of allDatabases field in read access requests") flagSet.BoolVar(&c.AllDatabasesWriteEnabled, "all-databases-enabled-write", false, "Enable usage of allDatabases field in write access requests") diff --git a/internal/controller/customrole_controller.go b/internal/controller/customrole_controller.go index c149d0f9..b34ca952 100644 --- a/internal/controller/customrole_controller.go +++ b/internal/controller/customrole_controller.go @@ -47,6 +47,8 @@ type CustomRoleReconciler struct { client.Client Log logr.Logger + SuperuserRoleName string + // HostCredentials contains a map of credentials for hosts (keyed by host name) HostCredentials map[string]postgres.Credentials } @@ -191,6 +193,10 @@ func (r *CustomRoleReconciler) reconcileOnHost(log logr.Logger, host string, cre } defer adminDB.Close() + if err := postgres.Preflight(log, adminDB, r.SuperuserRoleName); err != nil { + return err + } + // Resolve the effective database list and, when scoped, all user databases // (so each domain can run its cleanup pass without an extra query). databases, allUserDatabases, err := resolveTargetDatabases(log, adminDB, targetDatabases) diff --git a/internal/controller/postgresqldatabase_controller.go b/internal/controller/postgresqldatabase_controller.go index f6100a88..785bcf29 100644 --- a/internal/controller/postgresqldatabase_controller.go +++ b/internal/controller/postgresqldatabase_controller.go @@ -43,7 +43,8 @@ type PostgreSQLDatabaseReconciler struct { client.Client Log logr.Logger - ManagerRoleName string + ManagerRoleName string + SuperuserRoleName string // contains a map of credentials for hosts HostCredentials map[string]postgres.Credentials } @@ -110,6 +111,11 @@ func (r *PostgreSQLDatabaseReconciler) reconcile(ctx context.Context, reqLogger } status.host = host reqLogger = reqLogger.WithValues("host", host) + + if err := r.runPreflight(reqLogger, host, *adminCredentials); err != nil { + return status, err + } + user, err := kube.ResourceValue(r.Client, database.Spec.User, request.Namespace) if err != nil { if !ctlerrors.IsInvalid(err) { @@ -284,6 +290,25 @@ func (r *PostgreSQLDatabaseReconciler) EnsurePostgreSQLDatabase(ctx context.Cont return nil } +// runPreflight opens an admin connection to the host and verifies controller +// invariants (superuser role membership) before any reconcile work. The +// connection is closed before returning - the downstream postgres.Database +// call opens its own. +func (r *PostgreSQLDatabaseReconciler) runPreflight(log logr.Logger, host string, admin postgres.Credentials) error { + db, err := postgres.Connect(postgres.ConnectionString{ + Host: host, + Database: "postgres", + User: admin.User, + Password: admin.Password, + Params: admin.Params, + }) + if err != nil { + return fmt.Errorf("preflight: connect to host %s: %w", host, err) + } + defer db.Close() + return postgres.Preflight(log, db, r.SuperuserRoleName) +} + type adminCredentialsParams struct { // namespace is the namespace containing the target PostgreSQLDatabase // resource. diff --git a/internal/controller/postgresqldatabase_controller_test.go b/internal/controller/postgresqldatabase_controller_test.go index 909fcd70..740b8094 100644 --- a/internal/controller/postgresqldatabase_controller_test.go +++ b/internal/controller/postgresqldatabase_controller_test.go @@ -237,10 +237,11 @@ func TestPostgreSQLDatabase_Reconcile_hostCredentialsResourceReference(t *testin // Create a controller object with the fake client but otherwise "live" setup // with database interaction r := &PostgreSQLDatabaseReconciler{ - Client: cl, - Log: ctrl.Log.WithName(t.Name()), - ManagerRoleName: managerRole, - HostCredentials: nil, + Client: cl, + Log: ctrl.Log.WithName(t.Name()), + ManagerRoleName: managerRole, + SuperuserRoleName: "iam_creator", + HostCredentials: nil, } // seed database into the postgres host @@ -326,10 +327,11 @@ func TestPostgreSQLDatabase_Reconcile_noPassword(t *testing.T) { // Create a controller object with the fake client but otherwise "live" setup // with database interaction r := &PostgreSQLDatabaseReconciler{ - Client: cl, - Log: ctrl.Log.WithName(t.Name()), - ManagerRoleName: managerRole, - HostCredentials: nil, + Client: cl, + Log: ctrl.Log.WithName(t.Name()), + ManagerRoleName: managerRole, + SuperuserRoleName: "iam_creator", + HostCredentials: nil, } // seed database into the postgres host diff --git a/pkg/postgres/preflight.go b/pkg/postgres/preflight.go new file mode 100644 index 00000000..fe5991f9 --- /dev/null +++ b/pkg/postgres/preflight.go @@ -0,0 +1,57 @@ +package postgres + +import ( + "database/sql" + "fmt" + + "github.com/go-logr/logr" +) + +// Preflight verifies the invariants the controller relies on for every +// reconciliation cycle. It returns a descriptive error naming the violated +// assumption so operators can fix the underlying setup. +// +// Performed in order: +// - The database connection is alive. +// - The connecting user is a member of superuserRole. The role is +// expected to exist on the server. +func Preflight(log logr.Logger, db *sql.DB, superuserRole string) error { + if superuserRole == "" { + return fmt.Errorf("preflight: superuser role name is empty (configure --superuser-role-name)") + } + + if err := db.Ping(); err != nil { + return fmt.Errorf("preflight: ping database: %w", err) + } + + if err := assertSuperuserRoleMembership(db, superuserRole); err != nil { + return err + } + + log.V(1).Info("Preflight checks passed", "superuserRole", superuserRole) + return nil +} + +func assertSuperuserRoleMembership(db *sql.DB, superuserRole string) error { + var ( + currentUser string + isMember bool + ) + err := db.QueryRow(` + SELECT + current_user, + EXISTS( + SELECT 1 + FROM pg_roles r + WHERE r.rolname = $1 + AND pg_has_role(current_user, r.oid, 'MEMBER') + ) + `, superuserRole).Scan(¤tUser, &isMember) + if err != nil { + return fmt.Errorf("preflight: query connecting user privileges: %w", err) + } + if isMember { + return nil + } + return fmt.Errorf("preflight: connecting user %q is not a member of %s - grant %s to it before running the controller", currentUser, superuserRole, superuserRole) +} diff --git a/pkg/postgres/preflight_test.go b/pkg/postgres/preflight_test.go new file mode 100644 index 00000000..4c0fa84f --- /dev/null +++ b/pkg/postgres/preflight_test.go @@ -0,0 +1,85 @@ +package postgres_test + +import ( + "database/sql" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lunarway.com/postgresql-controller/pkg/postgres" + "go.lunarway.com/postgresql-controller/test" +) + +// The integration test fixture exposes "iam_creator" as a pre-existing role +// the connection user is implicitly a member of, so we use it as the +// configured superuser role to avoid provisioning RDS-specific roles. +const preflightSuperuserRole = "iam_creator" + +func TestPreflight_sunshine(t *testing.T) { + host := test.Integration(t) + log := test.SetLogger(t) + + db := preflightAdminConn(t, host) + defer db.Close() + + err := postgres.Preflight(log, db, preflightSuperuserRole) + assert.NoError(t, err, "preflight should pass when all assumptions are met") +} + +func TestPreflight_emptySuperuserRole(t *testing.T) { + host := test.Integration(t) + log := test.SetLogger(t) + + db := preflightAdminConn(t, host) + defer db.Close() + + err := postgres.Preflight(log, db, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "superuser role name is empty") +} + +func TestPreflight_userLacksSuperuserRoleMembership(t *testing.T) { + host := test.Integration(t) + log := test.SetLogger(t) + + adminDB := preflightAdminConn(t, host) + defer adminDB.Close() + + // A login role that has not been granted the configured superuser role + // must be rejected. + plebUser := fmt.Sprintf("preflight_pleb_%d", time.Now().UnixNano()) + plebPassword := "pleb" + _, err := adminDB.Exec(fmt.Sprintf("CREATE ROLE %s WITH LOGIN PASSWORD '%s' NOSUPERUSER NOCREATEROLE NOCREATEDB", plebUser, plebPassword)) + require.NoError(t, err, "create unprivileged role") + defer func() { + _, _ = adminDB.Exec(fmt.Sprintf("DROP ROLE %s", plebUser)) + }() + + plebDB, err := postgres.Connect(postgres.ConnectionString{ + Host: host, + Database: "postgres", + User: plebUser, + Password: plebPassword, + }) + require.NoError(t, err, "connect as unprivileged user") + defer plebDB.Close() + + err = postgres.Preflight(log, plebDB, preflightSuperuserRole) + require.Error(t, err) + assert.Contains(t, err.Error(), preflightSuperuserRole) + assert.Contains(t, err.Error(), plebUser) +} + +func preflightAdminConn(t *testing.T, host string) *sql.DB { + t.Helper() + db, err := postgres.Connect(postgres.ConnectionString{ + Host: host, + Database: "postgres", + User: "iam_creator", + Password: "iam_creator", + }) + require.NoError(t, err, "connect to database as admin") + return db +}