Standardize LCOW uVM bootfiles update (microsoft#1861)

`NewDefaultOptionsLCOW` sets `RootFSFile` and `KernelFile` depending
on the contents of the (default) `BootFilesPath` directory and
`KerenelDirect` field.
However, if `BootFilesPath` is subsequently updated, those fields are
not updated.
This can result in inconsistent behavior, where (depending on if the
default `BootFilesPath` contains `vmlinux` and `rootfs.vhd` files), a
uVM created with an overridden `BootFilesPath` may either use `initrd`
(`kernel`) or `vmlinux` (`rootfs.vhd`), respectively.

Add a `UpdateBootFilesPath` function to consistently change the
`BootFilesPath` and associated options.
Update annotation handling to use `UpdateBootFilesPath`.
Security policy is still performed after the update, so settings will be
re-overridden for the confidential case, or by other annotations, so
existing (normal) behavior is persisted.

Signed-off-by: Hamza El-Saawy <[email protected]>

internal/oci/uvm.go

@@ -140,17 +140,23 @@ func parseAnnotationsPreferredRootFSType(ctx context.Context, a map[string]strin
 	return def
 }
 
-// handleAnnotationKernelDirectBoot handles parsing annotationKernelDirectBoot and setting
-// implied annotations from the result.
+// handleAnnotationBootFilesPath handles parsing annotations.BootFilesRootPath and setting
+// implied options from the result.
+func handleAnnotationBootFilesPath(ctx context.Context, a map[string]string, lopts *uvm.OptionsLCOW) {
+	lopts.UpdateBootFilesPath(ctx, parseAnnotationsString(a, annotations.BootFilesRootPath, lopts.BootFilesPath))
+}
+
+// handleAnnotationKernelDirectBoot handles parsing annotations.KernelDirectBoot and setting
+// implied options from the result.
 func handleAnnotationKernelDirectBoot(ctx context.Context, a map[string]string, lopts *uvm.OptionsLCOW) {
 	lopts.KernelDirect = ParseAnnotationsBool(ctx, a, annotations.KernelDirectBoot, lopts.KernelDirect)
 	if !lopts.KernelDirect {
 		lopts.KernelFile = uvm.KernelFile
 	}
 }
 
-// handleAnnotationPreferredRootFSType handles parsing annotationPreferredRootFSType and setting
-// implied annotations from the result
+// handleAnnotationPreferredRootFSType handles parsing annotations.PreferredRootFSType and setting
+// implied options from the result
 func handleAnnotationPreferredRootFSType(ctx context.Context, a map[string]string, lopts *uvm.OptionsLCOW) {
 	lopts.PreferredRootFSType = parseAnnotationsPreferredRootFSType(ctx, a, annotations.PreferredRootFSType, lopts.PreferredRootFSType)
 	switch lopts.PreferredRootFSType {
@@ -161,8 +167,8 @@ func handleAnnotationPreferredRootFSType(ctx context.Context, a map[string]strin
 	}
 }
 
-// handleAnnotationFullyPhysicallyBacked handles parsing annotationFullyPhysicallyBacked and setting
-// implied annotations from the result. For both LCOW and WCOW options.
+// handleAnnotationFullyPhysicallyBacked handles parsing annotations.FullyPhysicallyBacked and setting
+// implied options from the result. For both LCOW and WCOW options.
 func handleAnnotationFullyPhysicallyBacked(ctx context.Context, a map[string]string, opts interface{}) {
 	switch options := opts.(type) {
 	case *uvm.OptionsLCOW:
@@ -244,7 +250,7 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) (
 			WARNING!!!!!!!!!!
 
 			When adding an option here which must match some security policy by default, make sure that the correct default (ie matches
-			a default security policy) is applied in handleSecurityPolicy. Inadvertantly adding an "option" which defaults to false but MUST be
+			a default security policy) is applied in handleSecurityPolicy. Inadvertently adding an "option" which defaults to false but MUST be
 			true for a default security	policy to work will force the annotation to have be set by the team that owns the box. That will
 			be practically difficult and we	might not find out until a little late in the process.
 		*/
@@ -254,7 +260,7 @@ func SpecToUVMCreateOpts(ctx context.Context, s *specs.Spec, id, owner string) (
 		lopts.VPMemSizeBytes = parseAnnotationsUint64(ctx, s.Annotations, annotations.VPMemSize, lopts.VPMemSizeBytes)
 		lopts.VPMemNoMultiMapping = ParseAnnotationsBool(ctx, s.Annotations, annotations.VPMemNoMultiMapping, lopts.VPMemNoMultiMapping)
 		lopts.VPCIEnabled = ParseAnnotationsBool(ctx, s.Annotations, annotations.VPCIEnabled, lopts.VPCIEnabled)
-		lopts.BootFilesPath = parseAnnotationsString(s.Annotations, annotations.BootFilesRootPath, lopts.BootFilesPath)
+		handleAnnotationBootFilesPath(ctx, s.Annotations, lopts)
 		lopts.EnableScratchEncryption = ParseAnnotationsBool(ctx, s.Annotations, annotations.EncryptedScratchDisk, lopts.EnableScratchEncryption)
 		lopts.SecurityPolicy = parseAnnotationsString(s.Annotations, annotations.SecurityPolicy, lopts.SecurityPolicy)
 		lopts.SecurityPolicyEnforcer = parseAnnotationsString(s.Annotations, annotations.SecurityPolicyEnforcer, lopts.SecurityPolicyEnforcer)

internal/tools/uvmboot/lcow.go

@@ -161,13 +161,13 @@ func init() {
 		`.\uvmboot.exe -gcs lcow -boot-files-path "C:\ContainerPlat\LinuxBootFiles" -root-fs-type vhd -t -exec "/bin/bash"`
 }
 
-func createLCOWOptions(_ context.Context, c *cli.Context, id string) (*uvm.OptionsLCOW, error) {
+func createLCOWOptions(ctx context.Context, c *cli.Context, id string) (*uvm.OptionsLCOW, error) {
 	options := uvm.NewDefaultOptionsLCOW(id, "")
 	setGlobalOptions(c, options.Options)
 
 	// boot
 	if c.IsSet(bootFilesPathArgName) {
-		options.BootFilesPath = c.String(bootFilesPathArgName)
+		options.UpdateBootFilesPath(ctx, bootFilesPathArgName)
 	}
 
 	// kernel

internal/uvm/create_lcow.go

@@ -97,7 +97,10 @@ type OptionsLCOW struct {
 	*Options
 	*ConfidentialOptions
 
-	BootFilesPath           string               // Folder in which kernel and root file system reside. Defaults to \Program Files\Linux Containers
+	// Folder in which kernel and root file system reside. Defaults to \Program Files\Linux Containers.
+	//
+	// It is preferred to use [UpdateBootFilesPath] to change this value and update associated fields.
+	BootFilesPath           string
 	KernelFile              string               // Filename under `BootFilesPath` for the kernel. Defaults to `kernel`
 	KernelDirect            bool                 // Skip UEFI and boot directly to `kernel`
 	RootFSFile              string               // Filename under `BootFilesPath` for the UVMs root file system. Defaults to `InitrdFile`
@@ -144,7 +147,6 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW {
 	kernelDirectSupported := osversion.Build() >= 18286
 	opts := &OptionsLCOW{
 		Options:                 newDefaultOptions(id, owner),
-		BootFilesPath:           defaultLCOWOSBootFilesPath(),
 		KernelFile:              KernelFile,
 		KernelDirect:            kernelDirectSupported,
 		RootFSFile:              InitrdFile,
@@ -170,22 +172,59 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW {
 		},
 	}
 
+	opts.UpdateBootFilesPath(context.TODO(), defaultLCOWOSBootFilesPath())
+
+	return opts
+}
+
+// UpdateBootFilesPath updates the LCOW BootFilesPath field and associated settings.
+// Specifically, if [VhdFile] is found in path, RootFS is updated, and, if KernelDirect is set,
+// KernelFile is also updated if [UncompressedKernelFile] is found in path.
+//
+// This is a nop if the current BootFilesPath is equal to path (case-insensitive).
+func (opts *OptionsLCOW) UpdateBootFilesPath(ctx context.Context, path string) {
+	if p, err := filepath.Abs(path); err == nil {
+		path = p
+	} else {
+		// if its a filesystem issue, we'll error out elsewhere when we try to access the boot files
+		// otherwise, it might be transient, or a Go issue, so log and move on
+		log.G(ctx).WithFields(logrus.Fields{
+			logfields.Path:  p,
+			logrus.ErrorKey: err,
+		}).Warning("could not make boot files path absolute")
+	}
+
+	if strings.EqualFold(opts.BootFilesPath, path) { // Windows is case-insensitive, so compare paths that way too
+		return
+	}
+
+	opts.BootFilesPath = path
+
 	if _, err := os.Stat(filepath.Join(opts.BootFilesPath, VhdFile)); err == nil {
 		// We have a rootfs.vhd in the boot files path. Use it over an initrd.img
 		opts.RootFSFile = VhdFile
 		opts.PreferredRootFSType = PreferredRootFSTypeVHD
+
+		log.G(ctx).WithFields(logrus.Fields{
+			logfields.UVMID: opts.ID,
+			VhdFile:         filepath.Join(opts.BootFilesPath, VhdFile),
+		}).Debug("updated LCOW root filesystem to " + VhdFile)
 	}
 
-	if kernelDirectSupported {
+	if opts.KernelDirect {
 		// KernelDirect supports uncompressed kernel if the kernel is present.
 		// Default to uncompressed if on box. NOTE: If `kernel` is already
 		// uncompressed and simply named 'kernel' it will still be used
 		// uncompressed automatically.
 		if _, err := os.Stat(filepath.Join(opts.BootFilesPath, UncompressedKernelFile)); err == nil {
 			opts.KernelFile = UncompressedKernelFile
+
+			log.G(ctx).WithFields(logrus.Fields{
+				logfields.UVMID:        opts.ID,
+				UncompressedKernelFile: filepath.Join(opts.BootFilesPath, UncompressedKernelFile),
+			}).Debug("updated LCOW kernel file to " + UncompressedKernelFile)
 		}
 	}
-	return opts
 }
 
 // Get an acceptable number of processors given option and actual constraints.
@@ -759,7 +798,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error
 	}
 
 	span.AddAttributes(trace.StringAttribute(logfields.UVMID, opts.ID))
-	log.G(ctx).WithField("options", fmt.Sprintf("%+v", opts)).Debug("uvm::CreateLCOW options")
+	log.G(ctx).WithField("options", log.Format(ctx, opts)).Debug("uvm::CreateLCOW options")
 
 	// We don't serialize OutputHandlerCreator so if it is missing we need to put it back to the default.
 	if opts.OutputHandlerCreator == nil {

internal/uvm/create_test.go

@@ -11,10 +11,11 @@ import (
 // Unit tests for negative testing of input to uvm.Create()
 
 func TestCreateBadBootFilesPath(t *testing.T) {
+	ctx := context.Background()
 	opts := NewDefaultOptionsLCOW(t.Name(), "")
-	opts.BootFilesPath = `c:\does\not\exist\I\hope`
+	opts.UpdateBootFilesPath(ctx, `c:\does\not\exist\I\hope`)
 
-	_, err := CreateLCOW(context.Background(), opts)
+	_, err := CreateLCOW(ctx, opts)
 	if err == nil || err.Error() != `kernel: 'c:\does\not\exist\I\hope\kernel' not found` {
 		t.Fatal(err)
 	}

test/functional/main_test.go

@@ -227,9 +227,9 @@ func requireFeatures(tb testing.TB, features ...string) {
 
 func defaultLCOWOptions(tb testing.TB) *uvm.OptionsLCOW {
 	tb.Helper()
-	opts := testuvm.DefaultLCOWOptions(tb, util.CleanName(tb.Name()), hcsOwner)
+	opts := testuvm.DefaultLCOWOptions(context.TODO(), tb, util.CleanName(tb.Name()), hcsOwner)
 	if p := *flagLinuxBootFilesPath; p != "" {
-		opts.BootFilesPath = p
+		opts.UpdateBootFilesPath(context.TODO(), p)
 	}
 	return opts
 }

test/pkg/uvm/lcow.go

@@ -41,11 +41,11 @@ func init() {
 // path.
 //
 // See [uvm.NewDefaultOptionsLCOW] for more information.
-func DefaultLCOWOptions(tb testing.TB, id, owner string) *uvm.OptionsLCOW {
+func DefaultLCOWOptions(ctx context.Context, tb testing.TB, id, owner string) *uvm.OptionsLCOW {
 	tb.Helper()
 	opts := uvm.NewDefaultOptionsLCOW(id, owner)
 	if lcowOSBootFiles != "" {
-		opts.BootFilesPath = lcowOSBootFiles
+		opts.UpdateBootFilesPath(ctx, lcowOSBootFiles)
 	}
 	return opts
 }
@@ -55,7 +55,7 @@ func DefaultLCOWOptions(tb testing.TB, id, owner string) *uvm.OptionsLCOW {
 // See [CreateAndStartLCOWFromOpts].
 func CreateAndStartLCOW(ctx context.Context, tb testing.TB, id string) *uvm.UtilityVM {
 	tb.Helper()
-	return CreateAndStartLCOWFromOpts(ctx, tb, DefaultLCOWOptions(tb, id, ""))
+	return CreateAndStartLCOWFromOpts(ctx, tb, DefaultLCOWOptions(ctx, tb, id, ""))
 }
 
 // CreateAndStartLCOWFromOpts creates an LCOW utility VM with the specified options.