-
Notifications
You must be signed in to change notification settings - Fork 344
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
feat: add entrypoint script, which can create a user and related database, to the full Docker image #1722
base: master
Are you sure you want to change the base?
Conversation
thanks @tauu! @SimoneLazzaris could you please check it? |
I get the point of the PR but I have two issues:
|
Regarding the two issues:
What are your thoughts regarding the just described intention for using a temporary server for the setup process? |
If a command requires a logged in client, a pre run is set for the the major level (e.g. user, database) otherwise the command will just inherit the default from the root command.
Reusing the same command line struct for all command unifies the initialization of immuclient. Furthermore it enables the shell to reuse the same connection for all commands.
Most commands already return errors, only a few did not. This should be consistent over all commands.
As suggested some work to make immuadmin more scriptable has been added to the PR. This turned into a quite significant number of changes to make all commands consistently scriptable. The major changes made are as follows.
In addition a few things had to be adjusted in immuadmin for being able to implement the shell:
For the new flags as well as the new connection handling, unit tests were created and included. Most of the new tests are in commandline_test.go and verify that the new flags work as intended. As a lot of changes were made through out the immuadmin code, the already existing tests for immuadmin were reenabled. A lot of them seem to have been commented out. To reenable them, the test setup has been changed and unified to create a test immudb server and a commandline struct the same way for each test. Apart from changing the setup part of the tests, no functional part was changed. Currently all tests pass. The initial entrypoint script was also adjusted to use immuadmin using the newly introduced features and simplifies the script. @SimoneLazzaris is this in line with your suggestion to make immuadmin more scriptable? I am looking forward to a review of the PR and apologise in advance for the rathe big number of changed lines. Getting immuadmin consistently scriptable required more changes than expected. 😅 |
May I inquire about the status of this PR? Is there something holding it or can I do something to facilitate handling it? :) |
Hi @tauu , we'll start the review of newer changes tomorrow cc: @SimoneLazzaris |
How is the review going? Can I be of any assistance? |
This PR adds an entrypoint script to the full container image. This script creates a database and a user which is granted readwrite access to this database. The intention of this change is to facilitate deployments. It allows to define a user without admin priveleges to be used by other services directly in e.g. a docker-compose.yml oder a helm values.yml file and create it automatically. Currently only an admin user is available during the deployment, but using an admin user for services accessing the database is not a preferred option.
The credentials of the to be crated user as well as the name of the database can be set using the environment variables:
Thereby IMMUDB_PASSWORD_FILE specifies a file that contains the password.