bootstrap: passwordless sudo for key-only platform users; groups once; inventory fixes

cmd/ansible_inventory.go

@@ -125,6 +125,16 @@ func deviceWireGuardActive(d *api.BackstageDevice) bool {
 	return wg.State == "active"
 }
 
+func unreachableReason(transport string) string {
+	switch transport {
+	case transportWireGuard:
+		return "WireGuard is not active"
+	case transportTunnel:
+		return "the SSH tunnel is not open"
+	}
+	return "no active WireGuard and no open SSH tunnel"
+}
+
 func withBecome(hv map[string]interface{}, password string) map[string]interface{} {
 	hv["ansible_become_user"] = "root"
 	hv["ansible_become_flags"] = "-E"
@@ -137,7 +147,8 @@ func withBecome(hv map[string]interface{}, password string) map[string]interface
 
 // buildInventory builds the Ansible dynamic inventory (--list form) for the
 // devices and writes the key files. Devices without login data or without a
-// usable transport are listed in their groups but get no host vars.
+// usable transport are left out (one line on warn each), so a run reports
+// only the devices it can reach.
 func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writer) (map[string]interface{}, error) {
 	if o.Transport == "" {
 		o.Transport = transportAuto
@@ -186,6 +197,7 @@ func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writ
 		}
 
 		if d.Spec.Settings.Username == "" || d.Spec.Settings.RSAPrivate == "" {
+			fmt.Fprintf(warn, "tnctl: device %q has no login data in the platform; left out of the inventory\n", name)
 			continue
 		}
 		username, password, key, err := decodeDeviceLogin(d)
@@ -205,6 +217,7 @@ func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writ
 		port := d.Spec.Subresources.Status.RemoteAccess.Port
 		tunnelOpen := d.Spec.Subresources.Status.RemoteAccess.Active && port != ""
 		if !useWG && !(o.Transport != transportWireGuard && tunnelOpen) {
+			fmt.Fprintf(warn, "tnctl: device %q is not reachable (%s); left out of the inventory\n", name, unreachableReason(o.Transport))
 			continue
 		}
 
@@ -242,6 +255,7 @@ func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writ
 				"ansible_kubectl_namespace":    ns,
 				"ansible_teknoir_namespace":    ns,
 				"ansible_teknoir_device":       name,
+				"ansible_teknoir_login_user":   username,
 				"ansible_teknoir_domain":       o.Domain,
 				"ansible_teknoir_transport":    transportWireGuard,
 			}, password)
@@ -269,6 +283,7 @@ func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writ
 			"ansible_teknoir_tunnel_port":  portVal,
 			"ansible_teknoir_tunnel_open":  true,
 			"ansible_teknoir_device":       name,
+			"ansible_teknoir_login_user":   username,
 			"ansible_teknoir_domain":       o.Domain,
 			"ansible_teknoir_transport":    transportTunnel,
 		}, password)
@@ -281,7 +296,18 @@ func buildInventory(devices []api.BackstageDevice, o inventoryOpts, warn io.Writ
 	}
 	sort.Strings(names)
 	for _, g := range names {
-		inv[g] = map[string]interface{}{"hosts": groups[g], "vars": map[string]interface{}{}}
+		// Only hosts Ansible can reach: a device with no transport (no tunnel,
+		// no active WireGuard, no login) would be resolved by name and fail.
+		var hosts []string
+		for _, h := range groups[g] {
+			if _, ok := hostvars[h]; ok {
+				hosts = append(hosts, h)
+			}
+		}
+		if len(hosts) == 0 {
+			continue
+		}
+		inv[g] = map[string]interface{}{"hosts": hosts, "vars": map[string]interface{}{}}
 	}
 	return inv, nil
 }

cmd/ansible_inventory_test.go

@@ -1,7 +1,9 @@
 package cmd
 
 import (
+	"bytes"
 	"encoding/base64"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -123,11 +125,20 @@ func TestBuildInventoryTransports(t *testing.T) {
 	}
 	for _, c := range cases {
 		t.Run(c.transport, func(t *testing.T) {
-			inv, err := buildInventory(devs, testOpts(t, c.transport), os.Stderr)
+			var warned bytes.Buffer
+			inv, err := buildInventory(devs, testOpts(t, c.transport), &warned)
 			if err != nil {
 				t.Fatal(err)
 			}
 			hv := hostvars(inv)
+			// every left-out device is named once on the warning stream
+			for _, d := range devs {
+				name := d.Metadata.Name
+				_, in := c.want[name]
+				if n := strings.Count(warned.String(), fmt.Sprintf("device %q", name)); (n == 1) == in {
+					t.Errorf("device %s: %d warning lines, in inventory = %v:\n%s", name, n, in, warned.String())
+				}
+			}
 			if len(hv) != len(c.want) {
 				t.Fatalf("hostvars for %d hosts, want %d: %v", len(hv), len(c.want), hv)
 			}
@@ -136,16 +147,29 @@ func TestBuildInventoryTransports(t *testing.T) {
 					t.Errorf("%s transport = %v, want %s", h, got, tr)
 				}
 			}
-			// groups: every device listed regardless of transport
+			// groups: only hosts with a transport (Ansible would try to resolve
+			// the others by name)
 			g := inv["teknoir_demo"].(map[string]interface{})["hosts"].([]string)
-			if len(g) != 5 {
-				t.Errorf("namespace group hosts = %v", g)
+			if len(g) != len(c.want) {
+				t.Errorf("namespace group hosts = %v, want %d", g, len(c.want))
+			}
+			for _, h := range g {
+				if _, ok := c.want[h]; !ok {
+					t.Errorf("unreachable host %s in a group", h)
+				}
+			}
+			var wantLabel []string
+			for _, h := range []string{"wg1", "tun1"} {
+				if _, ok := c.want[h]; ok {
+					wantLabel = append(wantLabel, h)
+				}
 			}
-			if !reflect.DeepEqual(inv["device_group_test"].(map[string]interface{})["hosts"], []string{"wg1", "tun1"}) {
-				t.Errorf("label group = %v", inv["device_group_test"])
+			if !reflect.DeepEqual(inv["device_group_test"].(map[string]interface{})["hosts"], wantLabel) {
+				t.Errorf("label group = %v, want %v", inv["device_group_test"], wantLabel)
 			}
-			if _, ok := inv["site_name_a_b"]; !ok {
-				t.Errorf("sanitised label group missing: %v", inv)
+			_, ok := inv["site_name_a_b"]
+			if _, wg1 := c.want["wg1"]; ok != wg1 {
+				t.Errorf("sanitised label group present = %v, want %v", ok, wg1)
 			}
 		})
 	}

cmd/bootstrap.go

@@ -134,6 +134,10 @@ var bootstrapCmd = &cobra.Command{
 			K3SForce:           force,
 		}
 
+		if err := templates.ValidateTemplateData(data); err != nil {
+			return fmt.Errorf("device %s: %w", deviceName, err)
+		}
+
 		generate := func(tmpl *template.Template, filename string) error {
 			f, err := os.Create(filename)
 			if err != nil {

docs/commands/ansible.md

@@ -11,7 +11,12 @@ tnctl ansible-inventory --list     # the inventory itself (Ansible dynamic-inven
 tnctl ansible-inventory --host <device>
 ```
 
-`ansible` / `ansible-playbook` must be installed and in `PATH`. All arguments are passed through untouched; only
+tnctl replaces the Teknoir Ansible inventory plugin (`inventory.py` and `install.sh` in
+[teknoir/ansible](https://github.com/teknoir/ansible)), which is deprecated. The playbooks in that repo run through
+`tnctl ansible-playbook`.
+
+`ansible` / `ansible-playbook` must be installed and in `PATH`. With `ansible-core` alone, also install the
+collections the Teknoir playbooks use: `ansible-galaxy collection install ansible.posix community.general`. All arguments are passed through untouched; only
 tnctl's own `-d`, `-n`, `--config` and `--debug` are accepted in front of them.
 
 ```bash
@@ -29,8 +34,29 @@ Hosts are the devices of the active domain and namespace (`-d`/`-n`, or the save
 
 A device needs a login name and a private key in the catalog. The password is optional; it only sets
 `ansible_become_pass` / `ansible_sudo_pass` (always with `ansible_become_user=root`, `ansible_become_flags=-E`).
-A device without login data, or without a usable transport (see below), stays in its groups but gets **no host
-variables**, so Ansible reports it as unreachable instead of failing the run.
+A device without login data, or without a usable transport (see below), is left out of the inventory and its groups,
+with one line on stderr per device (`tnctl: device "x" is not reachable (...)`), so a run only targets devices it can
+reach.
+
+`ansible_teknoir_login_user` holds the platform user, also when you log in as someone else with
+`-e ansible_user=...`.
+
+## Become (sudo)
+
+`-b` runs tasks as root through sudo. The device bootstrap gives the platform user passwordless sudo when the platform
+holds no password for it (key-only login, the default); otherwise the inventory supplies the device password from the
+platform. A device bootstrapped before passwordless sudo existed needs one re-bootstrap (see `playbooks/rebootstrap-device.yml`
+in teknoir/ansible) or `-e ansible_user=<another sudoer>` until then.
+
+## synchronize (rsync)
+
+`ansible.posix.synchronize` runs its own `ssh` for rsync. Pass `use_ssh_args: true` so it uses the inventory's transport
+(the WireGuard `ProxyCommand` or the tunnel), and `verify_host: true` so it keeps the host key check: without it the
+module puts `StrictHostKeyChecking=no` in front of the inventory's options, and ssh uses the first value.
+
+```bash
+tnctl ansible <device> -b -m synchronize -a "src=/var/log/syslog dest=./ mode=pull use_ssh_args=yes verify_host=yes"
+```
 
 Private keys are written to `<user cache dir>/tnctl/ansible/<domain>/<namespace>/<device>.pem` (directories `0700`,
 files `0600`) and rewritten when the catalog key changes.

pkg/templates/agent.sh.tmpl

@@ -78,8 +78,6 @@ tn_inst_finish
 
 info "K3s agent node joined successfully!"
 
-warn "To enable tunneling, the user ${_FIRST_USER_NAME} has to be created."
-
 {{template "user_setup.sh.tmpl" .}}
 
 {{template "user_invocation.sh.tmpl" .}}

pkg/templates/docker_server.sh.tmpl

@@ -56,8 +56,6 @@ info "Device bootstrapped successfully!"
 info "Notice!"
 info "No networking or user settings has been changed to match settings in the platform."
 
-warn "To enable tunneling, the user ${_FIRST_USER_NAME} has to be created."
-
 {{template "user_setup.sh.tmpl" .}}
 setup_user
 

pkg/templates/partials/user_invocation.sh.tmpl

@@ -2,6 +2,7 @@ CREATE_USER=${CREATE_USER:-ask}
 if [ "${OS_BUILD}" ] || [ "${CREATE_USER}" = true ]; then
   setup_user
 else
+  warn "To enable tunneling, the user ${_FIRST_USER_NAME} has to be created."
   if id -u "${_FIRST_USER_NAME}" >/dev/null 2>&1; then
     warn "The user ${_FIRST_USER_NAME} already exist! This operation will change the password and OVERWRITE ssh keys for this users."
   fi

pkg/templates/partials/user_setup.sh.tmpl

@@ -40,18 +40,65 @@ cp "${CONFIG_PATH}/rsa_private.pem" "/home/${_FIRST_USER_NAME}/.ssh/id_rsa"
 ssh-keygen -y -f "${CONFIG_PATH}/rsa_private.pem" > "/home/${_FIRST_USER_NAME}/.ssh/id_rsa.pub"
 cat "/home/${_FIRST_USER_NAME}/.ssh/id_rsa.pub" > "/home/${_FIRST_USER_NAME}/.ssh/authorized_keys"
 if [ -z "\${_FIRST_USER_KEY+x}" ] || [ "\${_FIRST_USER_KEY}" = "" ]; then
-  echo "[INFO] _FIRST_USER_KEY is not set, will continue with only password auth";
+  echo "[INFO] No extra SSH key for the user in the platform; the device key authorizes logins";
 else
   echo "\${_FIRST_USER_KEY}" >> "/home/${_FIRST_USER_NAME}/.ssh/authorized_keys"
 fi
 chmod 600 /home/${_FIRST_USER_NAME}/.ssh/*
 chmod 744 /home/${_FIRST_USER_NAME}/.ssh
 chown -R "${_FIRST_USER_NAME}:${_FIRST_USER_NAME}" "/home/${_FIRST_USER_NAME}"
 
-# Add root user to users and sudo groups
-if [ -e /etc/group ]; then
-    sed -i "/^users:/s/\(.*\)/\1,${_FIRST_USER_NAME}/;s/:,/:/" /etc/group
-    sed -i "/^sudo:/s/\(.*\)/\1,${_FIRST_USER_NAME}/;s/:,/:/" /etc/group
-fi
+# Add the user to the users and sudo groups (once: a re-run must not list it twice)
+for _tn_group in users sudo; do
+  if { getent group "\${_tn_group}" > /dev/null 2>&1 || grep -q "^\${_tn_group}:" /etc/group 2> /dev/null; } && ! id -nG "${_FIRST_USER_NAME}" | tr ' ' '\\n' | grep -qx "\${_tn_group}"; then
+    if command -v usermod > /dev/null 2>&1; then
+      usermod -aG "\${_tn_group}" "${_FIRST_USER_NAME}"
+    else
+      sed -i "/^\${_tn_group}:/s/\\(.*\\)/\\1,${_FIRST_USER_NAME}/;s/:,/:/" /etc/group
+    fi
+  fi
+done
 EOF
+
+  setup_user_sudo
+}
+
+# A key-only platform user (no password in the platform) cannot answer sudo's
+# password prompt, so automation over SSH (Ansible become, tnctl) could never
+# act as root. Give exactly that user passwordless sudo with a validated
+# drop-in. With a password set, the drop-in is removed again and sudo asks for
+# the password. The rule names the user by uid (#<uid>), so no user name can
+# turn into a sudoers keyword, and the file name has no '.' (sudo's includedir
+# skips such files). Drop-ins of earlier platform users are removed.
+setup_user_sudo() {
+  _tn_sudoers="/etc/sudoers.d/90-teknoir-platform-user"
+  # find, not a glob: the bootstrap scripts run with noglob
+  if [ -d /etc/sudoers.d ]; then
+    $SUDO find /etc/sudoers.d -maxdepth 1 -type f -name '90-teknoir-*' ! -name '90-teknoir-platform-user' -exec rm -f {} \;
+  fi
+  if [ -n "${_FIRST_USER_PASS}" ]; then
+    $SUDO rm -f "${_tn_sudoers}"
+    return 0
+  fi
+  if [ ! -d /etc/sudoers.d ]; then
+    warn "No /etc/sudoers.d: ${_FIRST_USER_NAME} gets no passwordless sudo"
+    return 0
+  fi
+  _tn_uid=$(id -u "${_FIRST_USER_NAME}" 2> /dev/null) || _tn_uid=""
+  case "${_tn_uid}" in
+    ''|*[!0-9]*|0)
+      warn "No usable uid for ${_FIRST_USER_NAME}: no passwordless sudo"
+      return 0
+      ;;
+  esac
+  _tn_tmp=$($SUDO mktemp /etc/sudoers.d/.teknoir.XXXXXX) || { warn "Cannot create a sudoers drop-in: no passwordless sudo for ${_FIRST_USER_NAME}"; return 0; }
+  if ! echo "#${_tn_uid} ALL=(ALL:ALL) NOPASSWD: ALL" | $SUDO tee "${_tn_tmp}" > /dev/null \
+    || ! $SUDO chmod 0440 "${_tn_tmp}" \
+    || { command -v visudo > /dev/null 2>&1 && ! $SUDO visudo -cf "${_tn_tmp}" > /dev/null; }; then
+    $SUDO rm -f "${_tn_tmp}"
+    warn "The sudoers drop-in for ${_FIRST_USER_NAME} could not be written or did not validate; no passwordless sudo"
+    return 0
+  fi
+  $SUDO mv -f "${_tn_tmp}" "${_tn_sudoers}"
+  info "Key-only login: ${_FIRST_USER_NAME} has passwordless sudo (${_tn_sudoers})"
 }

pkg/templates/render.go

@@ -7,6 +7,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io/fs"
+	"regexp"
 	"strings"
 )
 
@@ -21,6 +22,12 @@ var allTemplatesFS embed.FS
 // characters would allow quoting breakouts or expansion.
 const unsafeChars = "\"`$\\"
 
+// loginNameRe is the device login name the scripts accept: a lowercase POSIX
+// login name (Debian's default NAME_REGEX, at most 32 characters). It keeps
+// sudoers keywords such as ALL and names with '.' (which sudo's includedir
+// skips) out of the sudoers drop-in.
+var loginNameRe = regexp.MustCompile(`^[a-z_][a-z0-9_-]{0,31}$`)
+
 type field struct {
 	name      string
 	value     string
@@ -54,6 +61,9 @@ func ValidateTemplateData(d TemplateData) error {
 			return fmt.Errorf("template data field %s must not contain a newline", f.name)
 		}
 	}
+	if d.Username != "" && (!loginNameRe.MatchString(d.Username) || d.Username == "root") {
+		return fmt.Errorf("template data field Username must be a lowercase login name (a-z, 0-9, _ and -, starting with a letter or _, at most 32 characters, not root)")
+	}
 	return nil
 }
 

pkg/templates/render_test.go

@@ -32,6 +32,11 @@ func TestGolden_Execute(t *testing.T) {
 			if err := tt.exec(&buf); err != nil {
 				t.Fatal(err)
 			}
+			if os.Getenv("UPDATE_GOLDEN") == "1" {
+				if err := os.WriteFile("testdata/"+tt.name+".golden.sh", buf.Bytes(), 0o644); err != nil {
+					t.Fatal(err)
+				}
+			}
 			if want := readGolden(t, tt.name); !bytes.Equal(buf.Bytes(), want) {
 				t.Fatalf("%s output differs from testdata/%s.golden.sh (got %d bytes, want %d)", tt.name, tt.name, buf.Len(), len(want))
 			}
@@ -103,6 +108,24 @@ func TestValidateTemplateData_Rejects(t *testing.T) {
 	}
 }
 
+func TestValidateTemplateData_Username(t *testing.T) {
+	for _, u := range []string{"teknoir", "golden-user", "_svc", "pi", "a1_b-c"} {
+		d := goldenData()
+		d.Username = u
+		if err := ValidateTemplateData(d); err != nil {
+			t.Errorf("%q rejected: %v", u, err)
+		}
+	}
+	// ALL would be a sudoers keyword, '.' makes sudo skip the drop-in, root is never the platform user
+	for _, u := range []string{"ALL", "Teknoir", "first.last", "root", "-x", "1user", "a b", "user~", strings.Repeat("a", 33)} {
+		d := goldenData()
+		d.Username = u
+		if err := ValidateTemplateData(d); err == nil || !strings.Contains(err.Error(), "Username") {
+			t.Errorf("%q accepted or error does not name the field: %v", u, err)
+		}
+	}
+}
+
 func TestTemplatesSHA256(t *testing.T) {
 	a, b := TemplatesSHA256(), TemplatesSHA256()
 	if a != b || len(a) != 64 {

pkg/templates/server.sh.tmpl

@@ -467,8 +467,6 @@ info "Device bootstrapped successfully!"
 info "Notice!"
 info "No networking or user settings has been changed to match settings in the platform."
 
-warn "To enable tunneling, the user ${_FIRST_USER_NAME} has to be created."
-
 {{template "user_setup.sh.tmpl" .}}
 
 {{template "user_invocation.sh.tmpl" .}}