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

fix(dokploy): exclude password column from application queries #645

Open
wants to merge 10 commits into
base: canary
Choose a base branch
from

Conversation

jmischler72
Copy link

No description provided.

Copy link
Contributor

@Siumauricio Siumauricio left a comment

Choose a reason for hiding this comment

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

I have a doubt here, what happens if we have already saved a password, and we re-save the docker provider?

I think it would clean the password and this would lead to every time we make a change we have to enter the password.

@jmischler72
Copy link
Author

I have a doubt here, what happens if we have already saved a password, and we re-save the docker provider?

I think it would clean the password and this would lead to every time we make a change we have to enter the password.

Yes you're right, it doesn't appear in the dashboard anymore but it stays in the database, maybe we should store a variable if the password has been set or maybe the password length and display stars in the dashboard

Copy link
Contributor

@Siumauricio Siumauricio left a comment

Choose a reason for hiding this comment

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

Did you make sure this works?

It doesn't work because the method that builds the docker needs the password and uses the method you modified.

This also breaks the types where the method you modified is used.

@jmischler72
Copy link
Author

Did you make sure this works?

It doesn't work because the method that builds the docker needs the password and uses the method you modified.

This also breaks the types where the method you modified is used.

I may overlooked this, i moved the code to hide the password in the router instead of the services and i adapted the data where the route is called

Copy link
Contributor

@Siumauricio Siumauricio left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@Siumauricio Siumauricio left a comment

Choose a reason for hiding this comment

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

Check the suggestions

@jmischler72
Copy link
Author

Check the suggestions

Done

@Siumauricio
Copy link
Contributor

fix the lint @jmischler72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants