refactor(proxmox): extract sessionRunner seam for testability (T01.1, REQ-057)

---ci---
project: orca
phase: 1
milestone: v0.8
status: execute
---/ci---
This commit is contained in:
Jon Chery
2026-08-04 00:51:15 +00:00
parent 97a10353da
commit 2786de166d
3 changed files with 82 additions and 40 deletions
+44 -26
View File
@@ -143,6 +143,10 @@ func BootstrapProxmox(ctx context.Context, opts Options) (*Result, error) {
}
defer conn.Close()
if sessionRunner == nil {
sessionRunner = &sshSessionRunner{client: conn}
}
log.Info("proxmox.ssh_connected",
slog.String("event", "proxmox.ssh_connected"),
slog.String("host", opts.Host),
@@ -150,38 +154,38 @@ func BootstrapProxmox(ctx context.Context, opts Options) (*Result, error) {
)
// Step 3: Deploy orca pubkey to ~orca/.ssh/authorized_keys (idempotent).
if err := deployPubKey(conn, opts.ProxmoxUser, string(pubLine)); err != nil {
if err := deployPubKey(opts.ProxmoxUser, string(pubLine)); err != nil {
return nil, fmt.Errorf("deploy pubkey: %w", err)
}
// Step 4: Create orca Linux system user (idempotent).
if err := createLinuxUser(conn, opts.ProxmoxUser); err != nil {
if err := createLinuxUser(opts.ProxmoxUser); err != nil {
return nil, fmt.Errorf("create user %s: %w", opts.ProxmoxUser, err)
}
// Step 5: Create OrcaOperator PVE role (idempotent).
if err := createPVERole(conn, opts.ProxmoxRole); err != nil {
if err := createPVERole(opts.ProxmoxRole); err != nil {
return nil, fmt.Errorf("create PVE role %s: %w", opts.ProxmoxRole, err)
}
// Step 6: Create orca@pam PVE user (idempotent).
if err := createPVEUser(conn, opts.ProxmoxUser); err != nil {
if err := createPVEUser(opts.ProxmoxUser); err != nil {
return nil, fmt.Errorf("create PVE user %s@pam: %w", opts.ProxmoxUser, err)
}
// Step 7: Assign OrcaOperator role to orca@pam on path / (idempotent).
if err := assignPVEACL(conn, opts.ProxmoxUser, opts.ProxmoxRole); err != nil {
if err := assignPVEACL(opts.ProxmoxUser, opts.ProxmoxRole); err != nil {
return nil, fmt.Errorf("assign ACL: %w", err)
}
// Step 8: Write /etc/sudoers.d/orca (AD-020: NOEXEC on pct/qm,
// no NOEXEC on apt-get/dpkg, pvesh EXCLUDED).
if err := writeSudoers(conn, opts.ProxmoxUser); err != nil {
if err := writeSudoers(opts.ProxmoxUser); err != nil {
return nil, fmt.Errorf("write sudoers: %w", err)
}
// Step 9: Validate sudoers with visudo -cf.
if err := validateSudoers(conn); err != nil {
if err := validateSudoers(); err != nil {
return nil, fmt.Errorf("validate sudoers: %w", err)
}
@@ -212,15 +216,29 @@ func (defaultSSHDialer) DialContext(ctx context.Context, network, addr string, c
return ssh.Dial(network, addr, config)
}
// runRemote runs a command over the SSH connection and returns its
// combined output. Returns an error if the command exits non-zero.
func runRemote(conn *ssh.Client, cmd string) ([]byte, error) {
session, err := conn.NewSession()
type sessionRunnerType interface {
CombinedOutput(cmd string) ([]byte, error)
}
var sessionRunner sessionRunnerType
type sshSessionRunner struct {
client *ssh.Client
}
func (r *sshSessionRunner) CombinedOutput(cmd string) ([]byte, error) {
session, err := r.client.NewSession()
if err != nil {
return nil, fmt.Errorf("new session: %w", err)
}
defer session.Close()
out, err := session.CombinedOutput(cmd)
return session.CombinedOutput(cmd)
}
// runRemote runs a command over the SSH connection and returns its
// combined output. Returns an error if the command exits non-zero.
func runRemote(cmd string) ([]byte, error) {
out, err := sessionRunner.CombinedOutput(cmd)
if err != nil {
return out, fmt.Errorf("run %q: %w (output: %s)", cmd, err, strings.TrimSpace(string(out)))
}
@@ -230,7 +248,7 @@ func runRemote(conn *ssh.Client, cmd string) ([]byte, error) {
// deployPubKey appends the orca public key to the remote user's
// authorized_keys file, creating the .ssh dir if needed. Idempotent:
// if the key is already present, it is not re-appended.
func deployPubKey(conn *ssh.Client, user, pubLine string) error {
func deployPubKey(user, pubLine string) error {
pubLine = strings.TrimSpace(pubLine)
if pubLine == "" {
return fmt.Errorf("deployPubKey: empty pub line")
@@ -246,7 +264,7 @@ func deployPubKey(conn *ssh.Client, user, pubLine string) error {
"mkdir -p %s && touch %s && chmod 0700 %s && chmod 0600 %s && grep -qF '%s' %s || echo '%s' >> %s",
sshDir, authFile, sshDir, authFile, pubLine, authFile, pubLine, authFile,
)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -254,9 +272,9 @@ func deployPubKey(conn *ssh.Client, user, pubLine string) error {
// createLinuxUser creates the orca system user if it doesn't already
// exist. Idempotent: `id -u` check before `useradd`.
func createLinuxUser(conn *ssh.Client, user string) error {
func createLinuxUser(user string) error {
cmd := fmt.Sprintf("id -u %s 2>/dev/null || useradd -m -s /bin/bash %s", user, user)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -264,12 +282,12 @@ func createLinuxUser(conn *ssh.Client, user string) error {
// createPVERole creates the OrcaOperator PVE role if it doesn't exist.
// Idempotent: probes `pveum role list` before `pveum role add`.
func createPVERole(conn *ssh.Client, role string) error {
func createPVERole(role string) error {
cmd := fmt.Sprintf(
"pveum role list 2>/dev/null | grep -q '^%s' || pveum role add %s --privs '%s'",
role, role, OrcaOperatorPrivileges,
)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -278,13 +296,13 @@ func createPVERole(conn *ssh.Client, role string) error {
// createPVEUser creates the orca@pam PVE user if it doesn't exist.
// Idempotent: probes `pveum user list` before `pveum user add`.
// Uses @pam realm (AD-019) since orca creates a Linux system user.
func createPVEUser(conn *ssh.Client, user string) error {
func createPVEUser(user string) error {
pveUserID := user + "@pam"
cmd := fmt.Sprintf(
"pveum user list 2>/dev/null | grep -q '%s' || pveum user add %s -comment 'Orca automation user'",
pveUserID, pveUserID,
)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -292,10 +310,10 @@ func createPVEUser(conn *ssh.Client, user string) error {
// assignPVEACL assigns the OrcaOperator role to orca@pam on path /
// (cluster-wide). `pveum acl modify` is idempotent (creates or updates).
func assignPVEACL(conn *ssh.Client, user, role string) error {
func assignPVEACL(user, role string) error {
pveUserID := user + "@pam"
cmd := fmt.Sprintf("pveum acl modify / -user %s -role %s", pveUserID, role)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -319,12 +337,12 @@ func sudoersContent(user string) string {
// writeSudoers writes the /etc/sudoers.d/orca file on the remote host
// with mode 0440. Uses a heredoc via cat to avoid quoting issues.
func writeSudoers(conn *ssh.Client, user string) error {
func writeSudoers(user string) error {
content := sudoersContent(user)
// Write via cat heredoc, then chmod 0440.
cmd := fmt.Sprintf("cat > /etc/sudoers.d/%s <<'ORCA_SUDOERS_EOF'\n%s\nORCA_SUDOERS_EOF\nchmod 0440 /etc/sudoers.d/%s",
user, content, user)
if _, err := runRemote(conn, cmd); err != nil {
if _, err := runRemote(cmd); err != nil {
return err
}
return nil
@@ -333,9 +351,9 @@ func writeSudoers(conn *ssh.Client, user string) error {
// validateSudoers runs `visudo -cf` on the sudoers file. Aborts the
// bootstrap if validation fails (prevents a broken sudoers from
// locking the orca user out of sudo).
func validateSudoers(conn *ssh.Client) error {
func validateSudoers() error {
cmd := "visudo -cf /etc/sudoers.d/orca"
out, err := runRemote(conn, cmd)
out, err := runRemote(cmd)
if err != nil {
return fmt.Errorf("visudo validation failed: %w (output: %s)", err, strings.TrimSpace(string(out)))
}
+2 -2
View File
@@ -315,7 +315,7 @@ func TestBootstrapProxmox_ContextCancelled(t *testing.T) {
}
func TestDeployPubKey_EmptyPubLine(t *testing.T) {
err := deployPubKey(nil, "orca", "")
err := deployPubKey("orca", "")
if err == nil {
t.Error("expected error for empty pub line")
}
@@ -325,7 +325,7 @@ func TestDeployPubKey_EmptyPubLine(t *testing.T) {
}
func TestDeployPubKey_WhitespaceOnlyPubLine(t *testing.T) {
err := deployPubKey(nil, "orca", " \n \t ")
err := deployPubKey("orca", " \n \t ")
if err == nil {
t.Error("expected error for whitespace-only pub line")
}
+36 -12
View File
@@ -238,12 +238,20 @@ func fakeSSHClient(t *testing.T, srv *fakeSSHServer) *ssh.Client {
return client
}
func withSessionRunner(t *testing.T, conn *ssh.Client) {
t.Helper()
orig := sessionRunner
t.Cleanup(func() { sessionRunner = orig })
sessionRunner = &sshSessionRunner{client: conn}
}
func TestRunRemote_Success(t *testing.T) {
srv := newFakeSSHServer(t)
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
out, err := runRemote(conn, "echo hello")
withSessionRunner(t, conn)
out, err := runRemote("echo hello")
if err != nil {
t.Fatalf("runRemote: %v", err)
}
@@ -257,7 +265,8 @@ func TestRunRemote_Failure(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
_, err := runRemote(conn, "exit 7")
withSessionRunner(t, conn)
_, err := runRemote("exit 7")
if err == nil {
t.Fatal("expected error for non-zero exit")
}
@@ -271,8 +280,9 @@ func TestDeployPubKey_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
withSessionRunner(t, conn)
if err := deployPubKey(conn, "orca", "ssh-ed25519 AAAA test@orca"); err != nil {
if err := deployPubKey("orca", "ssh-ed25519 AAAA test@orca"); err != nil {
t.Fatalf("deployPubKey: %v", err)
}
out := srv.readFile(filepath.Join(srv.authDir, "authorized_keys"))
@@ -286,11 +296,12 @@ func TestDeployPubKey_Idempotent(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
withSessionRunner(t, conn)
if err := deployPubKey(conn, "orca", "ssh-ed25519 AAAA test@orca"); err != nil {
if err := deployPubKey("orca", "ssh-ed25519 AAAA test@orca"); err != nil {
t.Fatalf("first deploy: %v", err)
}
if err := deployPubKey(conn, "orca", "ssh-ed25519 AAAA test@orca"); err != nil {
if err := deployPubKey("orca", "ssh-ed25519 AAAA test@orca"); err != nil {
t.Fatalf("second deploy: %v", err)
}
out := srv.readFile(filepath.Join(srv.authDir, "authorized_keys"))
@@ -304,7 +315,8 @@ func TestCreateLinuxUser_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
if err := createLinuxUser(conn, "orca"); err != nil {
withSessionRunner(t, conn)
if err := createLinuxUser("orca"); err != nil {
t.Fatalf("createLinuxUser: %v", err)
}
}
@@ -314,7 +326,8 @@ func TestCreatePVERole_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
if err := createPVERole(conn, "OrcaOperator"); err != nil {
withSessionRunner(t, conn)
if err := createPVERole("OrcaOperator"); err != nil {
t.Fatalf("createPVERole: %v", err)
}
}
@@ -324,7 +337,8 @@ func TestCreatePVEUser_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
if err := createPVEUser(conn, "orca"); err != nil {
withSessionRunner(t, conn)
if err := createPVEUser("orca"); err != nil {
t.Fatalf("createPVEUser: %v", err)
}
}
@@ -334,7 +348,8 @@ func TestAssignPVEACL_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
if err := assignPVEACL(conn, "orca", "OrcaOperator"); err != nil {
withSessionRunner(t, conn)
if err := assignPVEACL("orca", "OrcaOperator"); err != nil {
t.Fatalf("assignPVEACL: %v", err)
}
}
@@ -344,8 +359,9 @@ func TestWriteSudoers_Success(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
withSessionRunner(t, conn)
if err := writeSudoers(conn, "orca"); err != nil {
if err := writeSudoers("orca"); err != nil {
t.Fatalf("writeSudoers: %v", err)
}
if srv.state["sudoers_valid"] != "true" {
@@ -361,9 +377,10 @@ func TestValidateSudoers_ParsedOK(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
withSessionRunner(t, conn)
srv.state["sudoers_valid"] = "true"
if err := validateSudoers(conn); err != nil {
if err := validateSudoers(); err != nil {
t.Errorf("validateSudoers: %v", err)
}
}
@@ -373,9 +390,10 @@ func TestValidateSudoers_Failure(t *testing.T) {
defer srv.close()
conn := fakeSSHClient(t, srv)
defer conn.Close()
withSessionRunner(t, conn)
srv.state["sudoers_valid"] = "false"
if err := validateSudoers(conn); err == nil {
if err := validateSudoers(); err == nil {
t.Error("expected error for invalid sudoers")
}
}
@@ -400,6 +418,9 @@ func TestBootstrapProxmox_FullFlow_Success(t *testing.T) {
orig := sshDialer
defer func() { sshDialer = orig }()
origRunner := sessionRunner
defer func() { sessionRunner = origRunner }()
sessionRunner = nil
sshDialer = &staticDialer{client: fakeSSHClient(t, srv)}
host, _, _ := net.SplitHostPort(srv.addr())
@@ -439,6 +460,9 @@ func TestBootstrapProxmox_FullFlow_DeployPubKeyFails(t *testing.T) {
orig := sshDialer
defer func() { sshDialer = orig }()
origRunner := sessionRunner
defer func() { sessionRunner = origRunner }()
sessionRunner = nil
// Use a real client that connects to a server which will reject deploy
// by returning a non-zero exit for the mkdir command. We achieve this