Skip to content

Commit

Permalink
First pass on removing user and pass options for sql-server
Browse files Browse the repository at this point in the history
  • Loading branch information
fulghum committed Jan 30, 2025
1 parent caaa4bd commit 1ac00cb
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 246 deletions.
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
11 changes: 8 additions & 3 deletions go/cmd/dolt/commands/sqlserver/sqlserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const (
hostFlag = "host"
portFlag = "port"
skipRootUserInitialization = "skip-root-user-initialization"
passwordFlag = "password"
timeoutFlag = "timeout"
readonlyFlag = "readonly"
logLevelFlag = "loglevel"
Expand Down Expand Up @@ -163,9 +162,9 @@ 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()))
ap.SupportsString(commands.UserFlag, "u", "user", "This option is no longer supported. Instead, you can create users using CREATE USER and GRANT SQL statements.")
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 +234,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
4 changes: 0 additions & 4 deletions go/libraries/doltcore/servercfg/yaml_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,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
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

0 comments on commit 1ac00cb

Please sign in to comment.