refactor: Move runtime directory to user scope (#731)

Reviewed-by: Jakub Ciolek <jakub@unikraft.io> Approved-by: Jakub Ciolek <jakub@unikraft.io>
unikraft · Aug 21, 2023 · 55392fd · 55392fd
2 parents ceaab59 + 9fda1a7
commit 55392fd
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 117 deletions.
diff --git a/cmd/kraft/run/run.go b/cmd/kraft/run/run.go
@@ -9,9 +9,7 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
-	"strconv"
 	"strings"
 
 	"github.com/MakeNowJust/heredoc"
@@ -313,22 +311,6 @@ func (opts *Run) Run(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("could not make machine state dir: %w", err)
 		}
 
-		group, err := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-		if err == nil {
-			gid, err := strconv.ParseInt(group.Gid, 10, 32)
-			if err != nil {
-				return fmt.Errorf("could not parse group ID for kraftkit: %w", err)
-			}
-
-			if err := os.Chown(machine.Status.StateDir, os.Getuid(), int(gid)); err != nil {
-				return fmt.Errorf("could not change group ownership of machine state dir: %w", err)
-			}
-		} else {
-			log.G(ctx).
-				WithField("error", err).
-				Warn("kraftkit group not found, falling back to current user")
-		}
-
 		var ramfs *initrd.InitrdConfig
 		cwd, err := os.Getwd()
 		if err != nil {

diff --git a/cmd/kraft/run/runner_package.go b/cmd/kraft/run/runner_package.go
@@ -9,9 +9,7 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
-	"strconv"
 
 	"k8s.io/apimachinery/pkg/util/uuid"
 	machineapi "kraftkit.sh/api/machine/v1alpha1"
@@ -110,22 +108,6 @@ func (runner *runnerPackage) Prepare(ctx context.Context, opts *Run, machine *ma
 		return err
 	}
 
-	group, err := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-	if err == nil {
-		gid, err := strconv.ParseInt(group.Gid, 10, 32)
-		if err != nil {
-			return fmt.Errorf("could not parse group ID for kraftkit: %w", err)
-		}
-
-		if err := os.Chown(machine.Status.StateDir, os.Getuid(), int(gid)); err != nil {
-			return fmt.Errorf("could not change group ownership of machine state dir: %w", err)
-		}
-	} else {
-		log.G(ctx).
-			WithField("error", err).
-			Warn("kraftkit group not found, falling back to current user")
-	}
-
 	// Clean up the package directory if an error occurs before returning.
 	defer func() {
 		if err != nil {

diff --git a/config/config.go b/config/config.go
@@ -23,11 +23,11 @@ type KraftKit struct {
 	GitProtocol    string `yaml:"git_protocol" env:"KRAFTKIT_GIT_PROTOCOL" long:"git-protocol" usage:"Preferred Git protocol to use" default:"https"`
 	Pager          string `yaml:"pager,omitempty" env:"KRAFTKIT_PAGER" long:"pager" usage:"System pager to pipe output to"`
 	HTTPUnixSocket string `yaml:"http_unix_socket,omitempty" env:"KRAFTKIT_HTTP_UNIX_SOCKET" long:"http-unix-sock" usage:"When making HTTP(S) connections, pipe requests via this shared socket"`
-	RuntimeDir     string `yaml:"runtime_dir" env:"KRAFTKIT_RUNTIME_DIR" long:"runtime-dir" usage:"Directory for placing runtime files (e.g. pidfiles)" default:"/var/kraftkit"`
+	RuntimeDir     string `yaml:"runtime_dir" env:"KRAFTKIT_RUNTIME_DIR" long:"runtime-dir" usage:"Directory for placing runtime files (e.g. pidfiles)"`
 	DefaultPlat    string `yaml:"default_plat" env:"KRAFTKIT_DEFAULT_PLAT" usage:"The default platform to use when invoking platform-specific code" noattribute:"true"`
 	DefaultArch    string `yaml:"default_arch" env:"KRAFTKIT_DEFAULT_ARCH" usage:"The default architecture to use when invoking architecture-specific code" noattribute:"true"`
 	ContainerdAddr string `yaml:"containerd_addr,omitempty" env:"KRAFTKIT_CONTAINERD_ADDR" long:"containerd-addr" usage:"Address of containerd daemon socket" default:""`
-	EventsPidFile  string `yaml:"events_pidfile" env:"KRAFTKIT_EVENTS_PIDFILE" long:"events-pid-file" usage:"Events process ID used when running multiple unikernels" default:"/var/kraftkit/events.pid"`
+	EventsPidFile  string `yaml:"events_pidfile" env:"KRAFTKIT_EVENTS_PIDFILE" long:"events-pid-file" usage:"Events process ID used when running multiple unikernels"`
 	UserGroup      string `yaml:"user_group" env:"KRAFTKIT_USER_GROUP" long:"user-group" usage:"Group to use for common files" default:"kraftkit"`
 
 	Paths struct {

diff --git a/config/defaults.go b/config/defaults.go
@@ -12,8 +12,6 @@ import (
 )
 
 const (
-	DefaultRuntimeDir    = "/var/kraftkit"
-	DefaultEventsPidFile = "/var/kraftkit/events.pid"
 	defaultManifestIndex = "https://manifests.kraftkit.sh/index.yaml"
 )
 
@@ -39,6 +37,16 @@ func NewDefaultKraftKitConfig() (*KraftKit, error) {
 		c.Paths.Manifests = filepath.Join(DataDir(), "manifests")
 	}
 
+	// ..for runtime files..
+	if len(c.RuntimeDir) == 0 {
+		c.RuntimeDir = filepath.Join(DataDir(), "runtime")
+	}
+
+	// ..for events files..
+	if len(c.EventsPidFile) == 0 {
+		c.EventsPidFile = filepath.Join(c.RuntimeDir, "events.pid")
+	}
+
 	// ..and for cached source files
 	if len(c.Paths.Sources) == 0 {
 		c.Paths.Sources = filepath.Join(DataDir(), "sources")

diff --git a/machine/firecracker/v1alpha1.go b/machine/firecracker/v1alpha1.go
@@ -9,9 +9,7 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"time"
 
@@ -99,22 +97,6 @@ func (service *machineV1alpha1Service) Create(ctx context.Context, machine *mach
 		return machine, err
 	}
 
-	group, err := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-	if err == nil {
-		gid, err := strconv.ParseInt(group.Gid, 10, 32)
-		if err != nil {
-			return machine, fmt.Errorf("could not parse group ID for kraftkit: %w", err)
-		}
-
-		if err := os.Chown(machine.Status.StateDir, os.Getuid(), int(gid)); err != nil {
-			return machine, fmt.Errorf("could not change group ownership of machine state dir: %w", err)
-		}
-	} else {
-		log.G(ctx).
-			WithField("error", err).
-			Warn("kraftkit group not found, falling back to current user")
-	}
-
 	// Set and create the log file for this machine
 	if len(machine.Status.LogFile) == 0 {
 		machine.Status.LogFile = filepath.Join(machine.Status.StateDir, "machine.log")

diff --git a/machine/qemu/v1alpha1.go b/machine/qemu/v1alpha1.go
@@ -11,7 +11,6 @@ import (
 	"io/fs"
 	"net"
 	"os"
-	"os/user"
 	"path/filepath"
 	"reflect"
 	"strconv"
@@ -32,7 +31,6 @@ import (
 	"kraftkit.sh/exec"
 	"kraftkit.sh/internal/logtail"
 	"kraftkit.sh/internal/retrytimeout"
-	"kraftkit.sh/log"
 	"kraftkit.sh/machine/network/macaddr"
 	"kraftkit.sh/machine/qemu/qmp"
 	qmpapi "kraftkit.sh/machine/qemu/qmp/v7alpha2"
@@ -150,22 +148,6 @@ func (service *machineV1alpha1Service) Create(ctx context.Context, machine *mach
 		return machine, err
 	}
 
-	group, err := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-	if err == nil {
-		gid, err := strconv.ParseInt(group.Gid, 10, 32)
-		if err != nil {
-			return machine, fmt.Errorf("could not parse group ID for kraftkit: %w", err)
-		}
-
-		if err := os.Chown(machine.Status.StateDir, os.Getuid(), int(gid)); err != nil {
-			return machine, fmt.Errorf("could not change group ownership of machine state dir: %w", err)
-		}
-	} else {
-		log.G(ctx).
-			WithField("error", err).
-			Warn("kraftkit group not found, falling back to current user")
-	}
-
 	// Set and create the log file for this machine
 	if len(machine.Status.LogFile) == 0 {
 		machine.Status.LogFile = filepath.Join(machine.Status.StateDir, "machine.log")

diff --git a/oci/manager_options.go b/oci/manager_options.go
@@ -3,11 +3,8 @@ package oci
 import (
 	"context"
 	"fmt"
-	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
-	"strconv"
 
 	regtypes "github.com/docker/docker/api/types/registry"
 	"github.com/genuinetools/reg/repoutils"
@@ -46,26 +43,6 @@ func WithDetectHandler() OCIManagerOption {
 		// Fall-back to using a simpler directory/tarball-based OCI handler
 		ociDir := filepath.Join(config.G[config.KraftKit](ctx).RuntimeDir, "oci")
 
-		if err := os.MkdirAll(config.G[config.KraftKit](ctx).RuntimeDir, fs.ModeSetgid|0o775); err != nil {
-			return err
-		}
-
-		group, err := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-		if err == nil {
-			gid, err := strconv.ParseInt(group.Gid, 10, 32)
-			if err != nil {
-				return fmt.Errorf("could not parse group ID for kraftkit: %w", err)
-			}
-
-			if err := os.Chown(config.G[config.KraftKit](ctx).RuntimeDir, os.Getuid(), int(gid)); err != nil {
-				return fmt.Errorf("could not change group ownership of machine state dir: %w", err)
-			}
-		} else {
-			log.G(ctx).
-				WithField("error", err).
-				Warn("kraftkit group not found, falling back to current user")
-		}
-
 		log.G(ctx).WithFields(logrus.Fields{
 			"path": ociDir,
 		}).Trace("using oci directory handler")

diff --git a/oci/pack.go b/oci/pack.go
@@ -10,9 +10,7 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
-	"strconv"
 	"strings"
 
 	"github.com/google/go-containerregistry/pkg/crane"
@@ -120,22 +118,6 @@ func NewPackageFromTarget(ctx context.Context, targ target.Target, opts ...packm
 			return nil, fmt.Errorf("could not create local oci cache directory: %w", gerr)
 		}
 
-		group, gerr := user.LookupGroup(config.G[config.KraftKit](ctx).UserGroup)
-		if gerr == nil {
-			gid, gerr := strconv.ParseInt(group.Gid, 10, 32)
-			if gerr != nil {
-				return nil, fmt.Errorf("could not parse group ID for kraftkit: %w", gerr)
-			}
-
-			if gerr := os.Chown(config.G[config.KraftKit](ctx).RuntimeDir, os.Getuid(), int(gid)); gerr != nil {
-				return nil, fmt.Errorf("could not change group ownership of machine state dir: %w", gerr)
-			}
-		} else {
-			log.G(ctx).
-				WithField("error", err).
-				Warn("kraftkit group not found, falling back to current user")
-		}
-
 		ociDir := filepath.Join(config.G[config.KraftKit](ctx).RuntimeDir, "oci")
 
 		log.G(ctx).WithFields(logrus.Fields{