From 1706883b11388131e725e39fefd2cbe603fdda80 Mon Sep 17 00:00:00 2001 From: souravbiswassanto Date: Sun, 20 Sep 2026 13:47:34 +0600 Subject: [PATCH] Allow remote-config to be generated from a standby `kubectl dba remote-config` creates the replication user on the source before rendering the AppBinding and secrets, and that step needs a writable primary: CREATE ROLE / ALTER ROLE / GRANT are catalog writes a hot standby rejects. There are sources with no primary to exec into. A remote replica acting as the source of a chained replica is the clearest case: every member is a standby, and the replication role is already there anyway, carried over by physical WAL replication. Creating it is impossible and also unnecessary. Nothing in the generated config is primary-specific - the host comes from -d, the port from --port, the secret is synthesised and the client certificate's CN is the username - so only the side effect is coupled to the primary, not the artifact. Add --skip-user-creation, which leaves the source catalog alone and takes the given password at face value. It also drops the `status.phase == Ready` gate, since rendering manifests only reads the CR and a degraded source is exactly when a DR config is wanted. The role is still verified where possible: reading pg_roles (mysql.user for MySQL) works on a standby, so a missing role or one lacking the REPLICATION attribute fails with the SQL to run by hand, while an unreachable database only warns. Also fixes pod lookup, which returned nil - reported as success - when no pod matched, so the command wrote an auth secret for a user it never created and exited 0. It now fails with an actionable message, and skips pods that do not run the database container, such as the Postgres arbiter. Signed-off-by: souravbiswassanto --- pkg/common/mysql.go | 6 +- pkg/common/options.go | 43 +++++++++++++++ pkg/common/postgres.go | 6 +- pkg/remote_replica/mysql.go | 94 +++++++++++++++++++++++-------- pkg/remote_replica/pods.go | 98 +++++++++++++++++++++++++++++++++ pkg/remote_replica/pods_test.go | 87 +++++++++++++++++++++++++++++ pkg/remote_replica/postgres.go | 95 +++++++++++++++++++++++++------- 7 files changed, 382 insertions(+), 47 deletions(-) create mode 100644 pkg/common/options.go create mode 100644 pkg/remote_replica/pods.go create mode 100644 pkg/remote_replica/pods_test.go diff --git a/pkg/common/mysql.go b/pkg/common/mysql.go index 8f0c3a954..2d83f7faf 100644 --- a/pkg/common/mysql.go +++ b/pkg/common/mysql.go @@ -47,7 +47,9 @@ type MySQLOpts struct { ErrWriter *bytes.Buffer } -func NewMySQLOpts(f cmdutil.Factory, dbName, namespace string) (*MySQLOpts, error) { +func NewMySQLOpts(f cmdutil.Factory, dbName, namespace string, options ...OptionFunc) (*MySQLOpts, error) { + cfg := buildOptionConfig(options) + config, err := f.ToRESTConfig() if err != nil { return nil, err @@ -75,7 +77,7 @@ func NewMySQLOpts(f cmdutil.Factory, dbName, namespace string) (*MySQLOpts, erro return nil, err } - if db.Status.Phase != dbapi.DatabasePhaseReady { + if !cfg.skipReadinessCheck && db.Status.Phase != dbapi.DatabasePhaseReady { return nil, fmt.Errorf("MySQL %s/%s is not ready", namespace, dbName) } diff --git a/pkg/common/options.go b/pkg/common/options.go new file mode 100644 index 000000000..a37aedf0f --- /dev/null +++ b/pkg/common/options.go @@ -0,0 +1,43 @@ +/* +Copyright AppsCode Inc. and Contributors + +Licensed under the AppsCode Community License 1.0.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://github.com/appscode/licenses/raw/1.0.0/AppsCode-Community-1.0.0.md + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package common + +// OptionFunc tunes how a per-engine Opts value is built. +type OptionFunc func(*optionConfig) + +type optionConfig struct { + skipReadinessCheck bool +} + +// SkipReadinessCheck drops the `status.phase == Ready` gate on the database CR. +// Commands that only read the CR to render manifests do not need a healthy +// database, and refusing to run against a degraded source is actively unhelpful +// for disaster-recovery workflows, where the source being unhealthy is the +// whole reason the command is being run. +func SkipReadinessCheck() OptionFunc { + return func(cfg *optionConfig) { + cfg.skipReadinessCheck = true + } +} + +func buildOptionConfig(opts []OptionFunc) optionConfig { + var cfg optionConfig + for _, opt := range opts { + opt(&cfg) + } + return cfg +} diff --git a/pkg/common/postgres.go b/pkg/common/postgres.go index 3b8c0d8f8..1a30485d0 100644 --- a/pkg/common/postgres.go +++ b/pkg/common/postgres.go @@ -49,7 +49,9 @@ type PostgresOpts struct { ErrWriter *bytes.Buffer } -func NewPostgresOpts(f cmdutil.Factory, dbName, namespace string) (*PostgresOpts, error) { +func NewPostgresOpts(f cmdutil.Factory, dbName, namespace string, options ...OptionFunc) (*PostgresOpts, error) { + cfg := buildOptionConfig(options) + config, err := f.ToRESTConfig() if err != nil { return nil, err @@ -77,7 +79,7 @@ func NewPostgresOpts(f cmdutil.Factory, dbName, namespace string) (*PostgresOpts return nil, err } - if db.Status.Phase != dbapi.DatabasePhaseReady { + if !cfg.skipReadinessCheck && db.Status.Phase != dbapi.DatabasePhaseReady { return nil, fmt.Errorf("postgres %s/%s is not ready", namespace, dbName) } diff --git a/pkg/remote_replica/mysql.go b/pkg/remote_replica/mysql.go index 3e9a25fa3..94edeb37c 100644 --- a/pkg/remote_replica/mysql.go +++ b/pkg/remote_replica/mysql.go @@ -21,6 +21,7 @@ import ( "fmt" "log" "os" + "strings" "time" dbapi "kubedb.dev/apimachinery/apis/kubedb/v1" @@ -31,9 +32,9 @@ import ( core "k8s.io/api/core/v1" kerr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/klog/v2" cmdutil "k8s.io/kubectl/pkg/cmd/util" cm_util "kmodules.xyz/cert-manager-util/certmanager/v1" kutil "kmodules.xyz/client-go" @@ -59,7 +60,7 @@ const ( func MysqlAPP(f cmdutil.Factory) *cobra.Command { var userName, password, dns, ns string - var yes bool + var yes, skipUserCreation bool cmd := cobra.Command{ Use: "mysql", Short: desLong, @@ -71,12 +72,16 @@ func MysqlAPP(f cmdutil.Factory) *cobra.Command { if len(args) == 0 { log.Fatal("no database name given") } - if err := userPrompt(yes); err != nil { - log.Fatal(err) + // Nothing on the source is altered when user creation is skipped, so + // the "password will be altered" confirmation has nothing to confirm. + if !skipUserCreation { + if err := userPrompt(yes); err != nil { + log.Fatal(err) + } } var buffer []byte - buffer, err := generateMySQLConfig(f, userName, password, dns, ns, args[0]) + buffer, err := generateMySQLConfig(f, userName, password, dns, ns, args[0], skipUserCreation) if err != nil { log.Fatal(err) } @@ -111,13 +116,22 @@ func MysqlAPP(f cmdutil.Factory) *cobra.Command { log.Fatal(err) } cmd.PersistentFlags().BoolVarP(&yes, "yes", "y", false, "permission for alter password for the remote replica") + cmd.PersistentFlags().BoolVar(&skipUserCreation, "skip-user-creation", false, + "do not create or alter the replication user on the source; assume it already exists with the given password. "+ + "Needed when the source has no writable primary to exec into") return &cmd } -func generateMySQLConfig(f cmdutil.Factory, userName string, password string, dns string, ns string, dbname string) ([]byte, error) { +func generateMySQLConfig(f cmdutil.Factory, userName string, password string, dns string, ns string, dbname string, skipUserCreation bool) ([]byte, error) { var buffer []byte - opts, err := common.NewMySQLOpts(f, dbname, ns) + var dbOpts []common.OptionFunc + if skipUserCreation { + // Rendering manifests only reads the CR; a non-Ready source should not + // block a disaster recovery config from being generated. + dbOpts = append(dbOpts, common.SkipReadinessCheck()) + } + opts, err := common.NewMySQLOpts(f, dbname, ns, dbOpts...) if err != nil { return nil, fmt.Errorf("failed to get db %s, err:%v", dbname, err) } @@ -127,7 +141,7 @@ func generateMySQLConfig(f cmdutil.Factory, userName string, password string, dn return nil, fmt.Errorf("failed to get appbinding %v", err) } - authBuff, authSecretName, err := generateMySQLAuthSecret(userName, password, ns, opts) + authBuff, authSecretName, err := generateMySQLAuthSecret(userName, password, ns, skipUserCreation, opts) if err != nil { return nil, fmt.Errorf("failed to generate auth secret ,%v", err) } @@ -198,15 +212,20 @@ func generateMySQLTlsSecret(userName string, apb *appApi.AppBinding, ns string, return buffer, tlsSecret.Name, nil } -func generateMySQLAuthSecret(userName string, password string, ns string, opts *common.MySQLOpts) ([]byte, string, error) { - if userName != opts.Username { +func generateMySQLAuthSecret(userName string, password string, ns string, skipUserCreation bool, opts *common.MySQLOpts) ([]byte, string, error) { + switch { + case userName == opts.Username: + password = opts.Pass + case skipUserCreation: + if err := verifyMySQLUser(opts, userName); err != nil { + return nil, "", err + } + default: // generate user if not present err := generateMySQLUser(opts, userName, password) if err != nil { return nil, "", fmt.Errorf("failed to generate user err:%v", err) } - } else { - password = opts.Pass } // generate auth secret AuthSecret := core.Secret{ @@ -235,17 +254,48 @@ func generateMySQLAuthSecret(userName string, password string, ns string, opts * return buffer, AuthSecret.Name, nil } -func generateMySQLUser(opts *common.MySQLOpts, name string, password string) error { - label := opts.DB.OffshootLabels() - if *opts.DB.Spec.Replicas > 1 { - label["kubedb.com/role"] = "primary" +// verifyMySQLUser checks the replication user without writing anything. Reading +// mysql.user works on a read replica, so any running member will do. An +// unreachable database downgrades to a warning: generating the config is still +// the useful outcome. +func verifyMySQLUser(opts *common.MySQLOpts, name string) error { + pod, err := pickDBPod(opts.Client, opts.DB.Namespace, opts.DB.OffshootLabels(), MySQLContainerName, false) + if err != nil { + klog.Warningf("skipping verification of user %q: %v", name, err) + return nil } - pods, err := opts.Client.CoreV1().Pods(opts.DB.Namespace).List(context.TODO(), metav1.ListOptions{ - LabelSelector: labels.Set.String(label), - }) - if err != nil || len(pods.Items) == 0 { - return err + query := fmt.Sprintf("export MYSQL_PWD='%s' && mysql -uroot -N -B -e \"SELECT Repl_slave_priv FROM mysql.user WHERE user='%s'\"", opts.Pass, name) + out, err := exec_util.ExecIntoPod(opts.Config, pod, + exec_util.Command("bash", "-c", query), + exec_util.Container(MySQLContainerName), + ) + if err != nil { + klog.Warningf("skipping verification of user %q: failed to query %s: %v", name, pod.Name, err) + return nil + } + + switch strings.TrimSpace(out) { + case "": + return fmt.Errorf("user %q does not exist on %s/%s; create it on the writable primary first:\n "+ + "CREATE USER %s IDENTIFIED BY ''; GRANT REPLICATION SLAVE, CLONE_ADMIN, BACKUP_ADMIN ON *.* TO '%s'@'%%';", + name, opts.DB.Namespace, opts.DB.Name, name, name) + case "N": + return fmt.Errorf("user %q exists on %s/%s but lacks REPLICATION SLAVE; run on the writable primary:\n "+ + "GRANT REPLICATION SLAVE, CLONE_ADMIN, BACKUP_ADMIN ON *.* TO '%s'@'%%';", + name, opts.DB.Namespace, opts.DB.Name, name) + } + + fmt.Printf("user %q verified on %s (replication granted); leaving the source catalog untouched\n", name, pod.Name) + return nil +} + +func generateMySQLUser(opts *common.MySQLOpts, name string, password string) error { + // DDL only: a clustered source must be addressed through its primary. + pod, err := pickDBPod(opts.Client, opts.DB.Namespace, opts.DB.OffshootLabels(), MySQLContainerName, *opts.DB.Spec.Replicas > 1) + if err != nil { + return fmt.Errorf("%v; pass --skip-user-creation to generate the config against a standby "+ + "whose replication user already exists", err) } query := fmt.Sprintf("export MYSQL_PWD='%s' && mysql -uroot -e \"create user if not exists %s; alter user %s identified by '%s';"+ "GRANT REPLICATION SLAVE, CLONE_ADMIN, BACKUP_ADMIN ON *.* TO '%s'@'%%' WITH GRANT OPTION; \"", opts.Pass, @@ -257,7 +307,7 @@ func generateMySQLUser(opts *common.MySQLOpts, name string, password string) err container, } - _, err = exec_util.ExecIntoPod(opts.Config, &pods.Items[0], options...) + _, err = exec_util.ExecIntoPod(opts.Config, pod, options...) if err != nil { return err } diff --git a/pkg/remote_replica/pods.go b/pkg/remote_replica/pods.go new file mode 100644 index 000000000..eefac17bc --- /dev/null +++ b/pkg/remote_replica/pods.go @@ -0,0 +1,98 @@ +/* +Copyright AppsCode Inc. and Contributors + +Licensed under the AppsCode Community License 1.0.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://github.com/appscode/licenses/raw/1.0.0/AppsCode-Community-1.0.0.md + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote_replica + +import ( + "context" + "fmt" + + "kubedb.dev/apimachinery/apis/kubedb" + + core "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/kubernetes" +) + +// Containers holding the database server itself, the ones a client query has to +// be exec-ed into. +const ( + PostgresContainerName = "postgres" + MySQLContainerName = "mysql" +) + +// pickDBPod returns a running database pod to exec into. +// +// primaryOnly is set when the caller intends to run DDL. CREATE ROLE / ALTER +// ROLE / GRANT write to the shared catalog, and a hot standby rejects them with +// "cannot execute ... in a read-only transaction", so only a pod labelled +// kubedb.com/role=primary qualifies. Read-only catalog queries run fine on any +// member, so verification passes primaryOnly=false and merely prefers the +// primary when one happens to be labelled. +func pickDBPod(client kubernetes.Interface, ns string, selector map[string]string, container string, primaryOnly bool) (*core.Pod, error) { + pods, err := client.CoreV1().Pods(ns).List(context.TODO(), metav1.ListOptions{ + LabelSelector: labels.Set(selector).String(), + }) + if err != nil { + return nil, err + } + + pod, err := selectPod(pods.Items, container, primaryOnly) + if err != nil { + return nil, fmt.Errorf("in namespace %s: %v", ns, err) + } + return pod, nil +} + +// selectPod holds the choice itself, kept free of any client so it can be +// exercised directly. Only pods that actually carry `container` are eligible: a +// KubeDB cluster runs helper pods under the same offshoot labels - the Postgres +// arbiter, for one, runs pg-coordinator alone - and exec-ing a database query +// into those fails in a way that is hard to read. +func selectPod(pods []core.Pod, container string, primaryOnly bool) (*core.Pod, error) { + var usable []core.Pod + for i := range pods { + if pods[i].Status.Phase == core.PodRunning && hasRunningContainer(&pods[i], container) { + usable = append(usable, pods[i]) + } + } + if len(usable) == 0 { + // Reported explicitly: an empty list used to be swallowed and treated as + // success, which produced a config for a user that was never created. + return nil, fmt.Errorf("none of the %d database pod(s) has a running %q container", len(pods), container) + } + + for i := range usable { + if usable[i].Labels[kubedb.LabelRole] == kubedb.DatabasePodPrimary { + return &usable[i], nil + } + } + if primaryOnly { + return nil, fmt.Errorf("no pod is labelled %s=%s, so there is no writable primary to run DDL against", + kubedb.LabelRole, kubedb.DatabasePodPrimary) + } + return &usable[0], nil +} + +func hasRunningContainer(pod *core.Pod, container string) bool { + for _, cs := range pod.Status.ContainerStatuses { + if cs.Name == container { + return cs.State.Running != nil + } + } + return false +} diff --git a/pkg/remote_replica/pods_test.go b/pkg/remote_replica/pods_test.go new file mode 100644 index 000000000..268208255 --- /dev/null +++ b/pkg/remote_replica/pods_test.go @@ -0,0 +1,87 @@ +/* +Copyright AppsCode Inc. and Contributors + +Licensed under the AppsCode Community License 1.0.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://github.com/appscode/licenses/raw/1.0.0/AppsCode-Community-1.0.0.md + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package remote_replica + +import ( + "testing" + + "kubedb.dev/apimachinery/apis/kubedb" + + core "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func testPod(name, role string, phase core.PodPhase, containers ...string) core.Pod { + p := core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: map[string]string{kubedb.LabelRole: role}}, + Status: core.PodStatus{Phase: phase}, + } + for _, c := range containers { + p.Status.ContainerStatuses = append(p.Status.ContainerStatuses, core.ContainerStatus{ + Name: c, + State: core.ContainerState{Running: &core.ContainerStateRunning{}}, + }) + } + return p +} + +func TestSelectPod(t *testing.T) { + primary := testPod("pg-0", "primary", core.PodRunning, "postgres", "pg-coordinator") + standby := testPod("pg-1", "standby", core.PodRunning, "postgres", "pg-coordinator") + // The arbiter carries the same offshoot labels but runs no postgres + // container, so a catalog query cannot be exec-ed into it. + arbiter := testPod("pg-arbiter-0", "arbiter", core.PodRunning, "pg-coordinator") + pending := testPod("pg-2", "primary", core.PodPending, "postgres") + + cases := []struct { + name string + pods []core.Pod + primaryOnly bool + want string + wantErr bool + }{ + {name: "prefers the primary", pods: []core.Pod{standby, primary}, want: "pg-0"}, + {name: "prefers the primary for DDL too", pods: []core.Pod{standby, primary}, primaryOnly: true, want: "pg-0"}, + // A remote replica acting as the source of a chained replica has no + // primary at all; verification must still find a pod to query. + {name: "falls back to a standby", pods: []core.Pod{arbiter, standby}, want: "pg-1"}, + {name: "refuses DDL without a primary", pods: []core.Pod{standby}, primaryOnly: true, wantErr: true}, + {name: "skips pods without the container", pods: []core.Pod{arbiter}, wantErr: true}, + // An empty result used to be swallowed and reported as success, which + // produced a config referencing a user that was never created. + {name: "reports a pod that is not running", pods: []core.Pod{pending}, wantErr: true}, + {name: "reports an empty list", pods: nil, wantErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := selectPod(tc.pods, PostgresContainerName, tc.primaryOnly) + if tc.wantErr { + if err == nil { + t.Fatalf("expected an error, got pod %q", got.Name) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Name != tc.want { + t.Errorf("picked %q, want %q", got.Name, tc.want) + } + }) + } +} diff --git a/pkg/remote_replica/postgres.go b/pkg/remote_replica/postgres.go index 5e2bf9a79..58739db2e 100644 --- a/pkg/remote_replica/postgres.go +++ b/pkg/remote_replica/postgres.go @@ -38,7 +38,6 @@ import ( core "k8s.io/api/core/v1" kerr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" @@ -58,7 +57,7 @@ func PostgreSQlAPP(f cmdutil.Factory) *cobra.Command { var caCertPath, caKeyPath string var clientSANs []string var port int32 - var yes bool + var yes, skipUserCreation bool cmd := cobra.Command{ Use: "postgres", @@ -70,8 +69,12 @@ func PostgreSQlAPP(f cmdutil.Factory) *cobra.Command { if len(args) == 0 { log.Fatal("no database name given") } - if err := userPrompt(yes); err != nil { - log.Fatal(err) + // Nothing on the source is altered when user creation is skipped, so + // the "password will be altered" confirmation has nothing to confirm. + if !skipUserCreation { + if err := userPrompt(yes); err != nil { + log.Fatal(err) + } } // Issuing a certificate needs the CA's PRIVATE key; ca.crt alone cannot // sign anything, so the pair travels together. @@ -90,7 +93,7 @@ func PostgreSQlAPP(f cmdutil.Factory) *cobra.Command { } var buffer []byte - buffer, err := generateConfig(f, userName, password, dns, ns, authSecretName, replicaName, port, args[0], tlsIssueOptions{ + buffer, err := generateConfig(f, userName, password, dns, ns, authSecretName, replicaName, port, args[0], skipUserCreation, tlsIssueOptions{ CACertPath: caCertPath, CAKeyPath: caKeyPath, DNSSANs: clientSANs, @@ -137,6 +140,10 @@ func PostgreSQlAPP(f cmdutil.Factory) *cobra.Command { cmd.PersistentFlags().StringVar(&caCertPath, "ca-cert", "", "path to a CA certificate PEM; when set (together with --ca-key) the client certificate is issued locally from this CA instead of through cert-manager") cmd.PersistentFlags().StringVar(&caKeyPath, "ca-key", "", "path to the CA private key PEM matching --ca-cert; required to sign the client certificate") cmd.PersistentFlags().StringSliceVar(&clientSANs, "client-sans", nil, "comma separated DNS names to set as SANs on the generated client certificate") + cmd.PersistentFlags().BoolVar(&skipUserCreation, "skip-user-creation", false, + "do not create or alter the replication user on the source; assume it already exists with the given password. "+ + "Needed when the source has no writable primary to exec into - a standby, or a remote replica acting as the source of a chained replica, "+ + "where the replication role already arrived through WAL replication") return &cmd } @@ -149,9 +156,16 @@ type tlsIssueOptions struct { DNSSANs []string } -func generateConfig(f cmdutil.Factory, userName string, password string, dns string, ns string, authSecretName string, replicaName string, port int32, dbname string, tlsOpt tlsIssueOptions) ([]byte, error) { +func generateConfig(f cmdutil.Factory, userName string, password string, dns string, ns string, authSecretName string, replicaName string, port int32, dbname string, skipUserCreation bool, tlsOpt tlsIssueOptions) ([]byte, error) { var buffer []byte - opts, err := common.NewPostgresOpts(f, dbname, ns) + var dbOpts []common.OptionFunc + if skipUserCreation { + // Rendering manifests only reads the CR. A source stuck in a non-Ready + // phase - mid-failover, or standby-only - is precisely when a disaster + // recovery config is wanted, so do not refuse to run. + dbOpts = append(dbOpts, common.SkipReadinessCheck()) + } + opts, err := common.NewPostgresOpts(f, dbname, ns, dbOpts...) if err != nil { return nil, fmt.Errorf("failed to get db %s, err:%v", dbname, err) } @@ -162,7 +176,7 @@ func generateConfig(f cmdutil.Factory, userName string, password string, dns str return nil, fmt.Errorf("failed to get appbinding %v", err) } - authBuff, authSecretName, err := generateAuthSecret(userName, password, ns, authSecretName, opts) + authBuff, authSecretName, err := generateAuthSecret(userName, password, ns, authSecretName, skipUserCreation, opts) if err != nil { return nil, fmt.Errorf("failed to generate auth secret ,%v", err) } @@ -356,15 +370,20 @@ func generateTlsSecret(userName string, apb *appApi.AppBinding, ns string, extra return buffer, tlsSecret.Name, nil } -func generateAuthSecret(userName string, password string, ns string, secretName string, opts *common.PostgresOpts) ([]byte, string, error) { - if userName != opts.Username { +func generateAuthSecret(userName string, password string, ns string, secretName string, skipUserCreation bool, opts *common.PostgresOpts) ([]byte, string, error) { + switch { + case userName == opts.Username: + password = opts.Pass + case skipUserCreation: + if err := verifyUser(opts, userName); err != nil { + return nil, "", err + } + default: // generate user if not present err := generateUser(opts, userName, password) if err != nil { return nil, "", fmt.Errorf("failed to generate user err:%v", err) } - } else { - password = opts.Pass } if secretName == "" { secretName = fmt.Sprintf("%s-remote-replica-auth", opts.DB.Name) @@ -396,14 +415,48 @@ func generateAuthSecret(userName string, password string, ns string, secretName return buffer, AuthSecret.Name, nil } +// verifyUser checks the replication role without writing anything. Reading +// pg_roles works on a hot standby, so this runs against any member of the +// cluster. When no pod can be reached the check is skipped with a warning +// rather than failing: generating the config is still the useful outcome, and +// the point of --skip-user-creation is to work where the database does not +// answer. +func verifyUser(opts *common.PostgresOpts, name string) error { + pod, err := pickDBPod(opts.Client, opts.DB.Namespace, opts.DB.OffshootLabels(), PostgresContainerName, false) + if err != nil { + klog.Warningf("skipping verification of role %q: %v", name, err) + return nil + } + + out, err := exec_util.ExecIntoPod(opts.Config, pod, + exec_util.Command("psql", "-qtAXc", fmt.Sprintf("SELECT rolreplication FROM pg_roles WHERE rolname='%s'", name)), + exec_util.Container(PostgresContainerName), + ) + if err != nil { + klog.Warningf("skipping verification of role %q: failed to query %s: %v", name, pod.Name, err) + return nil + } + + switch strings.TrimSpace(out) { + case "": + return fmt.Errorf("role %q does not exist on %s/%s; create it on the writable primary first:\n "+ + "CREATE USER %s WITH REPLICATION PASSWORD ''; GRANT EXECUTE ON FUNCTION pg_read_binary_file(text) TO %s;", + name, opts.DB.Namespace, opts.DB.Name, name, name) + case "f", "false": + return fmt.Errorf("role %q exists on %s/%s but does not have the REPLICATION attribute; run on the writable primary:\n "+ + "ALTER ROLE %s WITH REPLICATION;", name, opts.DB.Namespace, opts.DB.Name, name) + } + + fmt.Printf("role %q verified on %s (replication enabled); leaving the source catalog untouched\n", name, pod.Name) + return nil +} + func generateUser(opts *common.PostgresOpts, name string, password string) error { - label := opts.DB.OffshootLabels() - label["kubedb.com/role"] = "primary" - pods, err := opts.Client.CoreV1().Pods(opts.DB.Namespace).List(context.TODO(), metav1.ListOptions{ - LabelSelector: labels.Set.String(label), - }) - if err != nil || len(pods.Items) == 0 { - return err + // DDL only: the writable primary is mandatory here. + pod, err := pickDBPod(opts.Client, opts.DB.Namespace, opts.DB.OffshootLabels(), PostgresContainerName, true) + if err != nil { + return fmt.Errorf("%v; pass --skip-user-creation to generate the config against a standby "+ + "whose replication role already exists", err) } query := fmt.Sprintf("SELECT rolname FROM pg_roles WHERE rolname='%s'", name) @@ -415,7 +468,7 @@ func generateUser(opts *common.PostgresOpts, name string, password string) error container, } - out, err := exec_util.ExecIntoPod(opts.Config, &pods.Items[0], options...) + out, err := exec_util.ExecIntoPod(opts.Config, pod, options...) if err != nil { return err } @@ -432,7 +485,7 @@ func generateUser(opts *common.PostgresOpts, name string, password string) error container, } - out, err = exec_util.ExecIntoPod(opts.Config, &pods.Items[0], options...) + out, err = exec_util.ExecIntoPod(opts.Config, pod, options...) if err != nil { return err }