Add LCOW logpath within uVM

Add annotation to tee container `stdin` and `stderr` to a specified path
within the uVM.

Add a dedicated `trapsort.MultiWriter` type that writes to both an
underlying `transport.Connection` and the provided `io.Writer`.

Also, move `logConnection` from `stdio` to `transport`, to keep the
different `Connection` implementations together.

Add functional tests.

Streamline `test/internal/util` by making `CleanName` operation on the
test name directly (`testing.TB.Name()`), since that is the majority of
the usage.
Add `CleanString` to handle the original functionality.

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

Author: helsaawy

internal/guest/runtime/hcsv2/container.go

@@ -45,8 +45,10 @@ const (
 )
 
 type Container struct {
-	id    string
-	vsock transport.Transport
+	id string
+
+	vsock   transport.Transport
+	logFile *os.File
 
 	spec          *oci.Spec
 	ociBundlePath string
@@ -77,10 +79,17 @@ type Container struct {
 
 func (c *Container) Start(ctx context.Context, conSettings stdio.ConnectionSettings) (int, error) {
 	log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Start")
-	stdioSet, err := stdio.Connect(c.vsock, conSettings)
+
+	// only use the logfile for the init process, since we don't want to tee stdio of execs
+	t := c.vsock
+	if c.logFile != nil {
+		t = transport.NewMultiWriter(c.vsock, c.logFile)
+	}
+	stdioSet, err := stdio.Connect(t, conSettings)
 	if err != nil {
 		return -1, err
 	}
+
 	if c.initProcess.spec.Terminal {
 		ttyr := c.container.Tty()
 		ttyr.ReplaceConnectionSet(stdioSet)
@@ -180,12 +189,24 @@ func (c *Container) GetAllProcessPids(ctx context.Context) ([]int, error) {
 
 // Kill sends 'signal' to the container process.
 func (c *Container) Kill(ctx context.Context, signal syscall.Signal) error {
-	log.G(ctx).WithField(logfields.ContainerID, c.id).Info("opengcs::Container::Kill")
+	entity := log.G(ctx).WithField(logfields.ContainerID, c.id)
+	entity.Info("opengcs::Container::Kill")
 	err := c.container.Kill(signal)
 	if err != nil {
 		return err
 	}
+
 	c.setExitType(signal)
+	if c.logFile != nil {
+		if err = c.logFile.Close(); err != nil {
+			entity.WithFields(logrus.Fields{
+				logrus.ErrorKey: err,
+				logfields.Path:  c.logFile.Name(),
+			}).Warn("failed to close log file")
+		}
+		c.logFile = nil
+	}
+
 	return nil
 }
 

internal/guest/runtime/hcsv2/uvm.go

@@ -46,6 +46,7 @@ import (
 	"github.com/Microsoft/hcsshim/internal/guest/storage/scsi"
 	"github.com/Microsoft/hcsshim/internal/guest/transport"
 	"github.com/Microsoft/hcsshim/internal/log"
+	"github.com/Microsoft/hcsshim/internal/logfields"
 	"github.com/Microsoft/hcsshim/internal/oci"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
 	"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
@@ -442,6 +443,26 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM
 		c.vsock = h.devNullTransport
 	}
 
+	logPath := settings.OCISpecification.Annotations[annotations.LCOWTeeLogPath]
+	if logPath != "" {
+		if !allowStdio {
+			return nil, errors.Errorf("teeing container stdio to log path %q denied due to policy not allowing stdio access", logPath)
+		}
+
+		if logPath, err = filepath.Abs(logPath); err != nil {
+			return nil, errors.Wrapf(err, "failed to evaluate log path: %s", logPath)
+		}
+
+		log.G(ctx).WithField(logfields.Path, logPath).Debug("creating container log file")
+		dir := filepath.Dir(logPath)
+		if err := mkdirAllModePerm(dir); err != nil {
+			return nil, errors.Wrapf(err, "failed to create log file parent directory: %s", dir)
+		}
+		if c.logFile, err = os.Create(logPath); err != nil {
+			return nil, errors.Wrapf(err, "failed to create log file: %s", logPath)
+		}
+	}
+
 	if envToKeep != nil {
 		settings.OCISpecification.Process.Env = []string(envToKeep)
 	}

internal/guest/stdio/connection.go

@@ -4,11 +4,9 @@
 package stdio
 
 import (
-	"os"
+	"github.com/pkg/errors"
 
 	"github.com/Microsoft/hcsshim/internal/guest/transport"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 // ConnectionSettings describe the stdin, stdout, stderr ports to connect the
@@ -19,49 +17,6 @@ type ConnectionSettings struct {
 	StdErr *uint32
 }
 
-type logConnection struct {
-	con  transport.Connection
-	port uint32
-}
-
-func (lc *logConnection) Read(b []byte) (int, error) {
-	return lc.con.Read(b)
-}
-
-func (lc *logConnection) Write(b []byte) (int, error) {
-	return lc.con.Write(b)
-}
-
-func (lc *logConnection) Close() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing connection")
-
-	return lc.con.Close()
-}
-
-func (lc *logConnection) CloseRead() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing read connection")
-
-	return lc.con.CloseRead()
-}
-
-func (lc *logConnection) CloseWrite() error {
-	logrus.WithFields(logrus.Fields{
-		"port": lc.port,
-	}).Debug("opengcs::logConnection::Close - closing write connection")
-
-	return lc.con.CloseWrite()
-}
-
-func (lc *logConnection) File() (*os.File, error) {
-	return lc.con.File()
-}
-
-var _ = (transport.Connection)(&logConnection{})
-
 // Connect returns new transport.Connection instances, one for each stdio pipe
 // to be used. If CreateStd*Pipe for a given pipe is false, the given Connection
 // is set to nil.
@@ -77,30 +32,21 @@ func Connect(tport transport.Transport, settings ConnectionSettings) (_ *Connect
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stdin Connection")
 		}
-		connSet.In = &logConnection{
-			con:  c,
-			port: *settings.StdIn,
-		}
+		connSet.In = transport.NewLogConnection(c, *settings.StdIn)
 	}
 	if settings.StdOut != nil {
 		c, err := tport.Dial(*settings.StdOut)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stdout Connection")
 		}
-		connSet.Out = &logConnection{
-			con:  c,
-			port: *settings.StdOut,
-		}
+		connSet.Out = transport.NewLogConnection(c, *settings.StdOut)
 	}
 	if settings.StdErr != nil {
 		c, err := tport.Dial(*settings.StdErr)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed creating stderr Connection")
 		}
-		connSet.Err = &logConnection{
-			con:  c,
-			port: *settings.StdErr,
-		}
+		connSet.Err = transport.NewLogConnection(c, *settings.StdErr)
 	}
 	return connSet, nil
 }

internal/guest/transport/log.go

@@ -0,0 +1,38 @@
+//go:build linux
+
+package transport
+
+import (
+	"github.com/sirupsen/logrus"
+)
+
+// logConnection wraps the underlying [Connection] and logs the Close*() operations with
+// the connection's port number.
+type logConnection struct {
+	Connection
+	entry *logrus.Entry
+}
+
+var _ Connection = &logConnection{}
+
+func NewLogConnection(c Connection, port uint32) Connection {
+	return &logConnection{c, logrus.WithField("port", port)}
+}
+
+func (c *logConnection) Close() error {
+	c.entry.Debug("opengcs::logConnection::Close - closing connection")
+
+	return c.Connection.Close()
+}
+
+func (c *logConnection) CloseRead() error {
+	c.entry.Debug("opengcs::logConnection::Close - closing read connection")
+
+	return c.Connection.CloseRead()
+}
+
+func (c *logConnection) CloseWrite() error {
+	c.entry.Debug("opengcs::logConnection::Close - closing write connection")
+
+	return c.Connection.CloseWrite()
+}

internal/guest/transport/multiwriter.go

@@ -0,0 +1,45 @@
+//go:build linux
+
+package transport
+
+import (
+	"fmt"
+	"io"
+)
+
+// multiWriter writes to both the underlying connection and the [io.Writer], w, via [io.multiWriter].
+type multiWriter struct {
+	t Transport
+	w io.Writer
+}
+
+var _ Transport = &multiWriter{}
+
+func NewMultiWriter(t Transport, w io.Writer) Transport {
+	return &multiWriter{t, w}
+}
+
+// Dial accepts a vsock socket port number as configuration, and
+// returns an unconnected VsockConnection struct.
+func (t *multiWriter) Dial(port uint32) (Connection, error) {
+	if t == nil || t.w == nil || t.t == nil {
+		return nil, fmt.Errorf("invalid transpot")
+	}
+
+	conn, err := t.t.Dial(port)
+	if err != nil {
+		return nil, fmt.Errorf("multiwriter base transport dial: %w", err)
+	}
+	return &multiWriterConnection{conn, io.MultiWriter(conn, t.w)}, nil
+}
+
+type multiWriterConnection struct {
+	Connection
+	multi io.Writer
+}
+
+var _ Connection = &multiWriterConnection{}
+
+func (c *multiWriterConnection) Write(buf []byte) (int, error) {
+	return c.multi.Write(buf)
+}

internal/guest/transport/transport.go

@@ -5,6 +5,10 @@ import (
 	"os"
 )
 
+// TODO: specialized [Transport] and [Connection] for [io.Reader]/[io.Writer] instead of both,
+// so either stdin or stdout/stderr can be specialized without affecting the other.
+// i.e., don't use [multiWriter] for stdin.
+
 // Transport is the interface defining a method of transporting data in a
 // connection-like way.
 // Examples of a Transport implementation could be:

pkg/annotations/annotations.go

@@ -105,6 +105,12 @@ const (
 
 	// LCOWPrivileged is used to specify that the container should be run in privileged mode.
 	LCOWPrivileged = "io.microsoft.virtualmachine.lcow.privileged"
+
+	// LCOWTeeLogPath specifies a path inside the Linux uVM to write container's stdio to,
+	// in addition to the usual vsock pipe.
+	//
+	// Functionally, it is similar to `LogDirectory` and `LogPath` for CRI, but within the guest.
+	LCOWTeeLogPath = "io.microsoft.container.lcow.tee-log-path"
 )
 
 // LCOW integrity protection and confidential container annotations.