fix(snap): simplify snap hooks (#3988)

* fix(snap): simplify snap hooks

The `post-refresh` hook runs after initial snap installation as well, so
there is no need to call the `install` hook from within the
`post-refresh` hook; instead, the logic can simply be moved into the
`post-refresh` hook directly, and the `install` hook removed.

Also, the existing `install` hook logic looked for an insecure
configuration, and if found, replaced the entire configuration file with
a minimal default in the current format. But OpenShell does that default
behavior without any config file, so we may as well simply remove the
configuration file entirely to keep up-to-date with the current default
behavior. Let OpenShell create a configuration file if it needs to,
rather than auto-create one via the packaging scripts.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* fix(snap): remove the connect-plug-docker hook

The `openshell:docker` is auto-connected to the system `:docker` slot,
so there should not be a need to separately restart the gateway service
when the interface is connected.

For locally-built test snaps which were not published to the store, the
autoconnection is not made, but when the snap is installed, the gateway
will attempt to start anyway and fail to find any available compute
driver, so quickly restart until it hits the systemd start-limit, after
which systemd prevents the service from being started again. If a user
tries to manually connect their locally-built `openshell` snap to the
`:docker` slot, then the `connect-plug-docker` hook runs and triggers a
restart of the gateway, which will usually fail because the start limit
has already been hit. An error in the hook will thus cause the interface
connection to be undone, which is undesirable.

Thus, we can remove this hook entirely, and instead allow interface
connections to succeed as intended. The user still needs to manually
restart the gateway service after making a manual connection (as was the
case previously) and probably needs to `systemctl reset-failed` first,
but at least connection will succeed beforehand so they can proceed with
these steps.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* fix(snap): set refresh-mode: endure again, with manual restart

Return to the previous behavior before commit a67567e, where the
gateway is not stopped before refreshes. The `post-refresh` hook
now restarts the gateway if the TLS configuration was corrected, so we
don't have to enforce restarting the gateway on every refresh even when
not necessary. Thus, set `refresh-mode: endure`, and let the hook decide
when the gateway needs to be restarted.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* fix(snap): update docs and tests to reflect snap hook changes

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* docs(snap): remove verbose explanation of snap gateway refresh behavior

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

---------

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

Author: olivercalder

docs/about/installation.mdx

@@ -126,13 +126,15 @@ openshell status
 
 Keep the client key private.
 
-To install a locally built snap, connect its interfaces manually:
+To install a locally built snap, connect its interfaces manually. The gateway may reach systemd's start limit before Docker is connected, so reset the failed unit and restart the gateway after connecting the interfaces:
 
 ```shell
 sudo snap install ./openshell_*.snap --dangerous
 sudo snap connect openshell:log-observe
 sudo snap connect openshell:system-observe
 sudo snap connect openshell:docker :docker
+sudo systemctl reset-failed snap.openshell.gateway.service
+sudo snap restart openshell.gateway
 ```
 
 ## Kubernetes

python/openshell/release_formula_test.py

@@ -4,7 +4,6 @@
 from __future__ import annotations
 
 import re
-import stat
 import subprocess
 import sys
 from pathlib import Path
@@ -168,17 +167,6 @@ def test_snap_wrapper_uses_optional_gateway_config_without_generating_toml() ->
     assert 'exec "${SNAP}/bin/openshell-gateway" "$@"' in wrapper
 
 
-def test_snap_docker_connect_hook_restarts_gateway() -> None:
-    repo_root = Path(__file__).resolve().parents[2]
-    hook = repo_root / "snap/hooks/connect-plug-docker"
-
-    assert hook.is_file()
-    assert hook.stat().st_mode & stat.S_IXUSR
-    assert 'snapctl restart "${SNAP_INSTANCE_NAME}.gateway"' in hook.read_text(
-        encoding="utf-8"
-    )
-
-
 def test_rpm_spec_seeds_and_migrates_gateway_defaults() -> None:
     repo_root = Path(__file__).resolve().parents[2]
     spec = (repo_root / "openshell.spec").read_text(encoding="utf-8")

snap/hooks/connect-plug-docker

@@ -2,11 +2,29 @@
 # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
 
-# Replace insecure gateway configs on refresh, then restart the gateway so the
-# new config takes effect even when the previous revision used
+# Remove insecure gateway configs on refresh, then restart the gateway so the
+# default config takes effect even when the previous revision used
 # refresh-mode: endure and kept its plaintext gateway running.
 
 set -eu
 
-"${SNAP}/meta/hooks/install"
+config_file="${SNAP_COMMON}/gateway.toml"
+insecure='^[[:space:]]*(allow_unauthenticated_users|disable_tls)[[:space:]]*=[[:space:]]*true([[:space:]#]|$)'
+
+# Keep secure operator configs, symlinks, and directories. Remove a config
+# that explicitly allows plaintext or anonymous access, even if it has other
+# edits.
+if [ ! -e "$config_file" ]; then
+	exit 0
+elif [ -L "$config_file" ]; then
+	exit 0
+elif [ -f "$config_file" ]; then
+	grep -Eq "$insecure" "$config_file" || exit 0
+	echo "openshell: removing insecure gateway config to use the mTLS default" >&2
+elif [ -e "$config_file" ]; then
+	exit 0
+fi
+
+rm -f "$config_file"
+
 snapctl restart "${SNAP_INSTANCE_NAME}.gateway"

snap/hooks/install

@@ -88,9 +88,10 @@ apps:
   gateway:
     command: bin/openshell-gateway-wrapper
     daemon: simple
-    # Refresh must activate the migrated mTLS config immediately. This
-    # interrupts active sandbox sessions.
-    refresh-mode: restart
+    # refresh-mode: endure prevents snapd from restarting the gateway daemon
+    # during snap refreshes, which would kill active sandbox sessions.
+    # Operators must manually restart the service after a refresh if needed.
+    refresh-mode: endure
     # Snapd runs this daemon as root. The wrapper serves TLS from the bundle
     # generated in $SNAP_COMMON/tls, and the default config requires client
     # certificates; the installer copies the client bundle to the target

snap/hooks/post-refresh

@@ -83,16 +83,14 @@ snapcraft="${ROOT}/snapcraft.yaml"
 snap_install_docs="${ROOT}/docs/about/installation.mdx"
 snap_canary="${ROOT}/.github/workflows/release-canary.yml"
 snap_repro="${ROOT}/nix/test-guest/scripts/snap-gateway-repro.sh"
-snap_docker_hook="${ROOT}/snap/hooks/connect-plug-docker"
-snap_install_hook="${ROOT}/snap/hooks/install"
+snap_post_refresh_hook="${ROOT}/snap/hooks/post-refresh"
 package_deb="${ROOT}/tasks/scripts/package-deb.sh"
 assert_file_exists "$snap_wrapper"
 assert_file_exists "$snapcraft"
 assert_file_exists "$snap_install_docs"
 assert_file_exists "$snap_canary"
 assert_file_exists "$snap_repro"
-assert_file_exists "$snap_docker_hook"
-assert_file_exists "$snap_install_hook"
+assert_file_exists "$snap_post_refresh_hook"
 assert_file_exists "$package_deb"
 assert_contains "$service" "ExecStartPre=/usr/bin/openshell-gateway config preflight"
 assert_contains "$package_deb" "\$src_dir/openshell-gateway.service"
@@ -114,28 +112,33 @@ for snap_file in \
   "$snap_install_docs" \
   "$snap_canary" \
   "$snap_repro" \
-  "$snap_docker_hook" \
-  "$snap_install_hook"; do
+  "$snap_post_refresh_hook"; do
   assert_not_contains "$snap_file" "docker:docker-daemon"
   assert_not_contains "$snap_file" "default-provider: docker"
 done
-if [[ ! -x "$snap_install_hook" ]]; then
-  echo "FAIL: Snap install hook must be executable" >&2
+if [[ -e "${ROOT}/snap/hooks/connect-plug-docker" ]]; then
+  echo "FAIL: obsolete Snap Docker connection hook must not exist" >&2
   exit 1
 fi
-assert_not_contains "$snap_install_hook" 'compute_driver'
-assert_not_contains "$snap_install_hook" 'allow_unauthenticated_users = true'
-assert_contains "$snapcraft" 'refresh-mode: restart'
-if [[ ! -x "$(dirname "$snap_install_hook")/post-refresh" ]]; then
+if [[ -e "${ROOT}/snap/hooks/install" ]]; then
+  echo "FAIL: obsolete Snap install hook must not exist" >&2
+  exit 1
+fi
+assert_contains "$snapcraft" 'refresh-mode: endure'
+if [[ ! -x "$snap_post_refresh_hook" ]]; then
   echo "FAIL: Snap post-refresh hook must be executable" >&2
   exit 1
 fi
 assert_not_contains "$ROOT/tasks/scripts/snap-gateway-wrapper.sh" 'OPENSHELL_DISABLE_TLS'
-bash "$ROOT/tasks/scripts/test-snap-install-hook.sh" "$snap_install_hook"
+bash "$ROOT/tasks/scripts/test-snap-post-refresh-hook.sh" "$snap_post_refresh_hook"
 assert_not_contains "$snap_install_docs" "snap connect openshell:home"
 assert_not_contains "$snap_install_docs" "snap connect openshell:network"
 assert_not_contains "$snap_install_docs" "snap connect openshell:network-bind"
 assert_contains "$snap_install_docs" "snap connect openshell:docker :docker"
+assert_contains "$snap_install_docs" "systemctl reset-failed snap.openshell.gateway.service"
+assert_contains "$snap_install_docs" "snap restart openshell.gateway"
+assert_contains "$snap_install_docs" "Snap refreshes keep the running gateway process active"
+assert_contains "$snap_install_docs" "install script refreshes and restarts the gateway automatically"
 assert_contains "$snap_canary" "install.sh | sh"
 assert_contains "$snap_canary" "ubuntu-snap-system-docker:"
 assert_contains "$snap_canary" "ubuntu-snap-docker-preflight:"