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

[BUG] Admin password getting lost after pod restart #417

Closed
git-hyagi opened this issue Jan 17, 2023 · 6 comments · Fixed by #543
Closed

[BUG] Admin password getting lost after pod restart #417

git-hyagi opened this issue Jan 17, 2023 · 6 comments · Fixed by #543
Labels
bug Something isn't working prio-list

Comments

@git-hyagi
Copy link
Contributor

Describe the bug
The Pulp admin password from admin-password secret is overwriting the password defined through Pulp API.

To Reproduce
Steps to reproduce the behavior:
Modify the admin password through Pulp API.
Delete Pulp pods (didn't test yet, but maybe just reprovisioning pulp-api pods would be enough).
Try to use the modified password and it will not work, but the "old" stored in the secret will.

Expected behavior
The password defined in the admin-password secret should be used only as a default password. As soon as a user changed it, the secret should not be checked anymore.

Additional context
In a quick chat with @dkliban++ we identified a possible fix:

Before the operator set the password, we need to:
* if `password_is_set: false`
  * run a query in the database to check if the password is still null:
  > select password from auth_user where username='admin';

  * if the password field (from the last query) is null, set it with the default value from secret
  * set a status field `password_is_set: true` in Pulp CR (this will be used to control if the password has already been set)
* if `password_is_set: true` do nothing 

Notes
We will probably need to also fix this in ansible version.
Ref https://issues.redhat.com/browse/AAP-8344

@git-hyagi git-hyagi added bug Something isn't working prio-list labels Jan 17, 2023
@git-hyagi
Copy link
Contributor Author

@dkliban dkliban transferred this issue from pulp/pulp-operator Jan 19, 2023
@dkliban
Copy link
Member

dkliban commented Jan 19, 2023

Thank you @git-hyagi for pointing out that this is an issue in the image itself[0].

[0] https://github.com/pulp/pulp-oci-images/blob/latest/images/assets/pulp-api#L19-L27

@dkliban
Copy link
Member

dkliban commented Jan 24, 2023

It sounds like we need to write a file 'password-has-been-set' somewhere that is mounted into the container. Then we should check for the existence of this file and not reset the password when it is present.

@mikedep333
Copy link
Member

Consider (but you're not required to implement) the example of the Nexus container.

It sets the password to a random default, and let's you view the password.

If you set the password, it gets written to a file, and it prevents the random default from being set.

@decko decko removed their assignment Apr 11, 2023
@mdellweg
Copy link
Member

mdellweg commented Sep 1, 2023

In the s6 images we only set the admin password (from $PULP_DEFAULT_ADMIN_PASSWORD) if there is no admin account yet. Can we just do the same here?
How much backwards compatibility do we need to maintain?

@git-hyagi
Copy link
Contributor Author

git-hyagi commented Sep 6, 2023

For the golang version of pulp-operator, I believe this is not needed anymore (but we still need to decide how to proceed with Galaxy/AH/Ansible version).
If we consider that the operator should be the "source of truth" for Pulp deployment, then I think it is fine to let the admin password in the Secret. One example where this can be helpful is for clusters with vault/eso integration. Modifying the password in vault would update it in Pulp without having to manually execute pulpcore-manager reset-admin-password or make a Pulp API request.

To avoid having to reprovision an API pod to re-run the pulpcore-manager reset-admin-password, we added a watcher in the admin-pwd-secret and, whenever it is modified, the operator will create a k8s job to update the password: https://docs.pulpproject.org/pulp_operator/configuring/reset_admin_pwd/

mdellweg added a commit to mdellweg/pulp-oci-images that referenced this issue Sep 12, 2023
The pulp-api script in pulp-minimal now interprets the
PULP_DEFAULT_ADMIN_PASSWORD environment variable in the same way as the
s6 variant.

fixes pulp#417
mdellweg added a commit to mdellweg/pulp-oci-images that referenced this issue Sep 19, 2023
The pulp-api script in pulp-minimal now interprets the
PULP_DEFAULT_ADMIN_PASSWORD environment variable in the same way as the
s6 variant.

fixes pulp#417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working prio-list
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants