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

Add way to set root password by environnement. #1814

Merged
merged 1 commit into from May 16, 2019

Conversation

Projects
None yet
4 participants
@darnuria
Copy link
Contributor

commented May 14, 2019

Add a condition test to pass by environement a predetermined
root password for setting up.

Related work in documentation: https://framagit.org/framasoft/peertube/documentation/merge_requests/16

@ldidry

This comment has been minimized.

Copy link
Collaborator

commented May 15, 2019

Maybe you could name your env var differently to make its use more obvious, like PT_ROOT_PASSWORD? Otherwise, admins may not know if it’s DB password[1] or root account password.

[1] yeah, I know, it’s dumb because that password is in config file but still, it’s better to have obvious env var.

@rigelk
Copy link
Collaborator

left a comment

That's the right place to go 👍

much like what @ldidry said, I would rename the environment variable to something even more explicity. PT_INITIAL_ROOT_PASSWORD seems fitting, because:

  • we make explicit the scope (PT and not Postgres)
  • we make explicit this only applies at the initialization
  • we make explicit the user name
@darnuria

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@ALL: Thanks for your review! ☺

@ldidry I will make a change, PASSWORD was a "placeholder" name, is really better PT_INITIAL_ROOT_PASSWORD.

@rigelk : Thanks for the pointers to the documentation.

Is it a good idea to let run password validation on this?

Note: The validation is about pasword length https://github.com/Chocobozzz/PeerTube/blob/develop/server/helpers/custom-validators/users.ts#L10

@darnuria darnuria force-pushed the darnuria:fix/issues/189 branch 2 times, most recently from a6161f4 to 1a1e40f May 15, 2019

@darnuria darnuria changed the title [🔨wip] Add way to set root password by environnement. Add way to set root password by environnement. May 15, 2019

Show resolved Hide resolved support/doc/production.md Outdated
@rigelk
Copy link
Collaborator

left a comment

Apart from a typo, LGTM.

@darnuria darnuria force-pushed the darnuria:fix/issues/189 branch from 1a1e40f to 82614e7 May 15, 2019

@darnuria

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

update:

  • Fixed some typo.
  • Fixed style, removing useless semicolon
Add way to set root password by environment.
Add a condition test to pass by environment a predetermined
root password for setting up.

@darnuria darnuria force-pushed the darnuria:fix/issues/189 branch from 82614e7 to 597cb83 May 15, 2019

@Chocobozzz

This comment has been minimized.

Copy link
Owner

commented May 16, 2019

Thanks @darnuria

@Chocobozzz Chocobozzz merged commit 3daaa19 into Chocobozzz:develop May 16, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - client/package.json (Chocobozzz) No new issues
Details
security/snyk - package.json (Chocobozzz) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.