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

chore(Dockerfile): Simplify testssl user creation #2312

Merged

Conversation

polarathene
Copy link
Contributor

@polarathene polarathene commented Feb 2, 2023

Extracted from #2305 (specifically this commit)

Create a testssl user (and group) with no password (-D) and default their shell to bash (-s):

  • A group will implicitly be created with the same value as the user. addgroup testssl and -G testssl are not needed.
  • Gecos data (-g "testssl user") doesn't appear relevant to the project to be required? The default gecos value (Linux User,,,) should be fine.

polarathene and others added 2 commits February 2, 2023 14:07
Create `testssl` user (_and group_) with no password (`-D`) and default their shell to bash (`-s`):
- A group will implicitly be created with the same value as the user. `addgroup testssl` and `-G testssl` are not needed.
- Gecos data (`-g "testssl user"`) doesn't appear relevant to the project to be required? The default gecos value (`Linux User,,,`) should be fine.
@drwetter drwetter merged commit 1ee21b7 into drwetter:3.1dev Feb 7, 2023
@drwetter
Copy link
Owner

drwetter commented Feb 7, 2023

Thanks!

Copy link
Contributor Author

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

Looks like the merge caused a minor regression 😅

ln -s /home/testssl/testssl.sh /usr/local/bin/
adduser -D -s /bin/bash testssl && \
ln -s /home/testssl/testssl.sh /usr/local/bin/ && \
mkdir -m 755 -p /home/testssl/etc /home/testssl/bin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a heads up @drwetter when this PR was raised, I think it was separate from the mkdir change, which you had merged earlier.

When you updated the branch here before merging, it looks like it didn't diff well, and accidentally brought back the mkdir line? (the previous PR change is actually missing from the files history now).

You may want to remove the line again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

drwetter added a commit that referenced this pull request Feb 7, 2023
@drwetter
Copy link
Owner

drwetter commented Feb 7, 2023

Thanks, PR pending/merged

I manually had to resolve the conflict and I just removed w/o thinking what github labeled as conflict.

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.

None yet

2 participants