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

Conversation

fulghum
Copy link
Contributor

@fulghum fulghum commented Jan 30, 2025

The --user and --password options to sql-server (whether specified on the command line or in a configuration file) allow users to specify an ephemeral superuser to use for the life of the sql-server process. This is a convenient way to temporarily create a superuser, but has several edge cases that can cause unexpected behavior.

This change removes support for the --user and --password options and instead customers should now explicitly create that user using standard SQL statements for managing users, such as:

CREATE USER myUser@'%' IDENTIFIED BY 'myPassw0rd';
GRANT ALL ON *.* TO myUser@'%';

If user or password is specified in a config file, the sql-server will still startup, but will log a warning. If --user or --password is specified as a CLI argument the sql-server will error out with an error message explaining the change. The reason for the difference in handling is that it may be harder for customers to update config files.

@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch from 1ac00cb to 85deca0 Compare January 30, 2025 01:26
@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch from 85deca0 to 1d213cf Compare January 30, 2025 18:01
@dolthub dolthub deleted a comment from coffeegoddd Jan 30, 2025
@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch 2 times, most recently from 3e2418d to 7278643 Compare January 30, 2025 20:23
@dolthub dolthub deleted a comment from coffeegoddd Jan 30, 2025
@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch from 7278643 to ae8ed37 Compare January 30, 2025 22:09
@dolthub dolthub deleted a comment from coffeegoddd Jan 30, 2025
@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch 3 times, most recently from 0946c3a to 3adf49b Compare January 31, 2025 19:40
@dolthub dolthub deleted a comment from coffeegoddd Jan 31, 2025
@dolthub dolthub deleted a comment from coffeegoddd Jan 31, 2025
@dolthub dolthub deleted a comment from coffeegoddd Jan 31, 2025
@fulghum fulghum marked this pull request as ready for review January 31, 2025 20:47
@fulghum fulghum requested a review from macneale4 January 31, 2025 20:47
Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

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

couple comment updates. Feel free to ship without another read

go/cmd/dolt/commands/sqlserver/sqlserver.go Show resolved Hide resolved
integration-tests/bats/sql-server.bats Outdated Show resolved Hide resolved
@fulghum fulghum force-pushed the fulghum/rm-sql-server-user branch 2 times, most recently from 72c5fcc to 06bc291 Compare February 3, 2025 19:42
@coffeegoddd
Copy link
Contributor

@fulghum DOLT

comparing_percentages
100.000000 to 100.000000
version result total
9dd401c ok 5937457
version total_tests
9dd401c 5937457
correctness_percentage
100.0

@dolthub dolthub deleted a comment from coffeegoddd Feb 3, 2025
@fulghum fulghum merged commit d766206 into main Feb 5, 2025
21 checks passed
@fulghum fulghum deleted the fulghum/rm-sql-server-user branch February 5, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants