Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mcp/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func toolCreateSandbox(ctx context.Context, s *server, raw jsontext.Value) (stri
if args.TTLSeconds < 0 {
return "", fmt.Errorf("ttl_seconds must not be negative, got %d", args.TTLSeconds)
}
// an agent session outlives the node's 5-minute default, and nothing renews a lease
// An agent session outlives the node's 5-minute default, and nothing renews a lease
ttl := cmp.Or(time.Duration(args.TTLSeconds)*time.Second, defaultToolTTL)
opts := []sandbox.Option{sandbox.WithTimeout(ttl)}
if args.Net != "" {
Expand Down
6 changes: 3 additions & 3 deletions sandboxd/egress/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func LoadCA(rootCertPEM, interCertPEM, interKeyPEM []byte) (*CA, error) {
if err = inter.CheckSignatureFrom(root); err != nil {
return nil, fmt.Errorf("intermediate not signed by root: %w", err)
}
// reject a self-signed intermediate: it would put the root key on the node.
// Reject a self-signed intermediate: it would put the root key on the node.
if inter.CheckSignatureFrom(inter) == nil {
return nil, fmt.Errorf("intermediate must not be a self-signed root")
}
Expand All @@ -68,7 +68,7 @@ func LoadCA(rootCertPEM, interCertPEM, interKeyPEM []byte) (*CA, error) {
if err != nil {
return nil, fmt.Errorf("generate leaf key: %w", err)
}
// fingerprint the whole file: CertPEM bakes it verbatim.
// Fingerprint the whole file: CertPEM bakes it verbatim.
sum := sha256.Sum256(rootCertPEM)
return &CA{
rootPEM: rootCertPEM,
Expand All @@ -93,7 +93,7 @@ func (c *CA) SignLeaf(host string) (*tls.Certificate, error) {
return nil, err
}
now := time.Now()
// guests reject a chain whose leaf outlives its intermediate.
// Guests reject a chain whose leaf outlives its intermediate.
notAfter := now.Add(leafValidity)
if notAfter.After(c.interCert.NotAfter) {
notAfter = c.interCert.NotAfter
Expand Down
6 changes: 3 additions & 3 deletions sandboxd/egress/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (p *Proxy) serveIntercept(w http.ResponseWriter, r *http.Request, host stri
if _, err := io.WriteString(client, "HTTP/1.1 200 Connection Established\r\n\r\n"); err != nil {
return
}
// a guest that never sends a ClientHello would pin this goroutine until the sandbox dies.
// A guest that never sends a ClientHello would pin this goroutine until the sandbox dies.
_ = client.SetDeadline(time.Now().Add(interceptTimeout))
tlsConn := tls.Server(utils.WithBuffered(client, brw.Reader), cfg)
if err := tlsConn.HandshakeContext(r.Context()); err != nil {
Expand All @@ -67,7 +67,7 @@ func (p *Proxy) serveIntercept(w http.ResponseWriter, r *http.Request, host stri
func (p *Proxy) leafFor(host string) (*tls.Certificate, error) {
p.leafMu.Lock()
defer p.leafMu.Unlock()
// a leaf clamped to the intermediate's own end has nothing to renew into
// A leaf clamped to the intermediate's own end has nothing to renew into
if crt, ok := p.leaves[host]; ok && (time.Now().Before(crt.Leaf.NotAfter.Add(-leafRenewBefore)) || !crt.Leaf.NotAfter.Before(p.ca.interCert.NotAfter)) {
return crt, nil
}
Expand Down Expand Up @@ -97,7 +97,7 @@ func (h *interceptHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
p.relay(w, r, ev, rule, route, true, func(out *http.Request) {
out.URL.Scheme = "https"
out.URL.Host = h.authority
// keep the guest's Host when it names the CONNECT host: SigV4 signing breaks on a rewrite.
// Keep the guest's Host when it names the CONNECT host: SigV4 signing breaks on a rewrite.
if inner, _, _ := hostPort(r.Host, 0); !strings.EqualFold(inner, h.host) {
out.Host = h.authority
}
Expand Down
6 changes: 3 additions & 3 deletions sandboxd/egress/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func New(policy Evaluator, secrets Secrets, ca *CA, router Router, audit AuditFu
holder: holder,
conns: map[net.Conn]struct{}{},
}
// the stdlib default of 2 idle conns per host re-dials bursty same-host traffic.
// The stdlib default of 2 idle conns per host re-dials bursty same-host traffic.
pools := &connPools{tr: &http.Transport{DialContext: direct, MaxIdleConns: maxIdleConns, MaxIdleConnsPerHost: 8, IdleConnTimeout: idleConnTimeout}, routed: map[routeKey]*http.Transport{}}
if ca != nil {
pools.mitm = pools.tr.Clone()
Expand Down Expand Up @@ -314,7 +314,7 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) {

// tunnelDecision treats an inject rule as a plain one when injects is false.
func (p *Proxy) tunnelDecision(host string, port uint16, injects bool) (decision Decision, intercept bool) {
// host-gate interception: the tunnel's CONNECT verb is not the request method.
// Host-gate interception: the tunnel's CONNECT verb is not the request method.
if p.ca != nil {
if rule, d := p.policy.EvalHost(host, port); d == DecisionAllow &&
(rule.Intercept == InterceptAlways || rule.Intercept == InterceptInject && injects && p.holdsCredential(host)) {
Expand Down Expand Up @@ -374,7 +374,7 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul
var sc *scrubber
ev.Injected, sc = p.inject(rule, out, ev.Host, mitm)
if sc != nil {
// the scrub needs the whole body uncompressed.
// The scrub needs the whole body uncompressed.
for _, h := range []string{"Accept-Encoding", "Range", "If-Range"} {
out.Header.Del(h)
}
Expand Down
4 changes: 2 additions & 2 deletions sandboxd/engine/installca.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (e *Engine) silkdWriteFile(ctx context.Context, vsockSocket, path string, m
return err
}
defer s.close()
// a send racing the guest's early answer+close hits EPIPE; prefer the buffered verdict.
// A send racing the guest's early answer+close hits EPIPE; prefer the buffered verdict.
sendErr := func() error {
if serr := s.send(wire.FsWrite{Path: path, Mode: &mode}); serr != nil {
return serr
Expand Down Expand Up @@ -85,7 +85,7 @@ func (e *Engine) silkdOutput(ctx context.Context, vsockSocket string, argv ...st
defer s.close()
sendErr := s.send(wire.Exec{Argv: argv, Env: map[string]string{"PATH": guestExecPATH}})
if sendErr == nil {
// a child exiting without reading stdin races this close; prefer the buffered exit frame.
// A child exiting without reading stdin races this close; prefer the buffered exit frame.
sendErr = s.send(wire.StdinClose{})
}
var out, stdout []byte
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/engine/portconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (g *guestPortConn) Write(p []byte) (int, error) {
written := 0
for len(p) > 0 {
n := min(len(p), wire.PortWriteChunk)
// the hot relay path reuses one buffer instead of allocating per chunk.
// The hot relay path reuses one buffer instead of allocating per chunk.
g.wbuf = wire.AppendBulkRequest(g.wbuf, "data", p[:n])
if _, err := g.Conn.Write(g.wbuf); err != nil {
return written, err
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/engine/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (e *Engine) findVolumeDevice(ctx context.Context, vsockSocket, name string,
continue
}
device := "/dev/" + entry.Name
// the kernel publishes /sys/block before devtmpfs creates the node.
// The kernel publishes /sys/block before devtmpfs creates the node.
if err := e.silkdOnce[*wire.Stat](ctx, vsockSocket, wire.FsStat{Path: device}); err != nil {
if isNotFound(err) {
return "", false, nil
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/outbound/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func newDialer(verdict VerdictFunc) *net.Dialer {
// destVerdict reports whether ip is an internal address allow re-admits; any other internal address, self included, is an error.
func destVerdict(allow []egress.InternalAllow, self []netip.Addr, ip netip.Addr, port uint16) (bool, error) {
ip = canonicalAddr(ip)
// after the NAT64 unwrap and before the block, so a named prefix wins
// After the NAT64 unwrap and before the block, so a named prefix wins
if slices.ContainsFunc(allow, func(a egress.InternalAllow) bool { return a.Admits(ip, port) }) {
return true, nil
}
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/outbound/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (h *Host) ArmProxy(ctx context.Context, sb *types.Sandbox) error {
d := h.prebound[sb.VMName]
delete(h.prebound, sb.VMName)
h.mu.Unlock()
// only an intercepting pool pays for the TLS transport and leaf cache
// Only an intercepting pool pays for the TLS transport and leaf cache
ca := h.o.CA
if !v.Intercepts(sb.PolicyKey()) {
ca = nil
Expand Down
20 changes: 10 additions & 10 deletions sandboxd/pool/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (m *Manager) archiveOnce(ctx context.Context) {
}

func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error {
// must finish even if the sweep ctx is canceled, or the record diverges from the store.
// Must finish even if the sweep ctx is canceled, or the record diverges from the store.
ctx = context.WithoutCancel(ctx)
defer m.untrack(m.archiving, sb.ID)
m.mu.Lock()
Expand All @@ -98,18 +98,18 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error {
if err = m.checkArchiveRow(ctx, sb.ID); err != nil {
return fmt.Errorf("archive %s: registry: %w", sb.ID, err)
}
// pin the ck before Publish makes it listable, or a delete would strand the claim
// Pin the ck before Publish makes it listable, or a delete would strand the claim
ckID := store.CheckpointID(randHex(8))
m.mu.Lock()
m.pendingCks[ckID] = struct{}{}
m.mu.Unlock()
defer m.untrack(m.pendingCks, ckID)
// a hibernated source is copied from its wake image, so no VM starts.
// A hibernated source is copied from its wake image, so no VM starts.
ck, srcSnap, err := m.publishCheckpoint(ctx, sb, ckID, "archive", sb.Tenant, true)
if err != nil {
return err
}
// the claim must still hold the exact snapshot the export captured, or the ck is stale
// The claim must still hold the exact snapshot the export captured, or the ck is stale
sb.Transition.Lock()
m.mu.Lock()
if m.claimed[sb.ID] != sb || sb.HibernateSnap != srcSnap || sb.ArchiveCk != "" {
Expand Down Expand Up @@ -152,7 +152,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error {
if saveErr := m.store.commit(js); saveErr != nil {
m.mu.Lock()
var rb claimSnapshot
// a Release that landed meanwhile already deleted the claim; re-setting it would resurrect it
// A Release that landed meanwhile already deleted the claim; re-setting it would resurrect it
if m.claimed[sb.ID] == sb {
sb.ArchiveCk, sb.ArchiveEpoch, sb.ArchivePending, sb.ReplicaCk = "", 0, prevPending, replica
sb.HibernateSnap, sb.VsockSocket, sb.VMName = snap, sock, vmName
Expand All @@ -167,10 +167,10 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error {
}
m.counters.archives.Add(1)
m.recordUsage(ctx, usage.Event{Event: "archive", ID: sb.ID, Reference: ck.ID, Tenant: sb.Tenant, Deadline: sb.Deadline})
// disarm under Transition so a wake right after the release is not clobbered by a late disarm
// Disarm under Transition so a wake right after the release is not clobbered by a late disarm
m.out.Disarm(sb.ID, true)
sb.Transition.Unlock()
// the store ck is authoritative from here
// The store ck is authoritative from here
m.destroy(ctx, vmName)
m.dropSnap(ctx, snap)
if replica != "" {
Expand All @@ -181,7 +181,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error {

// Caller holds Transition.
func (m *Manager) wakeArchived(ctx context.Context, sb *types.Sandbox) (string, error) {
// egress never archives; a corrupt archived egress claim must fail closed.
// Egress never archives; a corrupt archived egress claim must fail closed.
if sb.Key.Net == types.NetEgress {
return "", fmt.Errorf("wake %s: egress lane cannot resume from archive", sb.ID)
}
Expand Down Expand Up @@ -227,7 +227,7 @@ func (m *Manager) wakeArchived(ctx context.Context, sb *types.Sandbox) (string,
return "", fmt.Errorf("wake %s: persist claims", sb.ID)
}
logger := log.WithFunc("pool.wakeArchived")
// the pre-commit marker keeps a failed delete retryable after the journal update.
// The pre-commit marker keeps a failed delete retryable after the journal update.
if delErr := m.ckpts.Delete(ctx, ck); delErr != nil {
logger.Warnf(ctx, "delete consumed archive ck %s: %v", ck, delErr)
} else {
Expand All @@ -236,7 +236,7 @@ func (m *Manager) wakeArchived(ctx context.Context, sb *types.Sandbox) (string,
}
m.dropTombstone(ctx, sb.ID, consumed)
}
// only the none lane reaches here, so there is no NIC to re-lock
// Only the none lane reaches here, so there is no NIC to re-lock
if proxyErr := m.out.ArmProxy(ctx, sb); proxyErr != nil {
logger.Errorf(ctx, proxyErr, "arm egress proxy %s", sb.ID)
}
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/pool/capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir
}
defer cleanup()
vmName, guestEnv = sb.VMName, m.heldGuestEnv(sb)
// a fresh snapshot is this capture's alone, so dials need not wait out its export; a wake consumes a hibernate snapshot
// A fresh snapshot is this capture's alone, so dials need not wait out its export; a wake consumes a hibernate snapshot
if snap != sb.HibernateSnap {
sb.Transition.Unlock()
} else {
Expand Down
12 changes: 6 additions & 6 deletions sandboxd/pool/checkpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (m *Manager) Checkpoint(ctx context.Context, id string, cred Cred, name, te

// ClaimCheckpoint provisions a fresh claim cloned from a checkpoint, attributed to tenant.
func (m *Manager) ClaimCheckpoint(ctx context.Context, ckptID string, o ClaimOptions) (*types.Sandbox, error) {
// a rejected id must not leave a lock-map entry, so resolve before recLock and quota
// A rejected id must not leave a lock-map entry, so resolve before recLock and quota
ckpt, err := m.loadCheckpoint(ctx, ckptID)
if err != nil {
return nil, err
Expand All @@ -76,7 +76,7 @@ func (m *Manager) ClaimCheckpointHeal(ctx context.Context, ckptID string, o Clai
if m.healer == nil || !store.CheckpointIDRe.MatchString(ckptID) {
return nil, ErrUnknownCheckpoint
}
// resolving moves a guest memory image, so quota rejects a full node before that cost
// Resolving moves a guest memory image, so quota rejects a full node before that cost
if err := m.overQuota(1, o.Tenant); err != nil {
return nil, err
}
Expand Down Expand Up @@ -130,7 +130,7 @@ func (m *Manager) DeleteCheckpoint(ctx context.Context, ckptID, tenant string, s
if err := m.ckpts.Delete(ctx, ckptID); err != nil {
return fmt.Errorf("delete checkpoint: %w", err)
}
// a shared backend has no per-node replicas to chase.
// A shared backend has no per-node replicas to chase.
if scope == DeleteFleet && m.peerDelete != nil && !m.ckptsShared {
m.peerDelete(context.WithoutCancel(ctx), ckptID)
}
Expand All @@ -157,7 +157,7 @@ func (m *Manager) FetchCheckpoint(ctx context.Context, ckptID string) (string, [
}
return "", nil, nil, fmt.Errorf("fetch checkpoint: %w", err)
}
// the read lock spans the transfer so a delete cannot pull the export from under it
// The read lock spans the transfer so a delete cannot pull the export from under it
return dir, meta, unlock, nil
}

Expand Down Expand Up @@ -384,14 +384,14 @@ func validateHealedCheckpoint(staging, wantID string) error {
return fmt.Errorf("healed record %s has an invalid key: %w", wantID, keyErr)
}
if !ckpt.Key.Capturable() {
// publishing a non-branchable key would poison the id and suppress a good owner
// Publishing a non-branchable key would poison the id and suppress a good owner
return fmt.Errorf("healed record %s has a non-branchable (egress) key", wantID)
}
export, err := os.ReadDir(filepath.Join(staging, store.ExportDir))
if err != nil {
return fmt.Errorf("healed record %s missing export dir: %w", wantID, err)
}
// an export with no regular file clones to nothing; deeper content is cocoon's own format
// An export with no regular file clones to nothing; deeper content is cocoon's own format
if !hasRegularFile(export) {
return fmt.Errorf("healed record %s has no export content", wantID)
}
Expand Down
20 changes: 10 additions & 10 deletions sandboxd/pool/claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (m *Manager) ClaimWarm(ctx context.Context, key types.PoolKey, o ClaimOptio
if err != nil {
return nil, err
}
// holds belong to this path until finalize; past it the sandbox carries them.
// Holds belong to this path until finalize; past it the sandbox carries them.
reserved := applied
defer func() { m.unreserveVolumes(reserved) }()
m.mu.Lock()
Expand Down Expand Up @@ -176,7 +176,7 @@ func (m *Manager) Renew(ctx context.Context, id string, cred Cred, ttl time.Dura
m.mu.Unlock()
return time.Time{}, ErrUnknownSandbox
}
// an archived claim's Deadline is a retention window, not a lease: a TTL over it schedules a purge
// An archived claim's Deadline is a retention window, not a lease: a TTL over it schedules a purge
if _, archiving := m.archiving[id]; sb.ArchiveCk != "" || archiving {
m.mu.Unlock()
return time.Time{}, ErrArchived
Expand All @@ -188,7 +188,7 @@ func (m *Manager) Renew(ctx context.Context, id string, cred Cred, ttl time.Dura
if err := m.store.commit(js); err != nil {
m.mu.Lock()
var rb claimSnapshot
// a concurrent renew that landed keeps its grant: roll back only what this call wrote
// A concurrent renew that landed keeps its grant: roll back only what this call wrote
if m.claimed[id] == sb && sb.Deadline.Equal(deadline) {
sb.Deadline, sb.LeaseSeconds, sb.OnExpire = prev, prevLease, prevExpire
rb = m.store.set(sb)
Expand Down Expand Up @@ -218,7 +218,7 @@ func (m *Manager) AgentSocket(id, token string) (string, error) {
if sb.Replicated() && m.fence.Load().fenced {
return "", ErrLeaseLost
}
// no activity stamp: a control-plane poll must not keep an idle sandbox awake
// No activity stamp: a control-plane poll must not keep an idle sandbox awake
return sb.VsockSocket, nil
}

Expand Down Expand Up @@ -326,7 +326,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand
logger.Errorf(ctx, saveErr, "persist release of %s", id)
return fmt.Errorf("release %s: %w", id, saveErr)
}
// cleanup must survive the caller hanging up; the claim is already dropped.
// Cleanup must survive the caller hanging up; the claim is already dropped.
ctx = context.WithoutCancel(ctx)
if ck != "" {
m.purgeArchiveCk(ctx, id, ck, sb.Tenant) // archived: no local VM
Expand Down Expand Up @@ -466,7 +466,7 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t
if err != nil {
m.mu.Unlock()
for _, sb := range sbs {
// the rw mount dirtied the journal, so the clean umount is still owed
// The rw mount dirtied the journal, so the clean umount is still owed
td := m.quiesceVolumes(ctx, sb)
m.removeOrRetry(ctx, sb.VMName, "", td)
}
Expand All @@ -491,7 +491,7 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t
return fmt.Errorf("arm egress %s: %w", sb.ID, armErr)
}
}
// usage lands only after the batch armed, so a rollback leaves no unterminated claim event
// Usage lands only after the batch armed, so a rollback leaves no unterminated claim event
for _, sb := range sbs {
m.recordUsage(ctx, usage.Event{
Event: "claim",
Expand Down Expand Up @@ -543,7 +543,7 @@ func (m *Manager) reapOnce(ctx context.Context) {
var dropped, purges []string
var unrows []*types.Sandbox
for id, sb := range m.claimed {
// a zero deadline means no expiry (an archived claim kept forever).
// A zero deadline means no expiry (an archived claim kept forever).
if sb.Deadline.IsZero() || !now.After(sb.Deadline) {
continue
}
Expand All @@ -557,7 +557,7 @@ func (m *Manager) reapOnce(ctx context.Context) {
expired = append(expired, victim{action: reapPurge, id: id, ck: sb.ArchiveCk, tenant: sb.Tenant, deadline: sb.Deadline, sb: sb})
purges = append(purges, sb.ArchiveCk)
case m.archivesAtDeadline(sb):
// archive instead of destroy, kept in m.claimed for archive() to move
// Archive instead of destroy, kept in m.claimed for archive() to move
if _, busy := m.archiving[id]; busy {
continue
}
Expand Down Expand Up @@ -642,7 +642,7 @@ func (m *Manager) reapOnce(ctx context.Context) {
logger.Error(ctx, saveErr, "persist reap; rolled back")
return
}
// engine teardown can take minutes, so fan out bounded and never stall the ticker loop
// Engine teardown can take minutes, so fan out bounded and never stall the ticker loop
m.runBounded(ctx, len(expired), func(ctx context.Context, i int) {
v := expired[i]
switch v.action {
Expand Down
2 changes: 1 addition & 1 deletion sandboxd/pool/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (m *Manager) writeEnv(ctx context.Context, id, tenant, verb string, next fu
if !ok || !tenantOwns(tenant, sb.Tenant) {
return ErrUnknownSandbox
}
// a guest write must not race a capture
// A guest write must not race a capture
locked := sb.Transition.TryLock()
if locked {
defer sb.Transition.Unlock()
Expand Down
Loading
Loading