Add Systemd Service for Deployment#314
Conversation
Signed-off-by: bupd <bupdprasanth@gmail.com>
📝 WalkthroughWalkthroughAdds comprehensive systemd integration for Harbor Satellite including hardened service unit files, drop-in configuration templates for SPIFFE authentication and CRI mirroring, environment variable examples, a detailed production-focused README, and an automated installer script for non-root service deployment. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codacy's Analysis Summary50 new issues (≤ 0 issue) Review Pull Request in Codacy →
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@deploy/systemd/examples/satellite.env.example`:
- Around line 42-47: Update the comment block in
deploy/systemd/examples/satellite.env.example that references the drop-in
filename: instead of saying "Create a 30-mirrors.conf drop-in" update the text
and the ExecStart example to reference the actual example filenames (e.g.,
30-mirrors-containerd.conf, 30-mirrors-docker.conf, 30-mirrors-podman.conf) so
the note and the ExecStart example match the provided drop-in files and avoid
confusion when following the instructions.
In `@deploy/systemd/README.md`:
- Around line 578-583: The added StartLimitBurst and StartLimitIntervalSec
directives were placed under the [Service] section but belong in the [Unit]
section for systemd >=230; move the two directives (StartLimitBurst,
StartLimitIntervalSec) from the [Service] block into the [Unit] block in the
README's example (matching how they appear in harbor-satellite.service) so
drop-in snippets created with systemctl edit use the correct section.
🧹 Nitpick comments (3)
deploy/systemd/install-satellite.sh (2)
82-90: Config directory permissions preventharbor-satellitegroup access to the env file.Line 84-85: the directory is
chown root:rootandchmod 750, so only root (and root-group members) can traverse it. Line 123 installs the env file asroot:harbor-satellitewith640, implying the service group should be able to read it. While systemd readsEnvironmentFileas PID 1 (root) so the service itself isn't affected, this makes manual debugging as the service user impossible (e.g.,sudo -u harbor-satellite cat /etc/harbor-satellite/satellite.env).Consider
chown root:$SERVICE_GROUPfor the directory as well:Proposed fix
## Config directory (root owned, group readable by service) mkdir -p "$INSTALL_CONFIG_DIR" - chown root:root "$INSTALL_CONFIG_DIR" + chown root:"$SERVICE_GROUP" "$INSTALL_CONFIG_DIR" chmod 750 "$INSTALL_CONFIG_DIR"
103-112: Installer only installs the single-instance service file; the template unit is omitted.The README documents multi-instance deployment via
harbor-satellite@.service, but the installer only copiesharbor-satellite.service. Users who want multi-instance need to manually install the template unit. Consider adding an optional step or a flag (e.g.,--multi-instance) to also installharbor-satellite@.service.deploy/systemd/README.md (1)
49-49: Codacy flags multiple Markdown list formatting issues.Static analysis reports many instances of "Lists should be surrounded by blank lines" throughout this file (e.g., lines 49, 129, 165, 189, 266, etc.). These are CommonMark style nits where numbered list items adjacent to code blocks lack surrounding blank lines. Consider adding blank lines around list items that precede or follow fenced code blocks for stricter Markdown compliance, or suppress these rules if the project doesn't enforce them.
Also applies to: 129-129, 165-177
| ## NOTE: CRI Mirroring Configuration | ||
| ## The --mirrors flag is NOT supported via environment variables | ||
| ## To enable container runtime mirroring: | ||
| ## 1. Install the 20-cri-mirroring.conf drop-in (see README) | ||
| ## 2. Create a 30-mirrors.conf drop-in to override ExecStart with --mirrors flags | ||
| ## Example: ExecStart=/opt/harbor-satellite/satellite --mirrors=containerd:docker.io,quay.io |
There was a problem hiding this comment.
Minor: drop-in filename in comment doesn't match actual example filenames.
Line 46 references 30-mirrors.conf, but the actual example files in this PR are named 30-mirrors-containerd.conf, 30-mirrors-docker.conf, and 30-mirrors-podman.conf. Consider updating the reference to avoid confusion.
Suggested fix
-## 2. Create a 30-mirrors.conf drop-in to override ExecStart with --mirrors flags
-## Example: ExecStart=/opt/harbor-satellite/satellite --mirrors=containerd:docker.io,quay.io
+## 2. Install a 30-mirrors-<runtime>.conf drop-in to override ExecStart with --mirrors flags
+## See examples: 30-mirrors-containerd.conf, 30-mirrors-docker.conf, 30-mirrors-podman.conf📝 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.
| ## NOTE: CRI Mirroring Configuration | |
| ## The --mirrors flag is NOT supported via environment variables | |
| ## To enable container runtime mirroring: | |
| ## 1. Install the 20-cri-mirroring.conf drop-in (see README) | |
| ## 2. Create a 30-mirrors.conf drop-in to override ExecStart with --mirrors flags | |
| ## Example: ExecStart=/opt/harbor-satellite/satellite --mirrors=containerd:docker.io,quay.io | |
| ## NOTE: CRI Mirroring Configuration | |
| ## The --mirrors flag is NOT supported via environment variables | |
| ## To enable container runtime mirroring: | |
| ## 1. Install the 20-cri-mirroring.conf drop-in (see README) | |
| ## 2. Install a 30-mirrors-<runtime>.conf drop-in to override ExecStart with --mirrors flags | |
| ## See examples: 30-mirrors-containerd.conf, 30-mirrors-docker.conf, 30-mirrors-podman.conf |
🤖 Prompt for AI Agents
In `@deploy/systemd/examples/satellite.env.example` around lines 42 - 47, Update
the comment block in deploy/systemd/examples/satellite.env.example that
references the drop-in filename: instead of saying "Create a 30-mirrors.conf
drop-in" update the text and the ExecStart example to reference the actual
example filenames (e.g., 30-mirrors-containerd.conf, 30-mirrors-docker.conf,
30-mirrors-podman.conf) so the note and the ExecStart example match the provided
drop-in files and avoid confusion when following the instructions.
| Add: | ||
| ```ini | ||
| [Service] | ||
| StartLimitBurst=10 | ||
| StartLimitIntervalSec=600 | ||
| ``` |
There was a problem hiding this comment.
StartLimitBurst/StartLimitIntervalSec should be under [Unit], not [Service].
The base service file correctly places these in [Unit] (lines 6-7 of harbor-satellite.service). In systemd 230+, these directives belong in [Unit]. While systemd may accept them in [Service] for backward compat, a drop-in created via systemctl edit should use the correct section to be consistent and avoid ambiguity.
Proposed fix
-[Service]
+[Unit]
StartLimitBurst=10
StartLimitIntervalSec=600🤖 Prompt for AI Agents
In `@deploy/systemd/README.md` around lines 578 - 583, The added StartLimitBurst
and StartLimitIntervalSec directives were placed under the [Service] section but
belong in the [Unit] section for systemd >=230; move the two directives
(StartLimitBurst, StartLimitIntervalSec) from the [Service] block into the
[Unit] block in the README's example (matching how they appear in
harbor-satellite.service) so drop-in snippets created with systemctl edit use
the correct section.
Description
Production-ready systemd service for Harbor Satellite with security hardening, multiple authentication methods, and optional container runtime integration.
Changes
Additional context
none
Summary by cubic
Adds a production‑ready systemd deployment for Harbor Satellite with strict sandboxing, multi‑instance support, and optional CRI mirroring. Ships an install script, hardened units, auth env templates, and complete docs for setup and operations.
New Features
Migration
Written for commit d414989. Summary will update on new commits.
Summary by CodeRabbit