fc: give every microVM its own guest hostname - #1074
Conversation
…ault debootstrap runs in a chroot that can't isolate the UTS namespace, so the baked Firecracker rootfs image always carries one fixed placeholder hostname (bake-fc-image.sh writes "onctl-runner" into it). Every VM booted from that image reported the same guest hostname, regardless of its actual onctl VM name — noticeable via `hostname`/`uname -n` inside the guest, and in anything that shells out and reads it (e.g. a GitHub Actions runner's "Machine name" in its job log, previously patched around per-caller in onctl-runners' own bootstrap script rather than fixed here). DebugfsRootfsPreparer.Prepare now takes a hostname and, when non-empty, rewrites /etc/hostname in the per-VM rootfs copy via the same debugfs-without-mounting technique already used for SSH key injection (factored the shared "write script, run debugfs -w" bit into runDebugfsScript). ProviderFC.Deploy passes server.Name through sanitizeGuestHostname (safe hostname charset, 63-byte RFC 1123 cap) — needed both because arbitrary VM names aren't guaranteed hostname-safe (fcTapName/fcMAC already hash the name for the same reason) and because the value lands in a debugfs script verbatim, so an unsanitized name could inject extra script commands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TB8wdxuaLyoYZBkfcD1vzv
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1167fb20ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
sanitizeGuestHostname previously truncated overlong names to a flat 63-byte prefix, so two VM names sharing the same first 63 sanitized bytes (e.g. a controller-generated run ID appended after a long shared prefix) collided on the same guest hostname. Reserve the last 9 bytes for a hyphen and an 8-hex-char hash of the full name so truncated names stay distinct. Addresses a Codex review comment on PR #1074. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SKUJRDCaXe4RnU6BXLatcs
|
Generated by Claude Code |
2 participants
Summary
onctl-runner), regardless of its actual VM name — becausebake-fc-image.sh's debootstrap runs in a chroot that can't isolate the UTS namespace, so it bakes one fixed placeholder hostname into the base rootfs image, and nothing at VM-creation time overrode it per-VM.DebugfsRootfsPreparer.Preparenow takes ahostnameparameter and, when non-empty, rewrites/etc/hostnamein the per-VM writable rootfs copy — same "edit the ext4 image viadebugfs -w, no mount, no root needed" technique already used for SSH-key injection. Shared the "build script, rundebugfs -w" plumbing into a newrunDebugfsScripthelper.ProviderFC.Deploypassesserver.Namethrough a newsanitizeGuestHostname(lowercase,[a-z0-9-]only, 63-byte RFC 1123 cap) before handing it toPrepare— needed because arbitrary VM names aren't guaranteed hostname-safe (fcTapName/fcMACalready hash the name for the same reason), and because this value is written into a debugfs script verbatim, so an unsanitized name could otherwise inject extra script commands.onctl createrun by hand, the sandboxes feature, etc.) now gets a guest hostname matching its actual VM name.Test plan
make(build +go mod tidy+go fmt) — cleango test ./pkg/cloud/... ./internal/providerfc/...— all pass, including new tests:TestDebugfsRootfsPreparer_Prepare_InjectsHostname,_NoHostname,_HostnameDebugfsFails, andTestSanitizeGuestHostname(charset/case/length edge cases)golangci-lint run ./pkg/cloud/... ./internal/providerfc/...— 0 issuesgo vet ./...,gofmt -lon changed files — cleango test ./cmd/...has 3 pre-existing failures (TestReadConfig_NoConfigDirectory,TestActionCmd_DownloadFile_404,TestCreateFlagsBindToViper) reproduced identically on unmodifiedorigin/main— unrelated to this change, not touched by this PR🤖 Generated with Claude Code
https://claude.ai/code/session_01TB8wdxuaLyoYZBkfcD1vzv