Extract setup-permissions.sh from install script#226
Conversation
Move user, group, and directory permission setup into a separate reusable script so it can be called independently by RPM post-install or other packaging workflows.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #226 +/- ##
=======================================
Coverage 86.27% 86.27%
=======================================
Files 34 34
Lines 9331 9331
Branches 9331 9331
=======================================
Hits 8050 8050
Misses 1148 1148
Partials 133 133 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # Directory permissions | ||
| # | ||
|
|
||
| mkdir -p "${PROVIDER_DIR}" |
There was a problem hiding this comment.
What should we do if this fails? https://linux.die.net/man/2/mkdir
There was a problem hiding this comment.
The script has #!/bin/bash -e at the top, so any command failure (including mkdir) will cause the script to exit immediately with a non-zero status.
There was a problem hiding this comment.
That seems ok with me but it's a bit inconsistent with the other operations here succeeding if the resource already exists. I think it's fine from a functional standpoint though.
|
|
||
| groupadd -f "${PROVIDER_GROUP}" | ||
| groupadd -f "${TOKEN_GROUP}" | ||
| useradd -r -M -d "${PROVIDER_DIR}" -s /sbin/nologin -g "${PROVIDER_GROUP}" -G "${TOKEN_GROUP}" "${PROVIDER_USER}" || [ $? -eq 9 ] |
There was a problem hiding this comment.
The useradd ... || [ $? -eq 9 ] idempotency pattern only creates the user the first time. If the script is re-run, the user already exists (exit 9), and the -G "${TOKEN_GROUP}" supplementary group membership is silently NOT updated. If TOKEN_GROUP changes or the user predated this group, the membership won't be reconciled. Consider running usermod -aG "${TOKEN_GROUP}" "${PROVIDER_USER}" after the useradd to ensure group membership is correct on re-runs.
AI-generated, feedback may be incorrect
| chmod 755 "${PROVIDER_DIR}" | ||
| chown "${PROVIDER_USER}" "${PROVIDER_DIR}" | ||
|
|
||
| echo "Permissions setup complete." No newline at end of file |
There was a problem hiding this comment.
Missing trailing newline at end of file. Combined with echo -n-free output this is minor, but POSIX text files should end with a newline; some tools and linters will complain.
AI-generated, feedback may be incorrect
|
|
||
| groupadd -f "${PROVIDER_GROUP}" | ||
| groupadd -f "${TOKEN_GROUP}" | ||
| useradd -r -M -d "${PROVIDER_DIR}" -s /sbin/nologin -g "${PROVIDER_GROUP}" -G "${TOKEN_GROUP}" "${PROVIDER_USER}" || [ $? -eq 9 ] |
There was a problem hiding this comment.
We should keep the original explanatory comment.
Restore explanatory comment for useradd exit code 9 handling and add trailing newline.
3 participants
Move user, group, and directory permission setup into a separate reusable script so it can be called independently by RPM post-install or other packaging workflows.
Description
Why is this change being made?
What is changing?
Related Links
Testing
How was this tested?
When testing locally, provide testing artifact(s):
Reviewee Checklist
Update the checklist after submitting the PR
If not, why:
If not, why:
If not, why:
If not, why:
If not, why:
If not, why: code refactor. no integ test change is required.
If not, why:
If not, why:
If not, why: NA
If not, why: NA
Reviewer Checklist
All reviewers please ensure the following are true before reviewing:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.