Create systemd file so the app can run as a Linux system daemon. #2737 #2754
Conversation
|
Warning Rate limit exceeded@Rajveerchoubisa has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes introduce a new systemd service file for the Talawa API, enabling it to be managed as a system service on Linux. The service file includes configuration for the service's operational parameters, such as dependencies, execution commands, and environment variables. It is set to run under specific user and group permissions and is designed to start automatically at boot. This implementation aligns with the requirements for running the application as a Linux system daemon. Changes
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
examples/linux/systemd/systemd-talawa-api.service (1)
1-20: Enhance setup instructions for production deploymentThe setup instructions should include additional critical steps for secure production deployment:
- Directory permissions and ownership:
- Set correct permissions for
/var/run/talawa-api- Set correct permissions for log directory
- SELinux context (for RedHat variants)
- Log rotation configuration
- Specify the name of the configuration file referenced in step 0
Would you like me to provide detailed instructions for these additional setup steps?
|
|
||
| # READ ALL STEPS BEFORE PROCEEDING | ||
| # | ||
| # 0) Change the daemon_directory setting in your configuration file to | ||
| # /var/run/talawa-api | ||
| # 1) Copy this file to one of these directories depending on your Linux version | ||
| # i. RedHat variants: /usr/lib/systemd/system/ | ||
| # ii. Debian/Ubuntu variants: /lib/systemd/system/ | ||
| # 2) Edit the CODEROOT path to be the full path of the Talawa API's root directory | ||
| # 3) Edit the TALAWA_API_CONFIGDIR path to be the full path of the Talawa API's configuration directory | ||
| # This defaults to /etc/ directory of the Talawa API codebase | ||
| # 4) Edit the User and Group to match the POSIX user you want the daemon | ||
| # to run as. | ||
| # 5) Run the command "sudo systemctl daemon-reload". This needs to be run only once | ||
| # 6) Run the command "sudo systemctl start talawa-api.service" to start | ||
| # 7) Run the command "sudo systemctl stop talawa-api.service" to stop | ||
| # 8) Run the command "sudo systemctl restart talawa-api.service" to restart | ||
| # 9) Run the command "sudo systemctl enable talawa-api.service" to make | ||
| # talawa-api start automatically on boot | ||
|
|
||
| [Unit] | ||
| Description=Talawa API Service | ||
| After=network.target | ||
|
|
||
| [Service] | ||
| User=talawa | ||
| Group=talawa | ||
| Environment=CODEROOT=/home/talawa/talawa-api | ||
| Environment=TALAWA_API_CONFIGDIR=/etc/talawa | ||
| Environment=VIRTUAL_ENV=$CODEROOT/venv/bin | ||
| Environment=PATH=$VIRTUAL_ENV:$PATH | ||
| Environment=START=--start "STOP=--stop --force" "RESTART=--restart --force" | ||
|
|
||
| # Command to start Talawa API | ||
| ExecStart=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $START' | ||
|
|
||
| # Command to stop Talawa API | ||
| ExecStop=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $STOP' | ||
|
|
||
| # Command to restart Talawa API | ||
| ExecReload=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $RESTART' | ||
|
|
||
| RemainAfterExit=yes | ||
| GuessMainPID=yes | ||
| Type=forking | ||
| RuntimeDirectory=talawa | ||
|
|
||
| [Install] | ||
| WantedBy=multi-user.target |
There was a problem hiding this comment.
💡 Codebase verification
Add recommended systemd service hardening directives
The service file is missing several important systemd directives that would improve service reliability and security:
- Add
Restart=on-failureto automatically restart the service on failures - Add
ProtectSystem=fullto make the /usr and /boot directories read-only - Add
NoNewPrivileges=yesto prevent privilege escalation - Consider adding other hardening options like:
ProtectHome=yesRestartSec=5PrivateTmp=yes
🔗 Analysis chain
Verify systemd service file syntax
Let's verify the service file syntax and check for common configuration patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify systemd service file syntax and common patterns
# Check syntax
systemd-analyze verify examples/linux/systemd/systemd-talawa-api.service 2>&1 || true
# Check for common systemd service patterns
rg -l "WantedBy=multi-user.target" | while read -r file; do
echo "Analyzing $file for common systemd patterns..."
rg "Restart=" "$file" || echo "Missing Restart directive in $file"
rg "ProtectSystem=" "$file" || echo "Missing ProtectSystem directive in $file"
rg "NoNewPrivileges=" "$file" || echo "Missing NoNewPrivileges directive in $file"
done
Length of output: 1178
| [Unit] | ||
| Description=Talawa API Service | ||
| After=network.target | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential service dependencies
The Unit section should include additional dependencies and documentation:
[Unit]
Description=Talawa API Service
+Documentation=https://github.com/talawa-api/
After=network.target
+After=postgresql.service redis.service
+Requires=postgresql.service
+Wants=redis.serviceThis ensures proper service startup order and documents the service's dependencies.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [Unit] | |
| Description=Talawa API Service | |
| After=network.target | |
| [Unit] | |
| Description=Talawa API Service | |
| Documentation=https://github.com/talawa-api/ | |
| After=network.target | |
| After=postgresql.service redis.service | |
| Requires=postgresql.service | |
| Wants=redis.service | |
| # Command to start Talawa API | ||
| ExecStart=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $START' | ||
|
|
||
| # Command to stop Talawa API | ||
| ExecStop=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $STOP' | ||
|
|
||
| # Command to restart Talawa API | ||
| ExecReload=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $RESTART' | ||
|
|
There was a problem hiding this comment.
Improve service execution security and reliability
The current ExecStart/Stop/Reload commands use shell expansion which could be unsafe. Also, missing important security and reliability settings.
- Simplify execution commands:
-ExecStart=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $START'
-ExecStop=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $STOP'
-ExecReload=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $RESTART'
+ExecStart=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${START}
+ExecStop=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${STOP}
+ExecReload=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${RESTART}- Add security and reliability directives:
Type=forking
RuntimeDirectory=talawa
+# Restart policy
+Restart=always
+RestartSec=3
+
+# Security settings
+NoNewPrivileges=yes
+PrivateTmp=yes
+ProtectSystem=full
+ProtectHome=read-only
+
+# Resource limits
+LimitNOFILE=65535
+LimitNPROC=4096📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Command to start Talawa API | |
| ExecStart=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $START' | |
| # Command to stop Talawa API | |
| ExecStop=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $STOP' | |
| # Command to restart Talawa API | |
| ExecReload=/bin/bash -c '${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py $RESTART' | |
| # Command to start Talawa API | |
| ExecStart=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${START} | |
| # Command to stop Talawa API | |
| ExecStop=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${STOP} | |
| # Command to restart Talawa API | |
| ExecReload=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${RESTART} |
| [Service] | ||
| User=talawa | ||
| Group=talawa | ||
| Environment=CODEROOT=/home/talawa/talawa-api | ||
| Environment=TALAWA_API_CONFIGDIR=/etc/talawa | ||
| Environment=VIRTUAL_ENV=$CODEROOT/venv/bin | ||
| Environment=PATH=$VIRTUAL_ENV:$PATH | ||
| Environment=START=--start "STOP=--stop --force" "RESTART=--restart --force" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use EnvironmentFile for better security and maintainability
Instead of hardcoding environment variables, use an EnvironmentFile:
[Service]
User=talawa
Group=talawa
-Environment=CODEROOT=/home/talawa/talawa-api
-Environment=TALAWA_API_CONFIGDIR=/etc/talawa
-Environment=VIRTUAL_ENV=$CODEROOT/venv/bin
-Environment=PATH=$VIRTUAL_ENV:$PATH
-Environment=START=--start "STOP=--stop --force" "RESTART=--restart --force"
+EnvironmentFile=/etc/talawa/talawa-api.envCreate /etc/talawa/talawa-api.env with:
CODEROOT=/home/talawa/talawa-api
TALAWA_API_CONFIGDIR=/etc/talawa
VIRTUAL_ENV=${CODEROOT}/venv/bin
PATH=${VIRTUAL_ENV}:${PATH}
START=--start
STOP=--stop --force
RESTART=--restart --force
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
examples/linux/systemd/systemd-talawa-api.service(1 hunks)examples/linux/systemd/talawa-api.env(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/linux/systemd/talawa-api.env
🔇 Additional comments (5)
examples/linux/systemd/systemd-talawa-api.service (5)
21-29: LGTM! Dependencies and documentation are well defined
The Unit section correctly specifies service dependencies and includes proper documentation links.
41-44: LGTM! Execution commands are properly defined
The ExecStart, ExecStop, and ExecReload commands are correctly specified without shell expansion.
50-62: LGTM! Service hardening is well implemented
The restart policy, security settings, and resource limits are properly configured with recommended hardening directives.
64-65: LGTM! Install section is correct
The service is properly configured to be enabled in multi-user.target.
34-39:
Fix environment configuration issues
- The environment configuration has redundant and potentially conflicting settings:
- Both inline Environment variables and EnvironmentFile are used
- Shell expansions in Environment variables could be unsafe
- The EnvironmentFile path is non-standard
Apply these changes:
-Environment=CODEROOT=/home/talawa/talawa-api
-Environment=TALAWA_API_CONFIGDIR=/etc/talawa
-Environment=VIRTUAL_ENV=$CODEROOT/venv/bin
-Environment=PATH=$VIRTUAL_ENV:$PATH
-Environment=START=--start "STOP=--stop --force" "RESTART=--restart --force"
-EnvironmentFile=/etc/examples/linux/systemd/talawa-api.env
+EnvironmentFile=/etc/talawa/talawa-api.envCreate /etc/talawa/talawa-api.env with:
CODEROOT=/home/talawa/talawa-api
TALAWA_API_CONFIGDIR=/etc/talawa
VIRTUAL_ENV=/home/talawa/talawa-api/venv/bin
PATH=${VIRTUAL_ENV}:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
START=--start
STOP=--stop --force
RESTART=--restart --force
Likely invalid or redundant comment.
| ExecStart=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${START} | ||
| ExecStop=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${STOP} | ||
| ExecReload=${VIRTUAL_ENV}/python3 ${CODEROOT}/main.py ${RESTART} |
There was a problem hiding this comment.
💡 Codebase verification
The systemd service file references a non-existent main.py script
The verification reveals that main.py does not exist in the repository, yet it's referenced in the systemd service file. This script is expected to handle start, stop, and restart operations but is missing from the codebase.
examples/linux/systemd/systemd-talawa-api.servicereferences a non-existentmain.py- The script is expected to support command-line arguments
${START},${STOP}, and${RESTART}
🔗 Analysis chain
Verify main.py supports the specified command-line arguments
Let's verify that the main.py script exists and supports the specified command-line arguments (--start, --stop, --restart).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify main.py exists and supports the required command-line arguments
# Check if main.py exists
echo "Checking for main.py..."
if ! fd "main.py$" ; then
echo "Error: main.py not found"
exit 1
fi
# Check for command-line argument handling
echo "Checking command-line argument handling..."
rg -A 5 "argparse|ArgumentParser|add_argument" "main.py" || echo "Warning: No argument parsing found in main.py"
# Look for start/stop/restart handling
echo "Checking start/stop/restart handling..."
rg -i "def (start|stop|restart)" "main.py" || echo "Warning: No start/stop/restart functions found in main.py"
Length of output: 678
| RemainAfterExit=yes | ||
| GuessMainPID=yes | ||
| Type=forking | ||
| RuntimeDirectory=talawa | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider service type setting
The Type=forking setting suggests the application daemonizes itself, which is not recommended for systemd services. Modern systemd services should use Type=exec or Type=simple and let systemd handle the daemonization.
RemainAfterExit=yes
GuessMainPID=yes
-Type=forking
+Type=simple
RuntimeDirectory=talawa📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RemainAfterExit=yes | |
| GuessMainPID=yes | |
| Type=forking | |
| RuntimeDirectory=talawa | |
| RemainAfterExit=yes | |
| GuessMainPID=yes | |
| Type=simple | |
| RuntimeDirectory=talawa | |
|
|
||
| # READ ALL STEPS BEFORE PROCEEDING | ||
| # | ||
| # 0) Change the daemon_directory setting in your configuration file to | ||
| # /var/run/talawa-api | ||
| # 1) Copy this file to one of these directories depending on your Linux version | ||
| # i. RedHat variants: /usr/lib/systemd/system/ | ||
| # ii. Debian/Ubuntu variants: /lib/systemd/system/ | ||
| # 2) Edit the CODEROOT path to be the full path of the Talawa API's root directory | ||
| # 3) Edit the TALAWA_API_CONFIGDIR path to be the full path of the Talawa API's configuration directory | ||
| # This defaults to /etc/ directory of the Talawa API codebase | ||
| # 4) Edit the User and Group to match the POSIX user you want the daemon | ||
| # to run as. | ||
| # 5) Run the command "sudo systemctl daemon-reload". This needs to be run only once | ||
| # 6) Run the command "sudo systemctl start talawa-api.service" to start | ||
| # 7) Run the command "sudo systemctl stop talawa-api.service" to stop | ||
| # 8) Run the command "sudo systemctl restart talawa-api.service" to restart | ||
| # 9) Run the command "sudo systemctl enable talawa-api.service" to make | ||
| # talawa-api start automatically on boot | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security context to setup instructions
The setup instructions should include additional security considerations:
- File permissions for configuration directory
- SELinux/AppArmor context if applicable
- Secure ownership of the runtime directory
Add these lines after step 4:
# 4) Edit the User and Group to match the POSIX user you want the daemon
# to run as.
+# 4a) Set appropriate permissions:
+# sudo chown -R talawa:talawa /etc/talawa
+# sudo chmod 750 /etc/talawa
+# 4b) If using SELinux, set the correct context:
+# sudo semanage fcontext -a -t bin_t "/home/talawa/talawa-api/main.py"
+# sudo restorecon -v /home/talawa/talawa-api/main.pyCommittable suggestion skipped: line range outside the PR's diff.
|
Please fix the failing tests and make sure code rabbit.ai approves the PR |
|
@Rajveerchoubisa you are failing the base branch check as you are pushing to main, create a new branch, make your changes and then push to develop |
|
Please submit your PRs against our Also if applicable, make sure your branch name is different from Closing. |
What kind of change does this PR introduce?
Issue Number:
Fixes #
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
systemd-talawa-api.servicefor automatic startup.Summary
This pull request introduces a
systemdservice file (systemd-talawa-api.service) to enable the Talawa API to run as a service on Linux systems. This allows the Talawa API to be easily managed using standardsystemctlcommands (such as start, stop, restart, and enable on boot).The service file sets up:
This change ensures that the Talawa API can be run as a background service in production environments, improving its manageability and availability.
Does this PR introduce a breaking change?
Other information
systemd-talawa-api.servicefile is configured for systems running RedHat/CentOS/Fedora or Debian/Ubuntu, based on the paths wheresystemdservices are typically stored.CODEROOTandTALAWA_API_CONFIGDIRpaths in the service file to match the actual directory paths on your system.Have you read the contributing guide?
Summary by CodeRabbit