Skip to content

feat: use rsandbox as ProxySQL monitor, add R/W split proxy users (#69, #70) #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
renecannao merged 1 commit into from
Apr 3, 2026
Merged

feat: use rsandbox as ProxySQL monitor, add R/W split proxy users (#69, #70) #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions providers/proxysql/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,35 @@ func GenerateConfig(cfg ProxySQLConfig) string {
b.WriteString(")\n\n")
}

b.WriteString(fmt.Sprintf("%s=\n(\n", usersKey))
b.WriteString(" {\n")
b.WriteString(fmt.Sprintf(" username=\"%s\"\n", cfg.MonitorUser))
b.WriteString(fmt.Sprintf(" password=\"%s\"\n", cfg.MonitorPass))
b.WriteString(" default_hostgroup=0\n")
b.WriteString(" }\n")
b.WriteString(")\n")
if isPgsql {
// PostgreSQL: single user entry (no R/W split)
b.WriteString(fmt.Sprintf("%s=\n(\n", usersKey))
b.WriteString(" {\n")
b.WriteString(fmt.Sprintf(" username=\"%s\"\n", cfg.MonitorUser))
b.WriteString(fmt.Sprintf(" password=\"%s\"\n", cfg.MonitorPass))
b.WriteString(" default_hostgroup=0\n")
b.WriteString(" }\n")
b.WriteString(")\n")
} else {
// MySQL: three users — general purpose, dedicated writer, dedicated reader
b.WriteString(fmt.Sprintf("%s=\n(\n", usersKey))
b.WriteString(" {\n")
b.WriteString(" username=\"msandbox\"\n")
b.WriteString(" password=\"msandbox\"\n")
b.WriteString(" default_hostgroup=0\n")
b.WriteString(" },\n")
b.WriteString(" {\n")
b.WriteString(" username=\"msandbox_rw\"\n")
b.WriteString(" password=\"msandbox\"\n")
b.WriteString(" default_hostgroup=0\n")
b.WriteString(" },\n")
b.WriteString(" {\n")
b.WriteString(" username=\"msandbox_ro\"\n")
b.WriteString(" password=\"msandbox\"\n")
b.WriteString(" default_hostgroup=1\n")
Comment on lines +95 to +110
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenerateConfig() now hard-codes MySQL frontend users/passwords to msandbox/msandbox (and msandbox_rw/msandbox_ro). This makes ProxySQL config incorrect when the sandbox is deployed with non-default --db-user/--db-password (the grants templates create these users with {{.DbUser}}/{{.DbPassword}}). Consider deriving these usernames/passwords from inputs (e.g., add fields to ProxySQLConfig or provider options for proxy user/password) so ProxySQL stays in sync with the sandbox credentials.

Copilot uses AI. Check for mistakes.
b.WriteString(" }\n")
b.WriteString(")\n")
Comment on lines +95 to +112
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The MySQL proxy users are now hardcoded to msandbox variants. This is inconsistent with the PostgreSQL implementation (lines 87-93) which still uses the configurable MonitorUser. This change prevents users from customizing the application user credentials via the monitor_user option as they could before. Consider allowing these to be configurable or at least using the monitor credentials as a fallback.

}

// For Group Replication / InnoDB Cluster topologies, configure ProxySQL
// to monitor GR status and automatically reroute on failover.
Expand Down
30 changes: 22 additions & 8 deletions providers/proxysql/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ func TestGenerateConfigBasic(t *testing.T) {
AdminHost: "127.0.0.1", AdminPort: 6032,
AdminUser: "admin", AdminPassword: "admin",
MySQLPort: 6033, DataDir: "/tmp/test",
MonitorUser: "msandbox", MonitorPass: "msandbox",
MonitorUser: "rsandbox", MonitorPass: "rsandbox",
}
result := GenerateConfig(cfg)
checks := []string{
`admin_credentials="admin:admin"`,
`interfaces="127.0.0.1:6033"`,
`monitor_username="msandbox"`,
`monitor_username="rsandbox"`,
`mysql_ifaces="127.0.0.1:6032"`,
}
for _, check := range checks {
Expand All @@ -31,7 +31,7 @@ func TestGenerateConfigWithBackends(t *testing.T) {
AdminHost: "127.0.0.1", AdminPort: 6032,
AdminUser: "admin", AdminPassword: "admin",
MySQLPort: 6033, DataDir: "/tmp/test",
MonitorUser: "msandbox", MonitorPass: "msandbox",
MonitorUser: "rsandbox", MonitorPass: "rsandbox",
Backends: []BackendServer{
{Host: "127.0.0.1", Port: 3306, Hostgroup: 0, MaxConns: 100},
{Host: "127.0.0.1", Port: 3307, Hostgroup: 1, MaxConns: 100},
Expand All @@ -57,8 +57,8 @@ func TestGenerateConfigMySQL(t *testing.T) {
AdminPassword: "admin",
MySQLPort: 6033,
DataDir: "/tmp/proxysql/data",
MonitorUser: "msandbox",
MonitorPass: "msandbox",
MonitorUser: "rsandbox",
MonitorPass: "rsandbox",
Backends: []BackendServer{
{Host: "127.0.0.1", Port: 3306, Hostgroup: 0, MaxConns: 200},
},
Expand All @@ -73,6 +73,20 @@ func TestGenerateConfigMySQL(t *testing.T) {
if !strings.Contains(config, "mysql_users") {
t.Error("expected mysql_users block")
}
// Monitor user should be rsandbox (not in mysql_users, only in mysql_variables)
if !strings.Contains(config, `monitor_username="rsandbox"`) {
t.Error("expected monitor_username=rsandbox")
}
// Three proxy users for R/W split
if !strings.Contains(config, `username="msandbox"`) {
t.Error("expected msandbox user in mysql_users")
}
if !strings.Contains(config, `username="msandbox_rw"`) {
t.Error("expected msandbox_rw user in mysql_users")
}
if !strings.Contains(config, `username="msandbox_ro"`) {
t.Error("expected msandbox_ro user in mysql_users")
}
}

func TestGenerateConfigPostgreSQL(t *testing.T) {
Expand Down Expand Up @@ -114,7 +128,7 @@ func TestGenerateConfigGRHostgroups(t *testing.T) {
AdminHost: "127.0.0.1", AdminPort: 6032,
AdminUser: "admin", AdminPassword: "admin",
MySQLPort: 6033, DataDir: "/tmp/test",
MonitorUser: "msandbox", MonitorPass: "msandbox",
MonitorUser: "rsandbox", MonitorPass: "rsandbox",
Topology: "innodb-cluster",
Backends: []BackendServer{
{Host: "127.0.0.1", Port: 3306, Hostgroup: 0, MaxConns: 200},
Expand All @@ -141,7 +155,7 @@ func TestGenerateConfigNoGRHostgroupsForReplication(t *testing.T) {
AdminHost: "127.0.0.1", AdminPort: 6032,
AdminUser: "admin", AdminPassword: "admin",
MySQLPort: 6033, DataDir: "/tmp/test",
MonitorUser: "msandbox", MonitorPass: "msandbox",
MonitorUser: "rsandbox", MonitorPass: "rsandbox",
Topology: "replication",
Backends: []BackendServer{
{Host: "127.0.0.1", Port: 3306, Hostgroup: 0, MaxConns: 200},
Expand All @@ -158,7 +172,7 @@ func TestGenerateConfigGRHostgroupsForGroup(t *testing.T) {
AdminHost: "127.0.0.1", AdminPort: 6032,
AdminUser: "admin", AdminPassword: "admin",
MySQLPort: 6033, DataDir: "/tmp/test",
MonitorUser: "msandbox", MonitorPass: "msandbox",
MonitorUser: "rsandbox", MonitorPass: "rsandbox",
Topology: "group",
Backends: []BackendServer{
{Host: "127.0.0.1", Port: 3306, Hostgroup: 0, MaxConns: 200},
Expand Down
9 changes: 5 additions & 4 deletions providers/proxysql/proxysql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ func (p *ProxySQLProvider) CreateSandbox(config providers.SandboxConfig) (*provi

monitorUser := config.Options["monitor_user"]
if monitorUser == "" {
monitorUser = "msandbox"
monitorUser = "rsandbox"
}
monitorPass := config.Options["monitor_password"]
if monitorPass == "" {
monitorPass = "msandbox"
monitorPass = "rsandbox"
}
Comment on lines 69 to 75
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The default monitor user and password are changed to rsandbox globally. This will likely break PostgreSQL sandboxes, which typically use postgres or msandbox and do not recognize the rsandbox user or the MySQL-specific replication privileges mentioned in the PR description. The default should be conditional on the backend provider.


host := config.Host
Expand Down Expand Up @@ -117,8 +117,9 @@ func (p *ProxySQLProvider) CreateSandbox(config providers.SandboxConfig) (*provi
scripts["use_proxy"] = fmt.Sprintf("#!/bin/bash\npsql -h %s -p %d -U %s \"$@\"\n",
host, mysqlPort, monitorUser)
} else {
scripts["use_proxy"] = fmt.Sprintf("#!/bin/bash\nmysql -h %s -P %d -u %s -p%s --prompt 'ProxySQL> ' \"$@\"\n",
host, mysqlPort, monitorUser, monitorPass)
// use_proxy connects as msandbox (app user), not the monitor user (rsandbox)
scripts["use_proxy"] = fmt.Sprintf("#!/bin/bash\nmysql -h %s -P %d -u msandbox -pmsandbox --prompt 'ProxySQL> ' \"$@\"\n",
host, mysqlPort)
Comment on lines +121 to +122
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use_proxy script for MySQL hardcodes the username and password (-u msandbox -pmsandbox). This makes the script fragile if the user credentials in the configuration are ever changed. It also makes it difficult to override the user via command-line arguments as the hardcoded flags will still be present and may conflict with user-provided arguments.

Comment on lines +120 to +122
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated use_proxy script is now hard-coded to connect as msandbox/msandbox for MySQL. This will fail for sandboxes created with custom --db-user/--db-password (and also makes it impossible to easily test msandbox_rw/msandbox_ro via the helper script). Suggest making the proxy client credentials configurable (e.g., via provider options) and/or generating separate helper scripts for rw/ro users.

Copilot uses AI. Check for mistakes.
}

for name, content := range scripts {
Expand Down
4 changes: 2 additions & 2 deletions sandbox/proxysql_topology.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ func DeployProxySQLForTopology(sandboxDir string, masterPort int, slavePorts []i
DbUser: "admin",
DbPassword: "admin",
Options: map[string]string{
"monitor_user": "msandbox",
"monitor_password": "msandbox",
"monitor_user": "rsandbox",
"monitor_password": "rsandbox",
"backends": strings.Join(backendParts, ","),
"backend_provider": backendProvider,
Comment on lines 86 to 90
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeployProxySQLForTopology hard-codes monitor_user/monitor_password to rsandbox. This breaks --with-proxysql for PostgreSQL sandboxes (cmd/single.go passes backendProvider="postgresql" but PostgreSQL sandboxes use DbUser="postgres"), causing ProxySQL to be configured with a non-existent monitoring/frontend user. Please set monitor credentials based on backendProvider (e.g., default to postgres for postgresql) and/or accept monitor_user/monitor_password as inputs instead of hard-coding them here.

Copilot uses AI. Check for mistakes.
"topology": topologyName(topology),
Expand Down
18 changes: 15 additions & 3 deletions test/proxysql-integration-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,22 @@ section "Configuration correctness"
dbdeployer deploy single $MYSQL_VERSION_1 --sandbox-binary=$SANDBOX_BINARY --with-proxysql > /dev/null 2>&1
SANDBOX_DIR=~/sandboxes/msb_$(echo $MYSQL_VERSION_1 | tr '.' '_')

if grep -q "msandbox" ${SANDBOX_DIR}/proxysql/proxysql.cnf 2>/dev/null; then
pass "config contains monitor_username=msandbox"
if grep -q 'monitor_username="rsandbox"' ${SANDBOX_DIR}/proxysql/proxysql.cnf 2>/dev/null; then
pass "config contains monitor_username=rsandbox"
else
fail "monitor username in config" "not found"
fail "monitor username in config" "rsandbox not found"
fi

if grep -q 'username="msandbox_rw"' ${SANDBOX_DIR}/proxysql/proxysql.cnf 2>/dev/null; then
pass "config contains msandbox_rw proxy user"
else
fail "msandbox_rw proxy user in config" "not found"
fi

if grep -q 'username="msandbox_ro"' ${SANDBOX_DIR}/proxysql/proxysql.cnf 2>/dev/null; then
pass "config contains msandbox_ro proxy user"
else
fail "msandbox_ro proxy user in config" "not found"
fi

if grep -q 'admin_credentials="admin:admin"' ${SANDBOX_DIR}/proxysql/proxysql.cnf 2>/dev/null; then
Expand Down
Loading