diff --git a/.golangci.yml b/.golangci.yml index 1354c27..4fc517d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,7 +9,6 @@ formatters: - gofmt - gofumpt - goimports - - golines - swaggo linters: @@ -41,6 +40,7 @@ linters: # Style linters that are total nuts. - wsl + - wsl_v5 - funlen # Enough parallelism for now. @@ -70,6 +70,18 @@ linters: # Not all errors need to be checked - errcheck + # It's OK to not initialize some struct fields + - exhaustruct + + # We'll control the variable name length ourselves + - varnamelen + + # Inline error handling keeps assignment and checking together + - noinlineerr + + # Avoid unrelated style churn for now + - funcorder + issues: # Don't hide multiple issues that belong to one class since GitHub annotations can handle them all nicely. max-issues-per-linter: 0 diff --git a/api/openapi.yaml b/api/openapi.yaml index fd668e3..f1b258c 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -791,7 +791,9 @@ components: model: macstudio hostDirs: type: array - description: Directories on the Orchard Worker host to mount to a VM + description: | + Directories on the Orchard Worker host to mount to a VM. + Requires running Orchard Controller with `--insecure-allow-host-dirs`. items: type: object properties: diff --git a/internal/command/controller/run.go b/internal/command/controller/run.go index b8cfff0..8c37222 100644 --- a/internal/command/controller/run.go +++ b/internal/command/controller/run.go @@ -30,6 +30,7 @@ var addressPprof string var debug bool var noTLS bool var sshNoClientAuth bool +var insecureAllowHostDirs bool var experimentalRPCV2 bool var noExperimentalRPCV2 bool var experimentalPingInterval time.Duration @@ -74,6 +75,8 @@ func newRunCommand() *cobra.Command { cmd.Flags().BoolVar(&sshNoClientAuth, "insecure-ssh-no-client-auth", false, "allow SSH clients to connect to the controller's SSH server without authentication, "+ "thus only authenticating on the target worker/VM's SSH server") + cmd.Flags().BoolVar(&insecureAllowHostDirs, "insecure-allow-host-dirs", false, + "allow unsafe path-based local host directory sharing") cmd.Flags().BoolVar(&experimentalRPCV2, "experimental-rpc-v2", false, "enable experimental RPC v2 (https://github.com/cirruslabs/orchard/issues/235)") _ = cmd.Flags().MarkHidden("experimental-rpc-v2") @@ -166,6 +169,10 @@ func runController(cmd *cobra.Command, args []string) (err error) { controllerOpts = append(controllerOpts, controller.WithSynthetic()) } + if insecureAllowHostDirs { + controllerOpts = append(controllerOpts, controller.WithInsecureAllowHostDirs()) + } + var controllerCert tls.Certificate if !noTLS { diff --git a/internal/command/dev/dev.go b/internal/command/dev/dev.go index eb3ddcf..632181f 100644 --- a/internal/command/dev/dev.go +++ b/internal/command/dev/dev.go @@ -35,6 +35,7 @@ var experimentalRPCV2 bool var addressPprof string var synthetic bool var workers int +var insecureAllowHostDirs bool func NewCommand() *cobra.Command { command := &cobra.Command{ @@ -57,6 +58,8 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&synthetic, "synthetic", false, "do not instantiate real Tart VM, use synthetic in-memory VMs suitable for load testing") command.Flags().IntVar(&workers, "workers", 1, "number of workers to start") + command.Flags().BoolVar(&insecureAllowHostDirs, "insecure-allow-host-dirs", false, + "allow unsafe path-based local host directory sharing") return command } @@ -105,6 +108,10 @@ func runDev(cmd *cobra.Command, args []string) error { additionalControllerOpts = append(additionalControllerOpts, controller.WithExperimentalRPCV2()) } + if insecureAllowHostDirs { + additionalControllerOpts = append(additionalControllerOpts, controller.WithInsecureAllowHostDirs()) + } + group, ctx := errgroup.WithContext(cmd.Context()) var additionalWorkerOpts []worker.Option diff --git a/internal/controller/api_vms.go b/internal/controller/api_vms.go index 593d4db..92afd84 100644 --- a/internal/controller/api_vms.go +++ b/internal/controller/api_vms.go @@ -158,6 +158,10 @@ func (controller *Controller) updateVMSpec(ctx *gin.Context) responder.Responder return responder.JSON(http.StatusBadRequest, NewErrorResponse("invalid JSON was provided")) } + if responder := controller.validateHostDirs(userVM.HostDirs); responder != nil { + return responder + } + name := ctx.Param("name") return controller.storeUpdate(func(txn storepkg.Transaction) responder.Responder { @@ -540,6 +544,14 @@ func (controller *Controller) validateHostDirs(hostDirs []v1.HostDir) responder. return nil } + if !controller.insecureAllowHostDirs { + return responder.JSON( + http.StatusBadRequest, + NewErrorResponse("host directory sharing is disabled; "+ + "restart the controller with --insecure-allow-host-dirs to enable this unsafe feature"), + ) + } + // Retrieve cluster settings var clusterSettings *v1.ClusterSettings var err error diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 70090d8..bf8ecc2 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -46,6 +46,7 @@ type Controller struct { listener net.Listener httpServer *http.Server insecureAuthDisabled bool + insecureAllowHostDirs bool scheduler *scheduler.Scheduler store storepkg.Store logger *zap.SugaredLogger @@ -195,6 +196,14 @@ func New(opts ...Option) (*Controller, error) { return nil, err } + // When no "--insecure-allow-host-dirs" is present, + // fail the VMs that have "hostDirs" set + if !controller.insecureAllowHostDirs { + if err := controller.failVMsWithHostDirs(); err != nil { + return nil, err + } + } + return controller, nil } @@ -305,6 +314,31 @@ func (controller *Controller) DeleteServiceAccount(name string) error { }) } +func (controller *Controller) failVMsWithHostDirs() error { + return controller.store.Update(func(txn storepkg.Transaction) error { + vms, err := txn.ListVMs() + if err != nil { + return err + } + + for _, vm := range vms { + if vm.TerminalState() || len(vm.HostDirs) == 0 { + continue + } + + vm.Status = v1.VMStatusFailed + vm.StatusMessage = "host directories are used, but host directory sharing is disabled" + vm.RestartPolicy = v1.RestartPolicyNever + + if err := txn.SetVM(vm); err != nil { + return err + } + } + + return nil + }) +} + func (controller *Controller) Run(ctx context.Context) error { // Run the scheduler so that each VM will eventually // be assigned to a specific Worker diff --git a/internal/controller/option.go b/internal/controller/option.go index adda403..e29b99a 100644 --- a/internal/controller/option.go +++ b/internal/controller/option.go @@ -48,6 +48,12 @@ func WithInsecureAuthDisabled() Option { } } +func WithInsecureAllowHostDirs() Option { + return func(controller *Controller) { + controller.insecureAllowHostDirs = true + } +} + func WithSwaggerDocs() Option { return func(controller *Controller) { controller.enableSwaggerDocs = true diff --git a/internal/tests/integration_test.go b/internal/tests/integration_test.go index f1145b8..ae76e42 100644 --- a/internal/tests/integration_test.go +++ b/internal/tests/integration_test.go @@ -372,12 +372,26 @@ func TestVMGarbageCollection(t *testing.T) { }), "failed to wait for the VM %s to be garbage-collected", vmName) } +func TestHostDirsDisabledByDefault(t *testing.T) { + devClient, _, _ := devcontroller.StartIntegrationTestEnvironment(t) + + err := devClient.VMs().Create(context.Background(), &v1.VM{ + Meta: v1.Meta{Name: "test-host-dirs-disabled"}, + Image: imageconstant.DefaultMacosImage, + HostDirs: []v1.HostDir{{Name: "src", Path: "/Users/ci/src"}}, + }) + require.Error(t, err) +} + func TestHostDirs(t *testing.T) { if runtime.GOOS != "darwin" { t.Skip("HostDirs is only supported on macOS with Tart") } - devClient, _, _ := devcontroller.StartIntegrationTestEnvironment(t) + devClient, _, _ := devcontroller.StartIntegrationTestEnvironmentWithAdditionalOpts(t, + false, []controller.Option{controller.WithInsecureAllowHostDirs()}, + false, nil, + ) dirToMount := t.TempDir() @@ -449,7 +463,10 @@ func TestHostDirsInvalidPolicy(t *testing.T) { t.Skip("HostDirs is only supported on macOS with Tart") } - devClient, _, _ := devcontroller.StartIntegrationTestEnvironment(t) + devClient, _, _ := devcontroller.StartIntegrationTestEnvironmentWithAdditionalOpts(t, + false, []controller.Option{controller.WithInsecureAllowHostDirs()}, + false, nil, + ) dirToMount := t.TempDir()