-
Notifications
You must be signed in to change notification settings - Fork 275
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
[Decision Issue] User Experience for RPM/DEB distributions with default admin credential changes #3916
Comments
A bunch of though from my perspective. About being allowed to pass environment variables with sudo
I would expect users allowed to run So I would not consider the above an issue… but this lead to the next point: About passing environment variables at install time / on first startI really don't like this. It feels fragile, it's not something usual when installing a package. For less advanced users, it makes things harder (and as stated above, GUI "wrappers" may lack options to do so). I would not consider this as a viable option (neither at install time nor at first start). About stopping with an error at install timeI don't like this neither. Installing is basically about extracting a bunch of files and failing because some run-time requirement are not found is too early. Installation should finish successfully. About stopping with an error at first startThis feels acceptable. If a configuration file is missing or absent, a service will not start and display an appropriate error. If a required password was not provided, an error message telling how it must be seeded is fine. Interactively prompting the user can be a pain for configuration management tools, so I would not go that way. Service managers generally have a pre-start section to handle this (e.g. systemd has The secret can be read from a configuration file (allowing a configuration management system to install the package, setup the config — including the password — and start the service). Passing the password in the environment can also be an option here (but it means the clear-text secret is available from the context of the application, or from Avoiding interruption on first startIf a password is required but not available, generating one and printing it to stdout make sense if the user has to change it on first connection. Locating it can be a pain if there is a lot of output however, which may make this impractical. About the demo certificates
💯 This should definitely be a separate action: installing the package should not generate them, starting the service should not generate them. Also, if the user wants demo certificates and run a utility that configure them, it must generate the demo CA / service cert+key and admin user cert+key so that all demos do not share the same admin credentials by default. If these files are missing, the service should refuse to start and says that it could not find the server certificate and key (indicating where they are looked for), and tell that demo PKI can be generated using the command described above. SummaryIMHO, the least astonishment is provided by a plain configuration file containing the required secrets. The package can install a sample configuration file with commented examples so that the user has to tune the config before being able to start the service for the first time (e.g. the pre-start command check that an admin user has been properly configured). |
[Triage] Hi @derek-ho thank you for filing this issue. This seems well detailed and it is great to see you getting input from various people! Going to mark as triaged since the issue can be closed when a decision is reached etc. |
Path Forward for RPM/DEBDecisions to be made:Should we keep install demo script inside the spec or postinstall? OR make it its own thing?
Should we improve logs to indicate what the issue is and how to solve it?
Should we support opensearch setup via GUI for people without terminal access? If yes, how?
Are we supporting people access to
|
IMHO we cannot support password validation in the script - although we have regex that we can use, we also have an additional requirements of strength provided by a library: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java#L103, unless we convert that to a tool (which would be non-trivial), we can only check the existence and regex, but not guarantee that it would pass the password strength requirement overall. |
Agreed. To keep it uniform, let's only check if the variable is defined as part of pre-install. We can add a log if there are any errors. To add more logging and make it easier for end-users, we can also catch the error, if any, thrown by the demo install script , and print out a message redirecting users to check the install script logs. Note: This approach prereqs the user already has access to read the logs. |
Here's the updated proposal number 4: 4. Update the pre-install script (PROPOSED)This requires adding an extra check in the pre-install script to check whether the env variable is defined. In addition, the script should print clear messages on what can users do next. This change is small, and the installation will quit before the package is installed leaving a clear message for the user that the password needs to be supplied to spin-up a >=2.12 cluster. Pros:
Cons:
|
FInal Implementation
(
Tests:PR: |
Feel free to re-open if any more input required. |
Is your feature request related to a problem?
It was recently brought to our attention that the way we are expecting the initial admin password to be set (via env. variable) may be problematic for RPM/DEB distributions. As a refresher, we are expecting the initial admin password to be set via
OPENSEARCH_INITIAL_ADMIN_PASSWORD
variable on install for example:sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! dpkg -i opensearch.deb
(deb)sudo env OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123! yum install opensearch.rpm
(rpm)@peterzhuamazon brought up a few issues with this approach:
yum
, but but not sudo access toenv
(this would make them unable to run opensearch)Possible solutions (ranked from low to high effort):
The text was updated successfully, but these errors were encountered: