guest: use OCIBundlePath as sandbox root source of truth by shreyanshjain7174 · Pull Request #2653 · microsoft/hcsshim

shreyanshjain7174 · 2026-03-31T12:55:20Z

Guest-side GCS currently derives sandbox paths by combining a hard-coded prefix (/run/gcs/c) with the sandbox ID, ignoring the OCIBundlePath the host already sends. This breaks with Shim v2 path layouts and multi-pod UVMs.

This PR stores settings.OCIBundlePath as the canonical sandbox root on the Host struct (sandboxRoots map), and threads the resolved root through all setup and cleanup functions instead of re-deriving it.

For virtual pods, the sandbox root is derived from OCIBundlePath's parent directory (preserving the virtual-pods/ layout). For workload containers, it's looked up from the mapping. A legacy fallback ensures backwards compatibility with shim v1.

When the old shim sends OCIBundlePath in the legacy format, all paths are identical to before — verified by parity tests and a full CRI lifecycle (crictl runp/create/start/stop/rm) on a Hyper-V UVM.

Replace heuristic sandbox path derivation (hard-coded /run/gcs/c prefix + ID) with host-provided OCIBundlePath as the canonical sandbox root directory. This change prepares the guest-side GCS for Shim v2 and multi-pod UVM support, where the host may use a different path layout than the legacy /run/gcs/c/<id>. Key changes: - Add sandboxRoots mapping on Host to store resolved sandbox root per sandbox ID - Sandbox containers: register OCIBundlePath as sandbox root - Virtual pods: derive sandbox root from OCIBundlePath parent + /virtual-pods/<id> - Workload containers: resolve sandbox root from Host mapping (fallback to legacy) - Standalone containers: use OCIBundlePath directly as root - Container.Delete: use stored sandboxRoot for cleanup paths - Remove duplicate setup functions (setupVirtualPod* merged into unified setup*) The refactor produces identical paths when the old shim sends OCIBundlePath in the legacy format, ensuring zero behavior change for existing deployments. Security: virtualPodID is validated against path traversal before use. Signed-off-by: Shreyansh Sancheti <shsancheti@microsoft.com>

rawahars

Overall love the simplification of the virtual pod vs normal pod differentiation.

Major comments are regarding keeping existing utility methods, while removing virtual pod specific ones and then changing these methods to return paths/values which you are creating on the fly. That would make it easier to review too.

rawahars · 2026-03-31T20:39:54Z

internal/guest/runtime/hcsv2/container.go

 	scratchDirPath string
+
+	// sandboxRoot is the resolved sandbox root directory for this container,
+	// populated from the Host's sandbox root mapping. Used during cleanup


nit: populated from the root directory of the pod within the guest.

rawahars · 2026-03-31T20:41:56Z

internal/guest/runtime/hcsv2/container.go

-		// remove user mounts in sandbox container - use virtual pod aware paths
-		if err := storage.UnmountAllInPath(ctx, specGuest.VirtualPodAwareSandboxMountsDir(c.id, virtualSandboxID), true); err != nil {
+	if c.isSandbox && c.sandboxRoot != "" {
+		if err := storage.UnmountAllInPath(ctx, filepath.Join(c.sandboxRoot, "sandboxMounts"), true); err != nil {


Maybe leave the utility methods as is which makes things a bit easier to read i.e. have method for resolving Mounts dir or tempfs dir.

rawahars · 2026-03-31T20:42:53Z

internal/guest/runtime/hcsv2/sandbox_container.go

-	return filepath.Join(specGuest.VirtualPodAwareSandboxRootDir(id, virtualSandboxID), "hosts")
-}
-
-func getSandboxResolvPath(id, virtualSandboxID string) string {


Same comment as earlier for these too.

rawahars · 2026-03-31T20:50:55Z

internal/guest/runtime/hcsv2/uvm.go

 	delete(h.containers, id)
+
+	// Clean up the sandbox root mapping if this was a sandbox container.
+	criType := c.spec.Annotations[annotations.KubernetesContainerType]


Instead of checking for the sandbox type, let's check if the id is present within the sandbox map. It makes me wonder if we need specific methods for Register, Resolve and Unregister since they would be called once in normal code flow. But I understand that we need those for unit testing.

rawahars · 2026-03-31T20:51:47Z

internal/guest/runtime/hcsv2/uvm.go

-	tmpfsDir := specGuest.SandboxTmpfsMountsDir(id)
+// setupTmpfsMountsPath creates the sandbox tmpfs mounts directory from a resolved root.
+func setupTmpfsMountsPath(sandboxRoot string) (err error) {
+	tmpfsDir := filepath.Join(sandboxRoot, "sandboxTmpfsMounts")


Same comment as earlier. Consider keeping method SandboxTmpfsMountsDir.

rawahars · 2026-03-31T20:55:47Z

internal/guest/runtime/hcsv2/uvm.go

+			// For virtual pods, derive the shared root from OCIBundlePath's parent
+			// directory, preserving the virtual-pods/ subdirectory layout.
+			var sandboxRoot string
+			if isVirtualPod {


This logic can move to the Register method itself.

rawahars · 2026-03-31T20:56:49Z

internal/guest/runtime/hcsv2/workload_container.go

 	// if spec didn't override them explicitly.
-	networkingMounts := specGuest.GenerateWorkloadContainerNetworkMounts(sbid, spec)
-	spec.Mounts = append(spec.Mounts, networkingMounts...)
+	for _, mountPath := range []string{"/etc/hostname", "/etc/hosts", "/etc/resolv.conf"} {


Again keep the method.

rawahars · 2026-03-31T20:57:14Z

internal/guest/runtime/hcsv2/uvm.go


+// RegisterSandboxRoot stores the resolved sandbox root directory for a given sandbox ID.
+// For virtual pods, the caller should transform the OCIBundlePath before registering.
+func (h *Host) RegisterSandboxRoot(sandboxID, sandboxRoot string) {


Do not export the methods if those will not be used outside this package.

rawahars · 2026-03-31T21:03:48Z

@anmaxvl Can you please take a look too to see if any invariant of Security Policy gets impacted with this change. As I see it that should not change but please do confirm that.

anmaxvl · 2026-04-01T00:20:19Z

@micromaomao as well.

micromaomao · 2026-04-01T00:42:56Z

@anmaxvl:

@micromaomao as well.

I will have a closer look tomorrow, but I think this is probably good because #2581 added validation to settings.OCIBundlePath (not in this PR's branch yet but in main) to force it to be the expected value.

shreyanshjain7174 requested a review from a team as a code owner March 31, 2026 12:55

shreyanshjain7174 force-pushed the sandbox-path-refactor branch 2 times, most recently from bbc4d4d to 7deb168 Compare March 31, 2026 13:05

shreyanshjain7174 requested a review from rawahars March 31, 2026 16:04

shreyanshjain7174 force-pushed the sandbox-path-refactor branch from a67564e to 9de87ec Compare March 31, 2026 16:37

rawahars requested a review from helsaawy March 31, 2026 20:28

rawahars requested changes Mar 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

guest: use OCIBundlePath as sandbox root source of truth#2653

guest: use OCIBundlePath as sandbox root source of truth#2653
shreyanshjain7174 wants to merge 1 commit intomicrosoft:mainfrom
shreyanshjain7174:sandbox-path-refactor

shreyanshjain7174 commented Mar 31, 2026

Uh oh!

rawahars left a comment

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars Mar 31, 2026

Uh oh!

rawahars commented Mar 31, 2026

Uh oh!

anmaxvl commented Apr 1, 2026

Uh oh!

micromaomao commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

shreyanshjain7174 commented Mar 31, 2026

Uh oh!

rawahars left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rawahars commented Mar 31, 2026

Uh oh!

anmaxvl commented Apr 1, 2026

Uh oh!

micromaomao commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants