Add --force flag to trace create command

Updates #4805

PiperOrigin-RevId: 449759478

pkg/sentry/seccheck/config.go

@@ -68,12 +68,19 @@ type SinkConfig struct {
 }
 
 // Create reads the session configuration and applies it to the system.
-func Create(conf *SessionConfig) error {
+func Create(conf *SessionConfig, force bool) error {
 	log.Debugf("Creating seccheck: %+v", conf)
 	sessionsMu.Lock()
 	defer sessionsMu.Unlock()
+
 	if _, ok := sessions[conf.Name]; ok {
-		return fmt.Errorf("session %q already exists", conf.Name)
+		if !force {
+			return fmt.Errorf("session %q already exists", conf.Name)
+		}
+		if err := deleteLocked(conf.Name); err != nil {
+			return err
+		}
+		log.Infof("Trace session %q was deleted to be replaced", conf.Name)
 	}
 	if conf.Name != DefaultSessionName {
 		return fmt.Errorf(`only a single "Default" session is supported`)
@@ -154,7 +161,11 @@ func setupSink(config SinkConfig) (*os.File, error) {
 func Delete(name string) error {
 	sessionsMu.Lock()
 	defer sessionsMu.Unlock()
+	return deleteLocked(name)
+}
 
+// +checklocks:sessionsMu
+func deleteLocked(name string) error {
 	session := sessions[name]
 	if session == nil {
 		return fmt.Errorf("session %q not found", name)

runsc/boot/controller.go

@@ -604,6 +604,7 @@ func (cm *containerManager) Signal(args *SignalArgs, _ *struct{}) error {
 // CreateTraceSessionArgs are arguments to the CreateTraceSession method.
 type CreateTraceSessionArgs struct {
 	Config seccheck.SessionConfig
+	Force  bool
 	urpc.FilePayload
 }
 
@@ -619,7 +620,7 @@ func (cm *containerManager) CreateTraceSession(args *CreateTraceSessionArgs, _ *
 			args.Config.Sinks[i].FD = fd
 		}
 	}
-	return seccheck.Create(&args.Config)
+	return seccheck.Create(&args.Config, args.Force)
 }
 
 // DeleteTraceSession deletes an existing trace session.

runsc/boot/seccheck.go

@@ -74,5 +74,5 @@ func (c *InitConfig) create(sinkFDs []int) error {
 			c.TraceSession.Sinks[i].FD = fd.New(sinkFD)
 		}
 	}
-	return seccheck.Create(&c.TraceSession)
+	return seccheck.Create(&c.TraceSession, false)
 }

runsc/cmd/trace/create.go

@@ -30,6 +30,7 @@ import (
 // create implements subcommands.Command for the "create" command.
 type create struct {
 	config string
+	force  bool
 }
 
 // Name implements subcommands.Command.
@@ -51,6 +52,7 @@ func (*create) Usage() string {
 // SetFlags implements subcommands.Command.
 func (l *create) SetFlags(f *flag.FlagSet) {
 	f.StringVar(&l.config, "config", "", "path to the JSON file that describes the session being created")
+	f.BoolVar(&l.force, "force", false, "deletes a conflicting session, if one exists")
 }
 
 // Execute implements subcommands.Command.
@@ -87,7 +89,7 @@ func (l *create) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}
 		util.Fatalf("loading sandbox: %v", err)
 	}
 
-	if err := c.Sandbox.CreateTraceSession(sessionConfig); err != nil {
+	if err := c.Sandbox.CreateTraceSession(sessionConfig, l.force); err != nil {
 		util.Fatalf("creating session: %v", err)
 	}
 

runsc/container/trace_test.go

@@ -17,6 +17,7 @@ package container
 import (
 	"encoding/json"
 	"io/ioutil"
+	"strings"
 	"testing"
 	"time"
 
@@ -28,6 +29,15 @@ import (
 	"gvisor.dev/gvisor/runsc/boot"
 )
 
+func remoteSinkConfig(endpoint string) seccheck.SinkConfig {
+	return seccheck.SinkConfig{
+		Name: "remote",
+		Config: map[string]interface{}{
+			"endpoint": endpoint,
+		},
+	}
+}
+
 // Test that setting up a trace session configuration in PodInitConfig creates
 // a session before container creation.
 func TestTraceStartup(t *testing.T) {
@@ -56,14 +66,7 @@ func TestTraceStartup(t *testing.T) {
 							ContextFields: []string{"container_id"},
 						},
 					},
-					Sinks: []seccheck.SinkConfig{
-						{
-							Name: "remote",
-							Config: map[string]interface{}{
-								"endpoint": server.Path,
-							},
-						},
-					},
+					Sinks: []seccheck.SinkConfig{remoteSinkConfig(server.Path)},
 				},
 			}
 			encoder := json.NewEncoder(podInitConfig)
@@ -144,16 +147,9 @@ func TestTraceLifecycle(t *testing.T) {
 				ContextFields: []string{"container_id"},
 			},
 		},
-		Sinks: []seccheck.SinkConfig{
-			{
-				Name: "remote",
-				Config: map[string]interface{}{
-					"endpoint": server.Path,
-				},
-			},
-		},
+		Sinks: []seccheck.SinkConfig{remoteSinkConfig(server.Path)},
 	}
-	if err := cont.Sandbox.CreateTraceSession(&session); err != nil {
+	if err := cont.Sandbox.CreateTraceSession(&session, false); err != nil {
 		t.Fatalf("CreateTraceSession(): %v", err)
 	}
 
@@ -217,3 +213,87 @@ func TestTraceLifecycle(t *testing.T) {
 		t.Errorf("point received after session was deleted: %+v", server.GetPoints())
 	}
 }
+
+func TestTraceForceCreate(t *testing.T) {
+	spec, conf := sleepSpecConf(t)
+	_, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf)
+	if err != nil {
+		t.Fatalf("error setting up container: %v", err)
+	}
+	defer cleanup()
+
+	// Create and start the container.
+	args := Args{
+		ID:        testutil.RandomContainerID(),
+		Spec:      spec,
+		BundleDir: bundleDir,
+	}
+	cont, err := New(conf, args)
+	if err != nil {
+		t.Fatalf("error creating container: %v", err)
+	}
+	defer cont.Destroy()
+	if err := cont.Start(conf); err != nil {
+		t.Fatalf("error starting container: %v", err)
+	}
+
+	// Create a new trace session that will be overwritten.
+	server, err := test.NewServer()
+	if err != nil {
+		t.Fatalf("newServer(): %v", err)
+	}
+	defer server.Close()
+
+	session := seccheck.SessionConfig{
+		Name: "Default",
+		Points: []seccheck.PointConfig{
+			{Name: "sentry/exit_notify_parent"},
+		},
+		Sinks: []seccheck.SinkConfig{remoteSinkConfig(server.Path)},
+	}
+	if err := cont.Sandbox.CreateTraceSession(&session, false); err != nil {
+		t.Fatalf("CreateTraceSession(): %v", err)
+	}
+
+	// Trigger the configured point to check that trace session is enabled.
+	if ws, err := execute(conf, cont, "/bin/true"); err != nil || ws != 0 {
+		t.Fatalf("exec: true, ws: %v, err: %v", ws, err)
+	}
+	if err := server.WaitForCount(1); err != nil {
+		t.Fatalf("WaitForCount(1): %v", err)
+	}
+	pt := server.GetPoints()[0]
+	if want := pb.MessageType_MESSAGE_SENTRY_EXIT_NOTIFY_PARENT; pt.MsgType != want {
+		t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType)
+	}
+	server.Reset()
+
+	// Check that creating the same session fails.
+	if err := cont.Sandbox.CreateTraceSession(&session, false); err == nil || !strings.Contains(err.Error(), "already exists") {
+		t.Errorf("CreateTraceSession() again failed with wrong error: %v", err)
+	}
+
+	// Re-create the session with a different point using force=true and check
+	// that it overwrote the other trace session.
+	session = seccheck.SessionConfig{
+		Name: "Default",
+		Points: []seccheck.PointConfig{
+			{Name: "sentry/task_exit"},
+		},
+		Sinks: []seccheck.SinkConfig{remoteSinkConfig(server.Path)},
+	}
+	if err := cont.Sandbox.CreateTraceSession(&session, true); err != nil {
+		t.Fatalf("CreateTraceSession(force): %v", err)
+	}
+
+	if ws, err := execute(conf, cont, "/bin/true"); err != nil || ws != 0 {
+		t.Fatalf("exec: true, ws: %v, err: %v", ws, err)
+	}
+	if err := server.WaitForCount(1); err != nil {
+		t.Fatalf("WaitForCount(1): %v", err)
+	}
+	pt = server.GetPoints()[0]
+	if want := pb.MessageType_MESSAGE_SENTRY_TASK_EXIT; pt.MsgType != want {
+		t.Errorf("wrong message type, want: %v, got: %v", want, pt.MsgType)
+	}
+}

runsc/sandbox/sandbox.go

@@ -390,7 +390,7 @@ func (s *Sandbox) Processes(cid string) ([]*control.Process, error) {
 }
 
 // CreateTraceSession creates a new trace session.
-func (s *Sandbox) CreateTraceSession(config *seccheck.SessionConfig) error {
+func (s *Sandbox) CreateTraceSession(config *seccheck.SessionConfig, force bool) error {
 	log.Debugf("Creating trace session in sandbox %q", s.ID)
 
 	sinkFiles, err := seccheck.SetupSinks(config.Sinks)
@@ -411,6 +411,7 @@ func (s *Sandbox) CreateTraceSession(config *seccheck.SessionConfig) error {
 
 	arg := boot.CreateTraceSessionArgs{
 		Config: *config,
+		Force:  force,
 		FilePayload: urpc.FilePayload{
 			Files: sinkFiles,
 		},