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

DietPi-Sofware | Homebridge: Bringing HomeKit support where there is none #6493

Merged
merged 5 commits into from
Jul 23, 2023
Merged

DietPi-Sofware | Homebridge: Bringing HomeKit support where there is none #6493

merged 5 commits into from
Jul 23, 2023

Conversation

Zer0x00
Copy link
Contributor

@Zer0x00 Zer0x00 commented Jul 19, 2023

Should close #5142

Documentation can be found here: MichaIng/DietPi-Docs#904

@Zer0x00 Zer0x00 marked this pull request as draft July 19, 2023 07:51
@Zer0x00 Zer0x00 marked this pull request as ready for review July 19, 2023 08:11
@Joulinar Joulinar requested review from MichaIng and Joulinar July 20, 2023 21:06
@Joulinar Joulinar added this to the v8.20 milestone Jul 20, 2023
@MichaIng
Copy link
Owner

MichaIng commented Jul 20, 2023

Many thanks for your implementation, looks clean and well done 🙂.

Started an install test on all platforms: https://github.com/MichaIng/DietPi/actions/runs/5615943138
Trixie installs are currently expected to fail (systemd related issue).

@MichaIng
Copy link
Owner

Should be also added to our test script, i.e. to not only test whether the install goes through, but also whether the service is running afterwards, ports are listened on an optionally whether CLI commands work (of present): https://github.com/MichaIng/DietPi/blob/master/.github/workflows/dietpi-software.bash

@Zer0x00
Copy link
Contributor Author

Zer0x00 commented Jul 20, 2023

Thanks for the kind words 🙂

The tests have been added, but I wonder if I missed the documentation for it? If there's no documentation it would be great if it could be added to https://github.com/MichaIng/DietPi/wiki/How-to-add-a-new-software-title which I followed to create this PR.

If you're wondering why > /dev/null is needed in the test:
hb-service status does an environment change, I thought it might break some other subsequent checks, so it's just a precautionary measure.

Another possible way would be to only check for hb-service if the workaround isn't desired.

dietpi/dietpi-software Outdated Show resolved Hide resolved
@MichaIng
Copy link
Owner

MichaIng commented Jul 23, 2023

Test failure:

Jul 23 19:18:20 DietPi systemd[2485]: homebridge.service: Failed to apply ambient capabilities (before UID change): Operation not permitted
Jul 23 19:18:20 DietPi systemd[2485]: homebridge.service: Failed at step CAPABILITIES spawning /opt/homebridge/start.sh: Operation not permitted

This does not happen on real machines but only within these containers. Pretty strange that it happens on all QEMU-emulated systems, but also on x86_64 Buster, but not anymore from Bullseye on. The service is exactly the same. Probably there has been some fix made in systemd(-container) and QEMU ships with an own limitation, similar to the PrivateUser problem on 32-bit ARM tests.

CAP_NET_BIND_SERVICE is not needed anyway as the port is >1024, CAP_NET_ADMIN would be needed for e.g. iptables, no idea what CAP_NET_RAW would be needed for 🤔. Let's see whether the service starts up fine if we just disable all capabilities for the test.

Zer0x00 and others added 5 commits July 23, 2023 20:42
- DietPi-Software | Homebridge: Minor coding, add service to dietpi-services, attempt to fix CI service test
- DietPi-Software | Homebridge: Disable for RISC-V, as the repo does not provide packages for it yet, and increase delay before checking TCP port in CI workflow
Copy link
Owner

@MichaIng MichaIng left a comment

Choose a reason for hiding this comment

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

Test issue solved by unsetting capabilities.

For some reason, in the test container on aarch64, the service takes very long to start up. However, otherwise it works well, also accessing the web UI and creating the initial admin account is all pretty smooth and simple. Could not find any issues when navigating the web UI etc. Merging it now for testing in beta.

@MichaIng
Copy link
Owner

@Zer0x00
Did you face as well the issue that plugins cannot be installed OOTB as npm tries to navigate to and in case create the homebridge user's home directory, which is /nonexistence, hence does not exist and (thank god) cannot be created?

The package install even prints funny errors about it:

Adding system user `homebridge' (UID 104) ...
Adding new group `homebridge' (GID 111) ...
Adding new user `homebridge' (UID 104) with group `homebridge' ...
Not creating `/nonexistent'.
cp: cannot create regular file '/home/homebridge/.bashrc': No such file or directory
chown: cannot access '/home/homebridge/.bashrc': No such file or directory

The postinst create the user via adduser --system without defining a home directory, in which case /nonexistent is used for system users, and of course not created. So all the pretty bashrc setup and stuff is doomed to fail. Also I do not understand, why as a system user /var/lib/homebridge is not used instead.

I fixed this our end: 3d8bafd
Will open a ticket a ticket at their repo to ask what the intention actually is and fix it their end: https://github.com/homebridge/homebridge-apt-pkg/issues

@MichaIng MichaIng mentioned this pull request Jul 29, 2023
@Zer0x00
Copy link
Contributor Author

Zer0x00 commented Jul 29, 2023

@MichaIng
Thanks for catching this issue, I didn‘t have time yet to do a clean installation. My current installation (which runs on debian buster) was from a prior time where no apt package was existent for homebridge, therefore I didn‘t ran into this issue on my Pi.

The plan was actually to have a start fresh with DietPi 8.20 and since my implementation did go until the webservice start I didn‘t expect any issues on the next steps, I‘m sorry - will do better next time! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants