From 10f56148d4188167056f72c7f8f706acad5ce5fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Thu, 14 Jul 2022 17:27:17 +0200 Subject: [PATCH] Allow saving configuration from the first-time wizard This just updates the config and saves it to `flamenco-manager.yaml`. Saving the configuration doesn't restart the Manager yet, that's for another commit. --- internal/manager/api_impl/interfaces.go | 3 + internal/manager/api_impl/meta.go | 70 ++++++++++++++ internal/manager/api_impl/meta_test.go | 96 +++++++++++++++++++ .../api_impl/mocks/api_impl_mock.gen.go | 14 +++ internal/manager/config/defaults.go | 8 +- web/app/src/views/FirstTimeWizardView.vue | 89 ++++++++++++++--- 6 files changed, 263 insertions(+), 17 deletions(-) diff --git a/internal/manager/api_impl/interfaces.go b/internal/manager/api_impl/interfaces.go index 4e7c1c4d..7dcc9632 100644 --- a/internal/manager/api_impl/interfaces.go +++ b/internal/manager/api_impl/interfaces.go @@ -165,6 +165,9 @@ type ConfigService interface { // ForceFirstRun forces IsFirstRun() to return true. This is used to force the // first-time wizard on a configured system. ForceFirstRun() + + // Save writes the in-memory configuration to the config file. + Save() error } type Shaman interface { diff --git a/internal/manager/api_impl/meta.go b/internal/manager/api_impl/meta.go index 25b5bbaa..950eaef3 100644 --- a/internal/manager/api_impl/meta.go +++ b/internal/manager/api_impl/meta.go @@ -11,6 +11,8 @@ import ( "os" "os/exec" "path/filepath" + "strconv" + "strings" "git.blender.org/flamenco/internal/appinfo" "git.blender.org/flamenco/internal/find_blender" @@ -168,6 +170,7 @@ func (f *Flamenco) FindBlenderExePath(e echo.Context) error { default: response = append(response, api.BlenderPathCheckResult{ IsUsable: true, + Input: result.Input, Path: result.FoundLocation, Cause: result.BlenderVersion, Source: result.Source, @@ -186,6 +189,7 @@ func (f *Flamenco) FindBlenderExePath(e echo.Context) error { default: response = append(response, api.BlenderPathCheckResult{ IsUsable: true, + Input: result.Input, Path: result.FoundLocation, Cause: result.BlenderVersion, Source: result.Source, @@ -212,6 +216,7 @@ func (f *Flamenco) CheckBlenderExePath(e echo.Context) error { ctx := e.Request().Context() checkResult, err := find_blender.CheckBlender(ctx, command) response := api.BlenderPathCheckResult{ + Input: command, Source: checkResult.Source, } @@ -229,6 +234,67 @@ func (f *Flamenco) CheckBlenderExePath(e echo.Context) error { return e.JSON(http.StatusOK, response) } +func (f *Flamenco) SaveWizardConfig(e echo.Context) error { + logger := requestLogger(e) + + var wizardCfg api.WizardConfig + if err := e.Bind(&wizardCfg); err != nil { + logger.Warn().Err(err).Msg("first-time wizard: bad request received") + return sendAPIError(e, http.StatusBadRequest, "invalid format") + } + + logger = logger.With().Interface("config", wizardCfg).Logger() + + if wizardCfg.StorageLocation == "" || + !wizardCfg.BlenderExecutable.IsUsable || + wizardCfg.BlenderExecutable.Path == "" { + logger.Warn().Msg("first-time wizard: configuration is incomplete, unable to accept") + return sendAPIError(e, http.StatusBadRequest, "configuration is incomplete") + } + + conf := f.config.Get() + conf.SharedStoragePath = wizardCfg.StorageLocation + + var executable string + switch wizardCfg.BlenderExecutable.Source { + case api.BlenderPathSourceFileAssociation: + // The Worker will try to use the file association when the command is set + // to the string "blender". + executable = "blender" + case api.BlenderPathSourcePathEnvvar: + // The input command can be found on $PATH, and thus we don't need to save + // the absolute path to Blender here. + executable = wizardCfg.BlenderExecutable.Input + case api.BlenderPathSourceInputPath: + // The path should be used as-is. + executable = wizardCfg.BlenderExecutable.Path + } + if commandNeedsQuoting(executable) { + executable = strconv.Quote(executable) + } + blenderCommand := fmt.Sprintf("%s %s", executable, config.DefaultBlenderArguments) + + // Use the same command for each platform for now, but put them each in their + // own definition so that they're easier to edit later. + conf.Variables["blender"] = config.Variable{ + IsTwoWay: false, + Values: config.VariableValues{ + {Platform: "linux", Value: blenderCommand}, + {Platform: "windows", Value: blenderCommand}, + {Platform: "darwin", Value: blenderCommand}, + }, + } + + // Save the final configuration to disk. + if err := f.config.Save(); err != nil { + logger.Error().Err(err).Msg("error saving configuration file") + return sendAPIError(e, http.StatusInternalServerError, "first-time wizard: error saving configuration file: %v", err) + } + + logger.Info().Msg("first-time wizard: updating configuration") + return e.NoContent(http.StatusNoContent) +} + func flamencoManagerDir() (string, error) { exename, err := os.Executable() if err != nil { @@ -236,3 +302,7 @@ func flamencoManagerDir() (string, error) { } return filepath.Dir(exename), nil } + +func commandNeedsQuoting(cmd string) bool { + return strings.ContainsAny(cmd, " \n\t;()") +} diff --git a/internal/manager/api_impl/meta_test.go b/internal/manager/api_impl/meta_test.go index 0ec97ef0..968b04c0 100644 --- a/internal/manager/api_impl/meta_test.go +++ b/internal/manager/api_impl/meta_test.go @@ -116,6 +116,102 @@ func TestCheckSharedStoragePath(t *testing.T) { } } +func TestSaveWizardConfig(t *testing.T) { + mf, finish := metaTestFixtures(t) + defer finish() + + doTest := func(body api.WizardConfig) config.Conf { + // Always start the test with a clean configuration. + originalConfig := config.DefaultConfig(func(c *config.Conf) { + c.SharedStoragePath = "" + }) + var savedConfig config.Conf + + // Mock the loading & saving of the config. + mf.config.EXPECT().Get().Return(&originalConfig) + mf.config.EXPECT().Save().Do(func() error { + savedConfig = originalConfig + return nil + }) + + // Call the API. + echoCtx := mf.prepareMockedJSONRequest(body) + err := mf.flamenco.SaveWizardConfig(echoCtx) + if !assert.NoError(t, err) { + t.FailNow() + } + + assertResponseNoContent(t, echoCtx) + return savedConfig + } + + // Test situation where file association with .blend files resulted in a blender executable. + { + savedConfig := doTest(api.WizardConfig{ + StorageLocation: mf.tempdir, + BlenderExecutable: api.BlenderPathCheckResult{ + IsUsable: true, + Input: "", + Path: "/path/to/blender", + Source: api.BlenderPathSourceFileAssociation, + }, + }) + assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath) + expectBlenderVar := config.Variable{ + Values: config.VariableValues{ + {Platform: "linux", Value: "blender " + config.DefaultBlenderArguments}, + {Platform: "windows", Value: "blender " + config.DefaultBlenderArguments}, + {Platform: "darwin", Value: "blender " + config.DefaultBlenderArguments}, + }, + } + assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"]) + } + + // Test situation where the given command could be found on $PATH. + { + savedConfig := doTest(api.WizardConfig{ + StorageLocation: mf.tempdir, + BlenderExecutable: api.BlenderPathCheckResult{ + IsUsable: true, + Input: "kitty", + Path: "/path/to/kitty", + Source: api.BlenderPathSourcePathEnvvar, + }, + }) + assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath) + expectBlenderVar := config.Variable{ + Values: config.VariableValues{ + {Platform: "linux", Value: "kitty " + config.DefaultBlenderArguments}, + {Platform: "windows", Value: "kitty " + config.DefaultBlenderArguments}, + {Platform: "darwin", Value: "kitty " + config.DefaultBlenderArguments}, + }, + } + assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"]) + } + + // Test a custom command given with the full path. + { + savedConfig := doTest(api.WizardConfig{ + StorageLocation: mf.tempdir, + BlenderExecutable: api.BlenderPathCheckResult{ + IsUsable: true, + Input: "/bin/cat", + Path: "/bin/cat", + Source: api.BlenderPathSourceInputPath, + }, + }) + assert.Equal(t, mf.tempdir, savedConfig.SharedStoragePath) + expectBlenderVar := config.Variable{ + Values: config.VariableValues{ + {Platform: "linux", Value: "/bin/cat " + config.DefaultBlenderArguments}, + {Platform: "windows", Value: "/bin/cat " + config.DefaultBlenderArguments}, + {Platform: "darwin", Value: "/bin/cat " + config.DefaultBlenderArguments}, + }, + } + assert.Equal(t, expectBlenderVar, savedConfig.Variables["blender"]) + } +} + func metaTestFixtures(t *testing.T) (mockedFlamenco, func()) { mockCtrl := gomock.NewController(t) mf := newMockedFlamenco(mockCtrl) diff --git a/internal/manager/api_impl/mocks/api_impl_mock.gen.go b/internal/manager/api_impl/mocks/api_impl_mock.gen.go index d898ee0f..17c1e8e5 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/internal/manager/api_impl/mocks/api_impl_mock.gen.go @@ -754,6 +754,20 @@ func (mr *MockConfigServiceMockRecorder) ResolveVariables(arg0, arg1 interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveVariables", reflect.TypeOf((*MockConfigService)(nil).ResolveVariables), arg0, arg1) } +// Save mocks base method. +func (m *MockConfigService) Save() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Save") + ret0, _ := ret[0].(error) + return ret0 +} + +// Save indicates an expected call of Save. +func (mr *MockConfigServiceMockRecorder) Save() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Save", reflect.TypeOf((*MockConfigService)(nil).Save)) +} + // MockTaskStateMachine is a mock of TaskStateMachine interface. type MockTaskStateMachine struct { ctrl *gomock.Controller diff --git a/internal/manager/config/defaults.go b/internal/manager/config/defaults.go index 03c9ba3f..fe5e2fd2 100644 --- a/internal/manager/config/defaults.go +++ b/internal/manager/config/defaults.go @@ -9,6 +9,8 @@ import ( // SPDX-License-Identifier: GPL-3.0-or-later +const DefaultBlenderArguments = "--factory-startup -b -y" + // The default configuration, use DefaultConfig() to obtain a copy. var defaultConfig = Conf{ Base: Base{ @@ -61,9 +63,9 @@ var defaultConfig = Conf{ // The default commands assume that the executables are available on $PATH. "blender": { Values: VariableValues{ - VariableValue{Platform: "linux", Value: "blender --factory-startup -b -y"}, - VariableValue{Platform: "windows", Value: "blender.exe --factory-startup -b -y"}, - VariableValue{Platform: "darwin", Value: "blender --factory-startup -b -y"}, + VariableValue{Platform: "linux", Value: "blender " + DefaultBlenderArguments}, + VariableValue{Platform: "windows", Value: "blender.exe " + DefaultBlenderArguments}, + VariableValue{Platform: "darwin", Value: "blender " + DefaultBlenderArguments}, }, }, "ffmpeg": { diff --git a/web/app/src/views/FirstTimeWizardView.vue b/web/app/src/views/FirstTimeWizardView.vue index 6f75fa48..84c06ad5 100644 --- a/web/app/src/views/FirstTimeWizardView.vue +++ b/web/app/src/views/FirstTimeWizardView.vue @@ -3,8 +3,8 @@

Welcome to Flamenco!

-

Before Flamenco can be used, a few things need to be set up.

-

This wizard will guide you through the configuration.

+

Before Flamenco can be used, a few things need to be set up. This + wizard will guide you through the configuration.

Shared Storage

@@ -13,10 +13,15 @@ Manager and Workers exchange files. This could be a NAS in your network, or some other file sharing server.

+

Make sure this path is the same for all machines involved.

+

Using a service like Syncthing, ownCloud, or Dropbox for this is not recommended, as Flamenco does not know when every machine has received the files.

+
@@ -33,7 +38,7 @@

Choose which Blender to use below:

-

... finding Blenders ...

+

... finding Blenders ...

@@ -46,7 +51,7 @@
Source
{{ sourceLabels[blender.source] }}
- +

Or provide an alternative command to try:

@@ -55,12 +60,35 @@
-

... checking ...

+

... checking ...

Found something, it is selected above.

{{ blenderExeCheckResult.cause }}

+ +
+

The Final Step

+

This is the configuration that will be used by Flamenco:

+
+
Storage
+
{{ sharedStorageCheckResult.path }}
+
Blender
+
+ Whatever Blender is associated with .blend files + (currently "{{ selectedBlender.path }}") +
+
+ The command "{{ selectedBlender.input }}" as found on $PATH + (currently "{{ selectedBlender.path }}") +
+
+ The command you provided: + "{{ selectedBlender.path }}" +
+
+ +