From 1661240cfb3c047ea68f25e959c18c5d8dd1a462 Mon Sep 17 00:00:00 2001 From: JP Meijers <165929400+jpm-canonical@users.noreply.github.com> Date: Thu, 13 Jun 2024 16:49:16 +0200 Subject: [PATCH] Run Chip Tool without sudo (#55) * Set tmpdir to snap home dir using command chain. Remove outdated path rewrites and unnecessary mounts. * Add post refresh hook Copy system-wide common files to root user specific common directory. * Simplify local part build command * Add a test for upgrading from stable to local/edge Make sure we can still control a paired device after upgrading * Update readme to not use sudo where it's not required Add a section explaining the change to no requiring sudo Add commands to move to a new user and fix a know error --- README.md | 27 ++++++++-- snap/hooks/post-refresh | 8 +++ snap/local/bin/set-tmp-dir.sh | 5 ++ snap/snapcraft.yaml | 31 +++--------- tests/common.go | 4 +- tests/upgrade_test.go | 93 +++++++++++++++++++++++++++++++++++ tests/wifi_test.go | 2 - 7 files changed, 140 insertions(+), 30 deletions(-) create mode 100644 snap/hooks/post-refresh create mode 100755 snap/local/bin/set-tmp-dir.sh create mode 100644 tests/upgrade_test.go diff --git a/README.md b/README.md index 62aa3ef..e87891f 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ sudo snap connect chip-tool:process-control ### Commissioning into IP network Discover using DNS-SD and pair: ```bash -sudo chip-tool pairing onnetwork 110 20202021 +chip-tool pairing onnetwork 110 20202021 ``` where: @@ -56,7 +56,7 @@ Done Discover and pair: ```bash -sudo chip-tool pairing ble-thread 110 hex:0e08...f7f8 20202021 3840 +chip-tool pairing ble-thread 110 hex:0e08...f7f8 20202021 3840 ``` where: @@ -70,7 +70,7 @@ where: ### Control Toggle: ```bash -sudo chip-tool onoff toggle 110 1 +chip-tool onoff toggle 110 1 ``` where: @@ -81,6 +81,27 @@ where: - `1` is the endpoint of the configured device +### Note on sudo + +The latest version of the chip-tool snap does not require the use of sudo (root access). If you have updated the snap from a previous version it will still work with sudo. If you run it as a normal user, the previous state of provisioned devices will not be available. + +To change from running with sudo to running without sudo, you need to copy the database files from the root user to your user, and update the file ownerships. This can be done with these two commands: + +``` +sudo cp /var/snap/chip-tool/common/mnt/chip_tool_* ~/snap/chip-tool/common/ +sudo chown $USER:$USER ~/snap/chip-tool/common/* +``` + +If you run chip-tool again without sudo and get an error similar to `CHIP Error 0x000000AF: Write to file failed`, either restart your computer to clear all temporary files, or run the following commands to delete them: + +``` +# Open a shell inside the chip-tool snap sandbox +sudo snap run --shell chip-tool.chip-tool +# Inside this shell, delete the temporary files +rm /tmp/chip_* +``` + + ## Build Build locally for the architecture same as the host: diff --git a/snap/hooks/post-refresh b/snap/hooks/post-refresh new file mode 100644 index 0000000..a84a13a --- /dev/null +++ b/snap/hooks/post-refresh @@ -0,0 +1,8 @@ +#!/bin/bash + +# This script runs as root, so $SNAP_USER_COMMON is /root/snap/chip-tool/common +# See https://forum.snapcraft.io/t/snapcraft-hook-support/19069/12 + +if [ -d $SNAP_COMMON/mnt ] && [ -n "$(ls -A $SNAP_COMMON/mnt)" ]; then + cp $SNAP_COMMON/mnt/* $SNAP_USER_COMMON/; +fi diff --git a/snap/local/bin/set-tmp-dir.sh b/snap/local/bin/set-tmp-dir.sh new file mode 100755 index 0000000..4f6c4b5 --- /dev/null +++ b/snap/local/bin/set-tmp-dir.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +export TMPDIR=$SNAP_USER_COMMON + +exec "$@" diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index ad9cd1b..2af9e58 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -13,11 +13,13 @@ architectures: - build-on: amd64 - build-on: arm64 -layout: - /mnt: - bind: $SNAP_COMMON/mnt - parts: + local: + plugin: nil + source: snap/local + override-build: | + cp -rv bin $SNAPCRAFT_PART_INSTALL/ + connectedhomeip: plugin: nil build-environment: @@ -40,23 +42,6 @@ parts: mkdir -p $CRAFT_PART_INSTALL/bin cd ../../connectedhomeip/src - - # The project writes its data to /tmp which isn't persisted. - # - # Setting TMPDIR env var when running the app isn't sufficient as - # chip_[config,counter,factory,kvs].ini still get written under /tmp. - # The chip-tool currently has no way of overriding the default paths to - # storage and security config files. - # - # Snap does not allow bind mounting a persistent directory on /tmp, - # so we need to replace it in the source with another path, e.g. /mnt. - # See the top-level layout definition which bind mounts a persisted - # directory within the confined snap space on /mnt. - # - # Replace storage paths: - sed -i 's/\/tmp/\/mnt/g' src/platform/Linux/CHIPLinuxStorage.h - # Replace key-value store path: - sed -i 's/\/tmp/\/mnt/g' src/platform/Linux/CHIPPlatformConfig.h # To avoid activation errors, don't treat unset variables as error set +u @@ -88,6 +73,7 @@ parts: apps: chip-tool: + command-chain: [bin/set-tmp-dir.sh] command: bin/chip-tool plugs: - network @@ -95,6 +81,3 @@ apps: - bluez - avahi-observe - process-control - environment: - # Replace the path for chip-tool configuration files - TMPDIR: "/mnt" diff --git a/tests/common.go b/tests/common.go index 7996236..4d8be61 100644 --- a/tests/common.go +++ b/tests/common.go @@ -7,8 +7,10 @@ import ( "github.com/stretchr/testify/require" ) +const allClusterSnap = "matter-all-clusters-app" +const chipToolSnap = "chip-tool" + func InstallChipTool(t *testing.T) { - const chipToolSnap = "chip-tool" // clean utils.SnapRemove(t, chipToolSnap) diff --git a/tests/upgrade_test.go b/tests/upgrade_test.go new file mode 100644 index 0000000..c0f9e19 --- /dev/null +++ b/tests/upgrade_test.go @@ -0,0 +1,93 @@ +package tests + +import ( + "github.com/canonical/matter-snap-testing/utils" + "github.com/stretchr/testify/assert" + "log" + "os" + "testing" + "time" +) + +func TestUpgrade(t *testing.T) { + start := time.Now() + + // Remove snaps and logs at end of test, even if it failed + t.Cleanup(func() { + utils.SnapRemove(nil, allClusterSnap) + utils.SnapDumpLogs(nil, start, allClusterSnap) + utils.SnapRemove(nil, chipToolSnap) + utils.SnapDumpLogs(nil, start, chipToolSnap) + }) + + // Start clean + utils.SnapRemove(t, allClusterSnap) + utils.SnapRemove(t, chipToolSnap) + + // Install stable chip tool from store + utils.SnapInstallFromStore(t, chipToolSnap, "latest/stable") + + // Setup chip-tool + utils.SnapConnect(t, chipToolSnap+":avahi-observe", "") + utils.SnapConnect(t, chipToolSnap+":bluez", "") + utils.SnapConnect(t, chipToolSnap+":process-control", "") + + // Install all clusters app + utils.SnapInstallFromStore(t, allClusterSnap, utils.ServiceChannel) + + // Setup all clusters app + utils.SnapSet(t, allClusterSnap, "args", "--wifi") + utils.SnapConnect(t, allClusterSnap+":avahi-control", "") + utils.SnapConnect(t, allClusterSnap+":bluez", "") + + // Start all clusters app + utils.SnapStart(t, allClusterSnap) + utils.WaitForLogMessage(t, + allClusterSnap, "CHIP minimal mDNS started advertising", start) + + // Pair device + t.Run("Commission", func(t *testing.T) { + stdout, _, _ := utils.Exec(t, "sudo chip-tool pairing onnetwork 110 20202021 2>&1") + assert.NoError(t, + os.WriteFile("chip-tool-pairing.log", []byte(stdout), 0644), + ) + }) + + // Control device + t.Run("Control with stable snap", func(t *testing.T) { + snapVersion := utils.SnapVersion(t, chipToolSnap) + snapRevision := utils.SnapRevision(t, chipToolSnap) + log.Printf("%s installed version %s build %s\n", chipToolSnap, snapVersion, snapRevision) + + stdout, _, _ := utils.Exec(t, "sudo chip-tool onoff toggle 110 1 2>&1") + assert.NoError(t, + os.WriteFile("chip-tool-onoff.log", []byte(stdout), 0644), + ) + + utils.WaitForLogMessage(t, + allClusterSnap, "CHIP:ZCL: Toggle ep1 on/off", start) + }) + + // Upgrade chip-tool to local snap or edge + if utils.LocalServiceSnap() { + utils.SnapInstallFromFile(t, utils.LocalServiceSnapPath) + } else { + utils.SnapRefresh(t, chipToolSnap, "latest/edge") + } + + // Control device again + t.Run("Control upgraded snap", func(t *testing.T) { + snapVersion := utils.SnapVersion(t, chipToolSnap) + snapRevision := utils.SnapRevision(t, chipToolSnap) + log.Printf("%s installed version %s build %s\n", chipToolSnap, snapVersion, snapRevision) + + stdout, _, _ := utils.Exec(t, "sudo chip-tool onoff toggle 110 1 2>&1") + assert.NoError(t, + os.WriteFile("chip-tool-onoff.log", []byte(stdout), 0644), + ) + + utils.WaitForLogMessage(t, + allClusterSnap, "CHIP:ZCL: Toggle ep1 on/off", start) + }) + +} diff --git a/tests/wifi_test.go b/tests/wifi_test.go index 2830437..feb2d30 100644 --- a/tests/wifi_test.go +++ b/tests/wifi_test.go @@ -9,8 +9,6 @@ import ( "github.com/stretchr/testify/assert" ) -const allClusterSnap = "matter-all-clusters-app" - func TestAllClustersAppWiFi(t *testing.T) { InstallChipTool(t)