Skip to content

fix: force VerifyServerDataDir fallback over TCP #3485

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

Closed
Bella-Giraffety wants to merge 1 commit into from
Closed

fix: force VerifyServerDataDir fallback over TCP #3485

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
75 changes: 73 additions & 2 deletions internal/doltserver/doltserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,46 @@ func buildDoltSQLCmd(ctx context.Context, config *Config, args ...string) *exec.
cmd.Dir = config.DataDir
setProcessGroup(cmd)

if config.IsRemote() && config.Password != "" {
if config.Password != "" {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+config.Password)
} else if config.IsRemote() {
if inherited, ok := os.LookupEnv("DOLT_CLI_PASSWORD"); ok {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+inherited)
} else {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=")
}
} else {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=")
}

return cmd
}

// buildDoltSQLTCPClientCmd constructs a dolt sql command that always connects
// to the running server over TCP, even when the configured host is local. This
// is used by verification paths that must avoid embedded-mode fallback.
func buildDoltSQLTCPClientCmd(ctx context.Context, config *Config, args ...string) *exec.Cmd {
fullArgs := []string{
"--host", config.EffectiveHost(),
"--port", strconv.Itoa(config.Port),
"--user", config.User,
"--no-tls",
"sql",
}
fullArgs = append(fullArgs, args...)

cmd := exec.CommandContext(ctx, "dolt", fullArgs...)
cmd.Dir = config.DataDir
if config.Password != "" {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+config.Password)
} else if config.IsRemote() {
if inherited, ok := os.LookupEnv("DOLT_CLI_PASSWORD"); ok {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD="+inherited)
} else {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=")
}
} else {
cmd.Env = append(os.Environ(), "DOLT_CLI_PASSWORD=")
}

return cmd
Expand Down Expand Up @@ -1018,7 +1056,7 @@ func VerifyServerDataDir(townRoot string) (bool, error) {
return true, nil
}

served, _, verifyErr := VerifyDatabases(townRoot)
served, verifyErr := queryServedDatabasesTCPClient(townRoot)
if verifyErr != nil {
return false, fmt.Errorf("could not query server databases: %w", verifyErr)
}
Expand All @@ -1041,6 +1079,39 @@ func VerifyServerDataDir(townRoot string) (bool, error) {
return true, nil
}

func queryServedDatabasesTCPClient(townRoot string) ([]string, error) {
config := DefaultConfig(townRoot)
if err := CheckServerReachable(townRoot); err != nil {
return nil, fmt.Errorf("server not reachable: %w", err)
}

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
cmd := buildDoltSQLTCPClientCmd(ctx, config,
"-r", "json",
"-q", "SHOW DATABASES",
)

var stderrBuf bytes.Buffer
cmd.Stderr = &stderrBuf
output, queryErr := cmd.Output()
cancel()
if queryErr != nil {
stderrMsg := strings.TrimSpace(stderrBuf.String())
errDetail := strings.TrimSpace(string(output))
if stderrMsg != "" {
errDetail = errDetail + " (stderr: " + stderrMsg + ")"
}
return nil, fmt.Errorf("querying SHOW DATABASES: %w (output: %s)", queryErr, errDetail)
}

served, parseErr := parseShowDatabases(output)
if parseErr != nil {
return nil, fmt.Errorf("parsing SHOW DATABASES output: %w", parseErr)
}

return served, nil
}

// KillImposters finds and kills any dolt sql-server process on the configured
// port that is NOT serving from the expected data directory. This handles the
// case where another tool (e.g., bd) launched its own embedded Dolt server
Expand Down
110 changes: 107 additions & 3 deletions internal/doltserver/doltserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3606,12 +3606,116 @@ func TestBuildDoltSQLCmd_RemoteNoPassword(t *testing.T) {
ctx := t.Context()
cmd := buildDoltSQLCmd(ctx, config, "-q", "SELECT 1")

// Should NOT have DOLT_CLI_PASSWORD in env
// Should set empty DOLT_CLI_PASSWORD to suppress interactive prompts.
for _, env := range cmd.Env {
if strings.HasPrefix(env, "DOLT_CLI_PASSWORD=") {
t.Error("remote cmd without password should not have DOLT_CLI_PASSWORD env var")
if env == "DOLT_CLI_PASSWORD=" {
return
}
}
t.Error("remote cmd without password should set empty DOLT_CLI_PASSWORD env var")
}

func TestBuildDoltSQLTCPClientCmd_Local(t *testing.T) {
config := &Config{
Host: "",
Port: 3307,
User: "root",
DataDir: "/tmp/dolt-data",
}

cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1")

if cmd.Dir != config.DataDir {
t.Errorf("cmd.Dir = %q, want %q", cmd.Dir, config.DataDir)
}

wantArgs := []string{"dolt", "--host", "127.0.0.1", "--port", "3307", "--user", "root", "--no-tls", "sql", "-q", "SELECT 1"}
if len(cmd.Args) != len(wantArgs) {
t.Fatalf("len(cmd.Args) = %d, want %d; args=%v", len(cmd.Args), len(wantArgs), cmd.Args)
}
for i, want := range wantArgs {
if cmd.Args[i] != want {
t.Errorf("cmd.Args[%d] = %q, want %q", i, cmd.Args[i], want)
}
}

found := false
for _, env := range cmd.Env {
if env == "DOLT_CLI_PASSWORD=" {
found = true
break
}
}
if !found {
t.Error("local explicit TCP cmd should set empty DOLT_CLI_PASSWORD env var")
}
}

func TestBuildDoltSQLTCPClientCmd_Remote(t *testing.T) {
config := &Config{
Host: "10.0.0.5",
Port: 3307,
User: "root",
Password: "secret",
DataDir: "/tmp/dolt-data",
}

cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1")

if cmd.Dir != config.DataDir {
t.Errorf("cmd.Dir = %q, want %q", cmd.Dir, config.DataDir)
}

wantArgs := []string{"dolt", "--host", "10.0.0.5", "--port", "3307", "--user", "root", "--no-tls", "sql", "-q", "SELECT 1"}
if len(cmd.Args) != len(wantArgs) {
t.Fatalf("len(cmd.Args) = %d, want %d; args=%v", len(cmd.Args), len(wantArgs), cmd.Args)
}
for i, want := range wantArgs {
if cmd.Args[i] != want {
t.Errorf("cmd.Args[%d] = %q, want %q", i, cmd.Args[i], want)
}
}

found := false
for _, env := range cmd.Env {
if env == "DOLT_CLI_PASSWORD=secret" {
found = true
break
}
}
if !found {
t.Error("remote explicit TCP cmd should set DOLT_CLI_PASSWORD env var")
}
}

func TestBuildDoltSQLTCPClientCmd_RemoteNoPasswordPreservesInheritedCredentials(t *testing.T) {
t.Setenv("DOLT_CLI_PASSWORD", "secret-from-env")

config := &Config{
Host: "10.0.0.5",
Port: 3307,
User: "root",
DataDir: "/tmp/dolt-data",
}

cmd := buildDoltSQLTCPClientCmd(t.Context(), config, "-q", "SELECT 1")

foundInherited := false
foundEmpty := false
for _, env := range cmd.Env {
if env == "DOLT_CLI_PASSWORD=secret-from-env" {
foundInherited = true
}
if env == "DOLT_CLI_PASSWORD=" {
foundEmpty = true
}
}
if !foundInherited {
t.Fatal("expected remote explicit TCP helper to preserve inherited DOLT_CLI_PASSWORD")
}
if foundEmpty {
t.Fatal("did not expect remote explicit TCP helper to overwrite inherited credentials with empty password")
}
}

// =============================================================================
Expand Down