search

sclorg / mysql-container

MySQL container images based on Red Hat Software Collections and intended for OpenShift and general usage. Users can choose between Red Hat Enterprise Linux, Fedora, and CentOS based images.
http://softwarecollections.org
Apache License 2.0
128 stars 201 forks source link

Fixes BZ#1653215 Operation ALTER USER failed #269

Closed phracek closed 4 years ago

phracek commented 4 years ago

This issue fixes BZ#1653215.

This commit fixes problem formatting MySQL commands in 50-passwd-change.sh script. Correct BASH syntax as an input to MySQL command is used.

Signed-off-by: Petr "Stone" Hracek phracek@redhat.com

centos-ci commented 4 years ago

Can one of the admins verify this patch?

centos-ci commented 4 years ago

Can one of the admins verify this patch?

centos-ci commented 4 years ago

Can one of the admins verify this patch?

rhscl-bot commented 4 years ago

Can one of the admins verify this patch?

rhscl-bot commented 4 years ago

Can one of the admins verify this patch?

rhscl-bot commented 4 years ago

Can one of the admins verify this patch?

hhorak commented 4 years ago

[test]

hhorak commented 4 years ago

Petr, using <<'EOF' instead of <<EOSQL makes the variable in the heredoc not expanded, why do you think this should fix the issue?

phracek commented 4 years ago

I got inspired by https://github.com/mysql/mysql-docker/commit/13507953098b9cb707418a216ec5ac9d84326221 and https://github.com/sclorg/postgresql-container/blob/master/src/root/usr/share/container-scripts/postgresql/start/set_passwords.sh#L14 but did a copy-paste error. The next commit will fix it.

hhorak commented 4 years ago

[test]

phracek commented 4 years ago

In the original 50-password-change.sh file was used <<EOSQL instead of <<-EOSQL. If the <<-EOSQL is used then all leading tab characters are stripped from input lines. I have rewritten it a bit so each line starts at the beginning of the line.

hhorak commented 4 years ago

I still don't understand why you think it is a problem with syntax. How I understand it, the problem ERROR 1396 (HY000) is that the newuser doesn't exist, but the script is trying to set the password for it.

If I'm right, then it is still not clear to me how the startup script should behave in this case -- we can either return an error and fail starting the container. Or we can create the specified user. I'd probably incline for the second option, creating the user if user specifies it, and write the action of creating the user to the log, so we have an entry in the container log what was done. This option has one disadvantage -- people can create a new user by mistake.

phracek commented 4 years ago

I have solved the issue now. The output of MySQL SELECT command is stored into environment variable. I have executed several local tests whether the functionality works properly. In case the user exists in mysql.user the database then the password is updated. In case the user does not exist in mysql.user the database then in container log can be seen the message, that the user does not exist.

phracek commented 4 years ago

[test]

phracek commented 4 years ago

[test-openshift]

pkubatrh commented 4 years ago

[test-openshift]

hhorak commented 4 years ago

Thanks a lot for the PR. I've changed the check section and used SQL directly to see whether the user exists. Also a regression test is added.

A similar PR for mariadb available at https://github.com/sclorg/mariadb-container/pull/121

hhorak commented 4 years ago

[test]