new Satellite provisioning method#256
Conversation
Declarative, idempotent alternative to the curl-pipe-bash install.sh path. Includes playbook, example inventory, ansible.cfg, and README.
📝 WalkthroughWalkthroughAdds an Ansible-based provisioning workspace for Companion Satellite: project config and ignore rules, an example inventory, a comprehensive README, and a large idempotent playbook that installs Node.js, fetches and installs satellite builds, configures systemd service and boot settings, and validates the service. Changes
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
satellite-provisioning/.gitignoresatellite-provisioning/README.mdsatellite-provisioning/ansible.cfgsatellite-provisioning/inventory.examplesatellite-provisioning/satellite.yml
| 1. Edit the ansible inventory with your satellite host(s) | ||
| ``` | ||
| cp inventory.example inventory | ||
| vim inventory | ||
| ``` | ||
|
|
||
| 2. Provision | ||
| ``` | ||
| cd satellite-provisioning | ||
| ansible-playbook satellite.yml | ||
| ``` |
There was a problem hiding this comment.
Quick-start snippets need consistent directory context (and fence language).
Line 11 assumes you are already in satellite-provisioning, while Line 17 changes into it. Also, Line 10 and Line 16 fences are missing a language tag (MD040).
Suggested patch
-1. Edit the ansible inventory with your satellite host(s)
-```
-cp inventory.example inventory
-vim inventory
-```
+1. Move into the provisioning directory and edit inventory
+```bash
+cd satellite-provisioning
+cp inventory.example inventory
+vim inventory
+```
2. Provision
-```
-cd satellite-provisioning
-ansible-playbook satellite.yml
-```
+```bash
+ansible-playbook satellite.yml
+```🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
| - name: Download Node.js tarball | ||
| ansible.builtin.get_url: | ||
| url: "{{ node_url }}" | ||
| dest: "{{ build_tmp }}/node.tar.xz" | ||
| mode: "0644" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the satellite.yml file and check its content around the flagged lines
find . -name "satellite.yml" -type f | head -1 | xargs -I {} sh -c 'echo "=== File: {} ===" && cat -n {} | sed -n "70,145p"'Repository: bitfocus/companion-satellite
Length of output: 3096
🏁 Script executed:
#!/bin/bash
# Search for checksum-related variables and patterns
rg -n "node_sha256|satellite_sha256|checksum:" . --type yaml -B2 -A2Repository: bitfocus/companion-satellite
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Look for where variables like node_url and satellite_url are defined
rg -n "node_url|satellite_url|node_version|satellite_version" . --type yaml | head -30Repository: bitfocus/companion-satellite
Length of output: 1467
Add checksum validation to tarball downloads.
The Node.js and satellite build tarballs are downloaded without integrity verification—a good catch for supply-chain safety! Lines 76–80 and 135–138 both use get_url without checksums.
For Node.js (line 76), you could add a checksum variable pointing to the official Node.js release hash. For satellite (line 135), since the URL comes from your API response (line 116), you'd want to either include a checksum in the API response or fetch it separately.
A simple start might look like adding checksum: "sha256:..." to each get_url task once you have the checksum sources sorted. The Ansible module supports it natively, so the integration is straightforward.
| - name: Fetch satellite build info from API | ||
| ansible.builtin.uri: | ||
| url: "{{ satellite_api_url }}" | ||
| return_content: true | ||
| register: satellite_api | ||
| delegate_to: localhost | ||
| become: false | ||
| run_once: true | ||
| when: satellite_version is not defined | ||
|
|
||
| - name: Set satellite build facts from API | ||
| ansible.builtin.set_fact: | ||
| satellite_version: "{{ satellite_api.json.packages[0].version }}" | ||
| satellite_url: "{{ satellite_api.json.packages[0].uri }}" | ||
| when: satellite_version is not defined | ||
|
|
||
| - name: Show satellite version | ||
| ansible.builtin.debug: | ||
| msg: "Satellite {{ satellite_version }} — {{ satellite_url }}" | ||
|
|
There was a problem hiding this comment.
Add explicit pin-variable validation for better failure mode.
If satellite_version is provided without satellite_url, Line 121 can fail with an undefined variable error. A fast assert gives a clear message.
Suggested patch
+ - name: Validate pinned satellite variables
+ ansible.builtin.assert:
+ that:
+ - (satellite_version is defined and satellite_url is defined) or
+ (satellite_version is not defined and satellite_url is not defined)
+ fail_msg: "Set both satellite_version and satellite_url together, or set neither."
+
- name: Fetch satellite build info from API
ansible.builtin.uri:
url: "{{ satellite_api_url }}"🧰 Tools
🪛 Checkov (3.2.334)
[medium] 103-114: Ensure that HTTPS url is used with uri
(CKV2_ANSIBLE_1)
| - name: Deploy boot config | ||
| ansible.builtin.copy: | ||
| dest: "{{ boot_config_path }}" | ||
| owner: root | ||
| group: root | ||
| mode: "0666" | ||
| content: | |
There was a problem hiding this comment.
Please avoid world-writable boot config permissions.
Line 265 sets 0666, which allows any local user to alter runtime config. This should be tightened.
Suggested patch
- mode: "0666"
+ mode: "0644"📝 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.
| - name: Deploy boot config | |
| ansible.builtin.copy: | |
| dest: "{{ boot_config_path }}" | |
| owner: root | |
| group: root | |
| mode: "0666" | |
| content: | | |
| - name: Deploy boot config | |
| ansible.builtin.copy: | |
| dest: "{{ boot_config_path }}" | |
| owner: root | |
| group: root | |
| mode: "0644" | |
| content: | |
| - name: Verify satellite is responding | ||
| ansible.builtin.uri: | ||
| url: "http://{{ ansible_host }}:9999/api/status" | ||
| return_content: false | ||
| status_code: 200 | ||
| retries: 5 | ||
| delay: 5 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n satellite-provisioning/satellite.yml | sed -n '285,305p'Repository: bitfocus/companion-satellite
Length of output: 786
Add until condition and fallback for host variable to ensure retries work correctly.
The retries and delay directives alone won't reliably retry without an until condition—the task will fail immediately instead of retrying. Additionally, using a fallback for ansible_host is a good defensive practice. The suggested patch registers the response and uses until to check the status code:
Suggested patch
- name: Verify satellite is responding
ansible.builtin.uri:
- url: "http://{{ ansible_host }}:9999/api/status"
+ url: "http://{{ ansible_host | default(inventory_hostname) }}:9999/api/status"
return_content: false
status_code: 200
+ register: satellite_status
retries: 5
delay: 5
+ until: satellite_status.status == 200📝 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.
| - name: Verify satellite is responding | |
| ansible.builtin.uri: | |
| url: "http://{{ ansible_host }}:9999/api/status" | |
| return_content: false | |
| status_code: 200 | |
| retries: 5 | |
| delay: 5 | |
| - name: Verify satellite is responding | |
| ansible.builtin.uri: | |
| url: "http://{{ ansible_host | default(inventory_hostname) }}:9999/api/status" | |
| return_content: false | |
| status_code: 200 | |
| register: satellite_status | |
| retries: 5 | |
| delay: 5 | |
| until: satellite_status.status == 200 |
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 291-300: Ensure that HTTPS url is used with uri
(CKV2_ANSIBLE_1)
| WantedBy=multi-user.target | ||
| notify: restart satellite | ||
|
|
||
| - name: Check for newer Raspberry Pi OS boot path |
There was a problem hiding this comment.
Maybe this path should be simplified, seeing as if you are using ansible you are unlikely to want to edit this file manually.
These paths are tried:
https://github.com/bitfocus/companion-satellite/blob/65ea421e8024834c09ecd03b058266b4bf6af980/satellite/src/fixup-pi-config.ts#L23C1-L29C2
so, you could simplify and go for just '/satellite-config', and avoid this detection and fiddling with paths.
Another thing to note is that this file is intended to be 'defaults'. It gets read at the next launch of the service, with any values propagated to the actual storage and then restored to default values. This is probably fine as is, but something to be aware of
| gather_facts: true | ||
|
|
||
| vars: | ||
| node_version: "24.13.0" |
There was a problem hiding this comment.
I'm not keen on the node_version being an input to the script here, we don't really document what version is needed and it does change sometimes. So I expect this will be a point of frustration in figuring out what the correct value should be.
It would be better if the .node_version from the repository was packaged into the builds, and used instead, with this as a sensible fallback value for older builds
(I'm a little surprised that I haven't already done this)
There was a problem hiding this comment.
Where is the source of truth for node_version? It is possible to pull dynamically from an external source, but it is already a var, so any build automation would do:
ansible-playbook satellite.yml \
-e node_version=xx.xx.x
Alternatively, perhaps there is a Bitfocus API endpoint for node_version as well? This playbook is effectively standalone from the companion-satellite repository (possibly a benefit for some users) except for when it calls the API to ultimately get satellite_url.
if the .node_version from the repository was packaged into the builds, and used instead
My philosophy keeps them separate as it bloats the build, but that's up to you :)
There was a problem hiding this comment.
The exact version to use really depends on the build.
- 2.7.0+ wants 24.13.0
- 2.3 - 2.6 wants 22.x
- 1.9 - 2.2 wants 20.x
They probably run fine with the 'wrong' version, but no guarantee (which is why I am surprised I havent handled this already, but I suppose I dont expect people to update often. I'm still using some 2.1/2.2 because there hasnt been a reason to update)
So yes, I do think that a .node-version file should be added to the builds. Its only a few bytes anyway, so not exactly bloat (much less so than the way we download an electron build to then extract the contents of it)
Its the 'linux img' download option: 
Just a minor clarification, the existing install script does work on x86_64 (it is meant to at least). It was originally written for pi, but has since expanded. But changing the path means breaking a lot of documentation and anyones automations (maybe the self-update ability too?), so hasnt been done. I am open to this, but have a couple of concerns:
Maybe it is time to start thinking about packaging this as a deb? It does the current approach of downloading and extracting simply because it works well enough and avoids the complexity of figuring out both deb packaging, and properly producing a standalone build. Doesn't have to be part of this, just a thought |
The satellite service already checks /boot, /boot/firmware, and / so we can skip the detection and symlink logic entirely.
There was a problem hiding this comment.
♻️ Duplicate comments (5)
satellite-provisioning/README.md (1)
9-19:⚠️ Potential issue | 🟡 MinorQuick-start still has mixed directory context and missing fence languages.
Nice docs overall—this is the same nit from the earlier pass: Step 1 assumes a directory, Step 2 changes into one, and both code fences should be tagged as
bash.Suggested patch
-1. Edit the ansible inventory with your satellite host(s) -``` -cp inventory.example inventory -vim inventory -``` +1. Move into the provisioning directory and edit inventory +```bash +cd satellite-provisioning +cp inventory.example inventory +vim inventory +``` 2. Provision -``` -cd satellite-provisioning -ansible-playbook satellite.yml -``` +```bash +ansible-playbook satellite.yml +```satellite-provisioning/satellite.yml (4)
76-80:⚠️ Potential issue | 🟠 MajorPlease add checksum verification for both tarball downloads.
This is still open from earlier review:
get_urlis used without integrity checks for Node.js and satellite archives.Suggested patch
- name: Download Node.js tarball ansible.builtin.get_url: url: "{{ node_url }}" dest: "{{ build_tmp }}/node.tar.xz" mode: "0644" + checksum: "{{ ('sha256:' ~ node_sha256) if (node_sha256 is defined) else omit }}" @@ - name: Download satellite build tarball ansible.builtin.get_url: url: "{{ satellite_url }}" dest: "{{ build_tmp }}/companion-satellite.tar.gz" mode: "0644" + checksum: "{{ ('sha256:' ~ satellite_sha256) if (satellite_sha256 is defined) else omit }}"Also applies to: 135-138
103-117:⚠️ Potential issue | 🟡 MinorAdd explicit validation when pinning satellite version/url.
Still the same issue from prior pass: if only one of
satellite_version/satellite_urlis provided, failure mode is unclear.Suggested patch
+ - name: Validate pinned satellite variables + ansible.builtin.assert: + that: + - (satellite_version is defined and satellite_url is defined) or + (satellite_version is not defined and satellite_url is not defined) + fail_msg: "Set both satellite_version and satellite_url together, or set neither." + - name: Fetch satellite build info from API ansible.builtin.uri:
251-257:⚠️ Potential issue | 🟠 MajorPlease tighten
/satellite-configpermissions.This remains from the earlier pass: mode
0666lets any local user modify runtime defaults.Suggested patch
- mode: "0666" + mode: "0644"
274-280:⚠️ Potential issue | 🟠 MajorRetries here won’t reliably apply without
until; also addansible_hostfallback.Same issue as earlier: add an
untilcondition for retry behavior, and default toinventory_hostnameifansible_hostis unset.Suggested patch
- name: Verify satellite is responding ansible.builtin.uri: - url: "http://{{ ansible_host }}:9999/api/status" + url: "http://{{ ansible_host | default(inventory_hostname) }}:9999/api/status" return_content: false status_code: 200 + register: satellite_status retries: 5 delay: 5 + until: satellite_status.status == 200
🧹 Nitpick comments (2)
satellite-provisioning/README.md (1)
61-69: Consider adding a short “maintenance model” note for operators.Given this is a standalone path, a brief note about how users should keep local playbooks updated (and what they miss vs legacy self-update behavior) would reduce support confusion.
satellite-provisioning/satellite.yml (1)
22-23:node_versionas a manual input can drift from satellite build requirements.Friendly suggestion: use build-provided metadata (when available) as source-of-truth, with this value as fallback for older builds.
That's kind of what I had in mind, if you want. At this time the playbook is standalone but if you want to incorporate it into your own build system, feel free :)
The playbook paradigm is especially helpful for users like me who like to run their own infra. If you'd like to incorporate it into your build system, I think it's a safer starting point than what I saw in pi-image/.
Lol, I didn't see the
That might help some users, but the |
2 participants
The project root of companion-satellite claims "A prebuilt image is available on the Bitfocus website". I think this used to exist(?) but I can't find it anymore. I wrote a playbook that modernizes the old
install.sh/update.shflow which was historically in thepi-image/directory.This new deployment paradigm should work with any Debian-based system (arm64 or x86_64), not just Raspberry Pi.
Tested with both a fresh Raspberry Pi OS 64-bit install as well as with an old install (from v2.4.1 -> v2.7.0).
From the README:
satellite.ymlis an Ansible playbook that deploys Companion Satellite to a Debian-based host (or 100 of them, if you want). It is an alternative to the curl-pipe-bashinstall.shand subsequentupdate.shpath found in thepi-image/directory. This modern solution is fully declarative, idempotent, and suitable for managing fleets of satellites from a single control node.Supports arm64 (e.g. Raspberry Pi) and x86_64.
Quick start
Variables
Change these values before running the playbook if you'd like.
companion_ip127.0.0.1branchstablebeta) (used to query the Bitfocus API)node_version24.13.0satellite_versionsatellite_url)satellite_urlAlternatively, you can override these vars at run time with
-e:What it deploys
/opt/nodefrom official tarball (arch-detected)/opt/companion-satellite50-satellite.rules)satellite.servicerunning as thesatelliteuser (in thedialoutgroup)/boot/satellite-configwithCOMPANION_IP(handles newer RPi OS/boot/firmwarepath)Updating
Simply re-run the playbook to pick up the latest build for the configured branch. The playbook queries the Bitfocus API and skips the download if the installed version already matches.
Otherwise, to pin a specific version:
Differences from install.sh / update.sh
This playbook is a standalone, declarative provisioning path. It installs Node.js directly from the official tarball (no fnm) and does not clone the companion-satellite git repo. This means:
satellite-update,satellite-help,satellite-license, andsatellite-edit-configare not available. Updates are done by re-running the playbook.node_versionvariable.These trade-offs keep the installation lightweight and fully managed by Ansible.
Summary by CodeRabbit
New Features
Documentation
Chores