Skip to content
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.

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

Remove user and password options for sql-server #8800

Merged
merged 1 commit into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 2 additions & 26 deletions go/cmd/dolt/commands/sqlserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,6 @@ func ConfigureServices(
// instead of dolt db initialization, because we only want to create the privileges database when it's
// used for a server, and because we want the same root initialization logic when a sql-server is started
// for a clone. More details: https://dev.mysql.com/doc/mysql-security-excerpt/8.0/en/default-privileges.html
//
// NOTE: The MySQL root user is created for host 'localhost', not any host ('%'). We could do the same here,
// but it seems like it would cause problems for users who want to connect from outside of Docker.
InitImplicitRootSuperUser := &svcs.AnonService{
InitF: func(ctx context.Context) error {
// If privileges.db has already been initialized, indicating that this is NOT the
Expand All @@ -395,9 +392,8 @@ func ConfigureServices(
ed := mysqlDb.Editor()
defer ed.Close()

// If no ephemeral superuser has been configured and root user initialization wasn't skipped,
// then create a root@localhost superuser.
if !serverConfig.UserIsSpecified() && !config.SkipRootUserInitialization {
// Create the root@localhost superuser, unless --skip-root-user-initialization was specified
if !config.SkipRootUserInitialization {
// Allow the user to override the default root host (localhost) and password ("").
// This is particularly useful in a Docker container, where you need to connect
// to the sql-server from outside the container and can't rely on localhost.
Expand Down Expand Up @@ -436,26 +432,6 @@ func ConfigureServices(
}
controller.Register(InitImplicitRootSuperUser)

// Add an ephemeral superuser if one was requested
InitEphemeralSuperUser := &svcs.AnonService{
InitF: func(context.Context) error {
mysqlDb := sqlEngine.GetUnderlyingEngine().Analyzer.Catalog.MySQLDb
ed := mysqlDb.Editor()

userSpecified := config.ServerUser != ""
if userSpecified {
superuser := mysqlDb.GetUser(ed, config.ServerUser, "%", false)
if superuser == nil {
mysqlDb.AddEphemeralSuperUser(ed, config.ServerUser, "%", config.ServerPass)
}
}
ed.Close()

return nil
},
}
controller.Register(InitEphemeralSuperUser)

var metListener *metricsListener
InitMetricsListener := &svcs.AnonService{
InitF: func(context.Context) (err error) {
Expand Down
38 changes: 23 additions & 15 deletions go/cmd/dolt/commands/sqlserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ func TestServerArgs(t *testing.T) {
StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
"-H", "localhost",
"-P", "15200",
"-u", "username",
"-p", "password",
"-t", "5",
"-l", "info",
"-r",
Expand All @@ -93,17 +91,34 @@ func TestServerArgs(t *testing.T) {
assert.NoError(t, err)
}

func TestDeprecatedUserPasswordServerArgs(t *testing.T) {
controller := svcs.NewController()
dEnv, err := sqle.CreateEnvWithSeedData()
require.NoError(t, err)
defer func() {
assert.NoError(t, dEnv.DoltDB.Close())
}()
err = StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
"-H", "localhost",
"-P", "15200",
"-u", "username",
"-p", "password",
"-t", "5",
"-l", "info",
"-r",
}, dEnv, dEnv.FS, controller)
require.Error(t, err)
require.Contains(t, err.Error(), "--user and --password have been removed from the sql-server command.")
require.Contains(t, err.Error(), "Create users explicitly with CREATE USER and GRANT statements instead.")
}

func TestYAMLServerArgs(t *testing.T) {
const yamlConfig = `
log_level: info

behavior:
read_only: true

user:
name: username
password: password

listener:
host: localhost
port: 15200
Expand All @@ -117,11 +132,11 @@ listener:
}()
controller := svcs.NewController()
go func() {

dEnv.FS.WriteFile("config.yaml", []byte(yamlConfig), os.ModePerm)
StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
err := StartServer(context.Background(), "0.0.0", "dolt sql-server", []string{
"--config", "config.yaml",
}, dEnv, dEnv.FS, controller)
require.NoError(t, err)
}()
err = controller.WaitForStart()
require.NoError(t, err)
Expand Down Expand Up @@ -284,8 +299,6 @@ func TestServerFailsIfPortInUse(t *testing.T) {
StartServer(context.Background(), "test", "dolt sql-server", []string{
"-H", "localhost",
"-P", "15200",
"-u", "username",
"-p", "password",
"-t", "5",
"-l", "info",
"-r",
Expand Down Expand Up @@ -516,7 +529,6 @@ func TestReadReplica(t *testing.T) {

func TestGenerateYamlConfig(t *testing.T) {
args := []string{
"--user", "my_name",
"--timeout", "11",
"--branch-control-file", "dir1/dir2/abc.db",
}
Expand All @@ -543,10 +555,6 @@ func TestGenerateYamlConfig(t *testing.T) {
# dolt_transaction_commit: false
# event_scheduler: "OFF"

user:
name: my_name
# password: ""

listener:
# host: localhost
# port: 3306
Expand Down
34 changes: 17 additions & 17 deletions go/cmd/dolt/commands/sqlserver/sqlserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/dolthub/go-mysql-server/sql"
"github.com/fatih/color"
"github.com/sirupsen/logrus"

"github.com/dolthub/dolt/go/cmd/dolt/cli"
"github.com/dolthub/dolt/go/cmd/dolt/commands"
Expand All @@ -39,7 +40,6 @@ const (
hostFlag = "host"
portFlag = "port"
skipRootUserInitialization = "skip-root-user-initialization"
passwordFlag = "password"
timeoutFlag = "timeout"
readonlyFlag = "readonly"
logLevelFlag = "loglevel"
Expand Down Expand Up @@ -96,10 +96,6 @@ SUPPORTED CONFIG FILE FIELDS:

{{.EmphasisLeft}}behavior.dolt_transaction_commit{{.EmphasisRight}}: If true all SQL transaction commits will automatically create a Dolt commit, with a generated commit message. This is useful when a system working with Dolt wants to create versioned data, but doesn't want to directly use Dolt features such as dolt_commit().

{{.EmphasisLeft}}user.name{{.EmphasisRight}}: The username for an ephemeral superuser that will exist for the lifetime of the sql-server process. This user will not be persisted to the privileges database.

{{.EmphasisLeft}}user.password{{.EmphasisRight}}: The password for an ephemeral superuser that will exist for the lifetime of the sql-server process. This user will not be persisted to the privileges database.

{{.EmphasisLeft}}listener.host{{.EmphasisRight}}: The host address that the server will run on. This may be {{.EmphasisLeft}}localhost{{.EmphasisRight}} or an IPv4 or IPv6 address

{{.EmphasisLeft}}listener.port{{.EmphasisRight}}: The port that the server should listen on
Expand Down Expand Up @@ -129,7 +125,7 @@ SUPPORTED CONFIG FILE FIELDS:
If a config file is not provided many of these settings may be configured on the command line.`,
Synopsis: []string{
"--config {{.LessThan}}file{{.GreaterThan}}",
"[-H {{.LessThan}}host{{.GreaterThan}}] [-P {{.LessThan}}port{{.GreaterThan}}] [-u {{.LessThan}}user{{.GreaterThan}}] [-p {{.LessThan}}password{{.GreaterThan}}] [-t {{.LessThan}}timeout{{.GreaterThan}}] [-l {{.LessThan}}loglevel{{.GreaterThan}}] [--data-dir {{.LessThan}}directory{{.GreaterThan}}] [-r]",
"[-H {{.LessThan}}host{{.GreaterThan}}] [-P {{.LessThan}}port{{.GreaterThan}}] [-t {{.LessThan}}timeout{{.GreaterThan}}] [-l {{.LessThan}}loglevel{{.GreaterThan}}] [--data-dir {{.LessThan}}directory{{.GreaterThan}}] [-r]",
},
}

Expand Down Expand Up @@ -163,9 +159,11 @@ func (cmd SqlServerCmd) ArgParserWithName(name string) *argparser.ArgParser {
ap.SupportsString(configFileFlag, "", "file", "When provided configuration is taken from the yaml config file and all command line parameters are ignored.")
ap.SupportsString(hostFlag, "H", "host address", fmt.Sprintf("Defines the host address that the server will run on. Defaults to `%v`.", serverConfig.Host()))
ap.SupportsUint(portFlag, "P", "port", fmt.Sprintf("Defines the port that the server will run on. Defaults to `%v`.", serverConfig.Port()))
ap.SupportsString(commands.UserFlag, "u", "user", fmt.Sprintf("Defines the server user. Defaults to `%v`. This should be explicit if desired.", serverConfig.User()))
// TODO: After December 2025, remove the deprecated user/password arguments completely from the command line
// and config file options for dolt sql-server.
ap.SupportsString(commands.UserFlag, "u", "user", "This option is no longer supported. Instead, you can create users using CREATE USER and GRANT SQL statements.")
fulghum marked this conversation as resolved.
Show resolved Hide resolved
ap.SupportsFlag(skipRootUserInitialization, "", "Skips the automatic creation of a default root super user on the first launch of a SQL server.")
ap.SupportsString(passwordFlag, "p", "password", fmt.Sprintf("Defines the server password. Defaults to `%v`.", serverConfig.Password()))
ap.SupportsString("password", "p", "password", "This option is no longer supported. Instead, you can create users using CREATE USER and GRANT SQL statements.")
ap.SupportsInt(timeoutFlag, "t", "connection timeout", fmt.Sprintf("Defines the timeout, in seconds, used for connections\nA value of `0` represents an infinite timeout. Defaults to `%v`.", serverConfig.ReadTimeout()))
ap.SupportsFlag(readonlyFlag, "r", "Disable modification of the database.")
ap.SupportsString(logLevelFlag, "l", "log level", fmt.Sprintf("Defines the level of logging provided\nOptions are: `trace`, `debug`, `info`, `warning`, `error`, `fatal`. Defaults to `%v`.", serverConfig.LogLevel()))
Expand Down Expand Up @@ -235,6 +233,12 @@ func validateSqlServerArgs(apr *argparser.ArgParseResults) error {
if multiDbDir {
cli.PrintErrln("WARNING: --multi-db-dir is deprecated, use --data-dir instead")
}
_, userSpecified := apr.GetValue(commands.UserFlag)
if userSpecified {
return fmt.Errorf("ERROR: --user and --password have been removed from the sql-server command. " +
"Create users explicitly with CREATE USER and GRANT statements instead.")
}

return nil
}

Expand Down Expand Up @@ -373,22 +377,18 @@ func getServerConfig(cwdFS filesys.Filesys, apr *argparser.ArgParseResults, data
return nil, err
}

// if command line user argument was given, override the config file's user and password
if user, hasUser := apr.GetValue(commands.UserFlag); hasUser {
if wcfg, ok := cfg.(servercfg.WritableServerConfig); ok {
pass, _ := apr.GetValue(passwordFlag)
wcfg.SetUserName(user)
wcfg.SetPassword(pass)
}
}

if connStr, ok := apr.GetValue(goldenMysqlConn); ok {
if yamlCfg, ok := cfg.(servercfg.YAMLConfig); ok {
cli.Println(connStr)
yamlCfg.GoldenMysqlConn = &connStr
}
}

if cfg.UserIsSpecified() {
logrus.Warn("user and password are no longer supported in sql-server configuration files." +
"Use CREATE USER and GRANT statements to manage user accounts.")
}

return cfg, nil
}

Expand Down
15 changes: 0 additions & 15 deletions go/libraries/doltcore/servercfg/yaml_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,6 @@ func ServerConfigAsYAMLConfig(cfg ServerConfig) *YAMLConfig {
DoltTransactionCommit: ptr(cfg.DoltTransactionCommit()),
EventSchedulerStatus: ptr(cfg.EventSchedulerStatus()),
},
UserConfig: UserYAMLConfig{
Name: ptr(cfg.User()),
Password: ptr(cfg.Password()),
},
ListenerConfig: ListenerYAMLConfig{
HostStr: ptr(cfg.Host()),
PortNumber: ptr(cfg.Port()),
Expand Down Expand Up @@ -260,10 +256,6 @@ func ServerConfigSetValuesAsYAMLConfig(cfg ServerConfig) *YAMLConfig {
DoltTransactionCommit: zeroIf(ptr(cfg.DoltTransactionCommit()), !cfg.ValueSet(DoltTransactionCommitKey)),
EventSchedulerStatus: zeroIf(ptr(cfg.EventSchedulerStatus()), !cfg.ValueSet(EventSchedulerKey)),
},
UserConfig: UserYAMLConfig{
Name: zeroIf(ptr(cfg.User()), !cfg.ValueSet(UserKey)),
Password: zeroIf(ptr(cfg.Password()), !cfg.ValueSet(PasswordKey)),
},
ListenerConfig: ListenerYAMLConfig{
HostStr: zeroIf(ptr(cfg.Host()), !cfg.ValueSet(HostKey)),
PortNumber: zeroIf(ptr(cfg.Port()), !cfg.ValueSet(PortKey)),
Expand Down Expand Up @@ -473,13 +465,6 @@ func (cfg YAMLConfig) withDefaultsFilledIn() YAMLConfig {
withDefaults.BehaviorConfig.DoltTransactionCommit = defaults.BehaviorConfig.DoltTransactionCommit
}

if withDefaults.UserConfig.Name == nil {
withDefaults.UserConfig.Name = defaults.UserConfig.Name
}
if withDefaults.UserConfig.Password == nil {
withDefaults.UserConfig.Password = defaults.UserConfig.Password
}

if withDefaults.ListenerConfig.HostStr == nil {
withDefaults.ListenerConfig.HostStr = defaults.ListenerConfig.HostStr
}
Expand Down
4 changes: 0 additions & 4 deletions go/libraries/doltcore/servercfg/yaml_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ behavior:
disable_client_multi_statements: false
event_scheduler: ON

user:
name: ""
password: ""

listener:
host: localhost
port: 3306
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,11 @@ func (h *harness) startDoltSqlServer(doltPersistentSystemVars map[string]string)

// use an admin user NOT named "root" to test that we don't require the "root" account
adminUser := "admin"
err = runDoltCommand(h.t, dir, "sql", fmt.Sprintf("-q=%s",
"CREATE USER IF NOT EXISTS admin@'%'; GRANT ALL ON *.* TO admin@'%';"))
if err != nil {
return -1, nil, err
}

if doltPersistentSystemVars != nil && len(doltPersistentSystemVars) > 0 {
// Initialize the dolt directory first
Expand All @@ -1066,7 +1071,6 @@ func (h *harness) startDoltSqlServer(doltPersistentSystemVars map[string]string)

args := []string{DoltDevBuildPath(),
"sql-server",
fmt.Sprintf("-u%s", adminUser),
"--loglevel=TRACE",
"--socket=/dev/null",
fmt.Sprintf("--data-dir=%s", dir),
Expand Down
1 change: 1 addition & 0 deletions integration-tests/bats/branch-control.bats
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ setup_test_user() {
dolt sql -q "create user test identified by ''"
dolt sql -q "grant all on *.* to test"
dolt sql -q "delete from dolt_branch_control where user='%'"
SQL_USER=test
}

@test "branch-control: fresh database. branch control tables exist" {
Expand Down
5 changes: 3 additions & 2 deletions integration-tests/bats/cli-hosted.bats
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ setup() {
# creation is done here. We may want to move this to helper/query-server-common.bash later.
PORT=$( definePORT )
DOLT_CLI_PASSWORD="d01t"
dolt sql-server --host 0.0.0.0 --port=$PORT --user="dolt" --password=$DOLT_CLI_PASSWORD --socket "dolt.$PORT.sock" &
dolt sql -q "create user dolt@'%' identified by '$DOLT_CLI_PASSWORD'; grant all on *.* to dolt@'%'"
dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" &
SERVER_PID=$!

# Also, wait_for_connection code is pulled in here and replaced with a use of `dolt sql` instead. This
Expand All @@ -33,7 +34,7 @@ setup() {
end_time=$((SECONDS+($timeout/1000)))

while [ $SECONDS -lt $end_time ]; do
run dolt --no-tls --host localhost --port $PORT -u "dolt" sql -q "SELECT 1;"
run dolt --no-tls --host localhost --port $PORT sql -q "SELECT 1;"
if [ $status -eq 0 ]; then
echo "Connected successfully!"
break
Expand Down
6 changes: 3 additions & 3 deletions integration-tests/bats/events.bats
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ make_test_repo_and_start_server() {
export DOLT_EVENT_SCHEDULER_PERIOD=1
start_sql_server

dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db information_schema sql -q "CREATE DATABASE repo1;"
dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "CREATE TABLE totals (id int PRIMARY KEY AUTO_INCREMENT, int_col int);"
dolt -u dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "call dolt_commit('-Am', 'creating table');"
dolt --port $PORT --host 0.0.0.0 --no-tls --use-db information_schema sql -q "CREATE DATABASE repo1;"
dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "CREATE TABLE totals (id int PRIMARY KEY AUTO_INCREMENT, int_col int);"
dolt --port $PORT --host 0.0.0.0 --no-tls --use-db repo1 sql -q "call dolt_commit('-Am', 'creating table');"
}

setup() {
Expand Down
19 changes: 8 additions & 11 deletions integration-tests/bats/helper/query-server-common.bash
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ SERVER_PID=""
DEFAULT_DB=""

# wait_for_connection(<PORT>, <TIMEOUT IN MS>) attempts to connect to the sql-server at the specified
# port on localhost, using the $SQL_USER (or 'dolt' if unspecified) as the user name, and trying once
# port on localhost, using $SQL_USER (or 'root' if unspecified) as the user name, and trying once
# per second until the millisecond timeout is reached. If a connection is successfully established,
# this function returns 0. If a connection was not able to be established within the timeout period,
# this function returns 1.
Expand All @@ -17,7 +17,7 @@ wait_for_connection() {
echo "Running in AWS Lambda; increasing timeout to: $timeout"
fi

user=${SQL_USER:-dolt}
user=${SQL_USER:-root}
end_time=$((SECONDS+($timeout/1000)))

while [ $SECONDS -lt $end_time ]; do
Expand All @@ -40,15 +40,15 @@ start_sql_server() {
if [[ $logFile ]]
then
if [ "$IS_WINDOWS" == true ]; then
dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" > $logFile 2>&1 &
dolt sql-server --host 0.0.0.0 --port=$PORT > $logFile 2>&1 &
else
dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" > $logFile 2>&1 &
dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" > $logFile 2>&1 &
fi
else
if [ "$IS_WINDOWS" == true ]; then
dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" &
dolt sql-server --host 0.0.0.0 --port=$PORT &
else
dolt sql-server --host 0.0.0.0 --port=$PORT --user "${SQL_USER:-dolt}" --socket "dolt.$PORT.sock" &
dolt sql-server --host 0.0.0.0 --port=$PORT --socket "dolt.$PORT.sock" &
fi
fi
echo db:$DEFAULT_DB logFile:$logFile PORT:$PORT CWD:$PWD
Expand Down Expand Up @@ -83,9 +83,6 @@ start_sql_server_with_config() {
echo "
log_level: debug

user:
name: dolt

listener:
host: 0.0.0.0
port: $PORT
Expand Down Expand Up @@ -134,9 +131,9 @@ start_multi_db_server() {
DEFAULT_DB="$1"
PORT=$( definePORT )
if [ "$IS_WINDOWS" == true ]; then
dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ &
dolt sql-server --host 0.0.0.0 --port=$PORT --data-dir ./ &
else
dolt sql-server --host 0.0.0.0 --port=$PORT --user dolt --data-dir ./ --socket "dolt.$PORT.sock" &
dolt sql-server --host 0.0.0.0 --port=$PORT --data-dir ./ --socket "dolt.$PORT.sock" &
fi
SERVER_PID=$!
wait_for_connection $PORT 8500
Expand Down
Loading
Loading