diff --git a/README.md b/README.md index cd6e476..6053b7f 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,69 @@ are ignored if present in a config file (unknown keys do not cause errors): `password_authentication`, `dh_file`, `stats_update_period`, `stats_db_update_period`, `grpc.enabled`, `task_manager.update_period` +## Command placeholders and server variables + +The daemon renders the game mod start command and the `scripts.*` wrappers +itself. Every `{placeholder}` is replaced once, in a single pass, so a +substituted value is never expanded again. + +### Built-in placeholders + +| Placeholder | Value +|-----------------------------------------|------------ +| `{dir}` | Absolute server directory (`work_path` + the server `dir`) +| `{work_dir}` | Absolute process working directory (see below) +| `{uuid}`, `{uuid_short}`, `{id}` | Server identifiers from the panel +| `{ip}`, `{host}` | Server IP +| `{port}`, `{SERVER_PORT}`, `{PORT}` | Connect port +| `{query_port}`, `{rcon_port}` | Query and RCON ports +| `{rcon_password}` | RCON password +| `{user}` | System user the server runs as +| `{game}` | Game start code; a mod variable named `game` wins (see below) +| `{node_work_path}`, `{node_tools_path}` | Daemon `work_path` and its `tools` subdirectory +| `{command}` | Wrapper templates only: the tokens of the server command + +### Game mod variables + +Every variable of the game mod (the `vars` list in the games catalogue) is a +placeholder too, in three spellings: the name as the catalogue stores it and its +all-lowercase and all-uppercase forms, so a variable `maxplayers` fills both +`{maxplayers}` and `{MAXPLAYERS}`. A mixed-case `{MaxPlayers}` is substituted +only when the catalogue spells the name that way. The value is resolved in this +order, the last one wins: + +1. the variable default from the game mod; +2. the server variables (`vars` on the admin server page); +3. the server settings (the server settings page). + +A built-in placeholder wins over a variable of the same name, except `{game}`: +the catalogue defines it as a mod variable (Quake 2 starts with +`+set game {game}` and a variable `game=baseq2`), so the variable wins and the +game start code is only the fallback. + +Values are substituted exactly as the panel stores them. Since GameAP 4.5 the +panel validates typed variables (`int`, `float`, `bool`, `select`, ...) and +stores the canonical text: a `bool` variable is its `true_value`/`false_value` +(`1`/`0` by default), a number is its decimal text. The daemon never parses or +converts a value. + +Templates are tokenized first and placeholders are substituted into the +individual tokens, so a value never splits into several arguments and shell +metacharacters in a value are harmless. An empty value yields an empty +argument: `+sv_setsteamaccount {server_token}` becomes +`+sv_setsteamaccount ""`. A flag-style `bool` variable with an empty +`false_value` should therefore be embedded in a larger token +(`--flag={var}`) rather than used as a standalone `{var}`. + +### Environment + +The merged variables (same order as above) are exported to the game server +process as environment variables. The name is upper-cased, `-` and spaces +become `_`, other characters are dropped (`max-players` becomes +`MAX_PLAYERS`). Then `HOME` is set when a `home_dir` is configured, and finally +`SERVER_PORT`, `PORT`, `QUERY_PORT` and `RCON_PORT`, which a variable cannot +override. + ## Game server working directory By default a game server process starts in the server directory diff --git a/internal/app/domain/commands.go b/internal/app/domain/commands.go index c2badd9..696ec59 100644 --- a/internal/app/domain/commands.go +++ b/internal/app/domain/commands.go @@ -109,6 +109,10 @@ func ReplaceShortCodes(commandTemplate string, cfg workDirReader, server *Server // {dir} is the server directory and {work_dir} the directory the process runs // in, even when a server variable is called work_dir. // +// {game} is the one exception: the games catalogue defines it as a mod variable +// (Quake 2 starts with "+set game {game}" and a variable game=baseq2), so a +// variable named game wins and the game start code is only the fallback. +// // It fails when the configured work_dir is not a relative path inside the // server directory; the same value is rejected again before the server starts. func newServerReplacer(cfg workDirReader, server *Server, paths CommandPaths) (*strings.Replacer, error) { @@ -148,7 +152,6 @@ func newServerReplacer(cfg workDirReader, server *Server, paths CommandPaths) (* "{query_port}", strconv.Itoa(server.QueryPort()), "{rcon_port}", strconv.Itoa(server.RCONPort()), "{rcon_password}", server.RCONPassword(), - "{game}", server.Game().StartCode, "{user}", server.User(), "{node_work_path}", cfg.WorkDir(), "{node_tools_path}", cfg.WorkDir()+"/tools", @@ -162,5 +165,7 @@ func newServerReplacer(cfg workDirReader, server *Server, paths CommandPaths) (* ) } + pairs = append(pairs, "{game}", server.Game().StartCode) + return strings.NewReplacer(pairs...), nil } diff --git a/internal/app/domain/commands_args_test.go b/internal/app/domain/commands_args_test.go index 16af477..212ce44 100644 --- a/internal/app/domain/commands_args_test.go +++ b/internal/app/domain/commands_args_test.go @@ -170,3 +170,23 @@ func TestBuildCommandArgsWithPaths_StillRejectsInvalidWorkDir(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "path is outside work directory") } + +func TestBuildCommandArgs_EmptyValueStaysAnEmptyArgument(t *testing.T) { + // The games catalogue relies on it: "+sv_setsteamaccount {server_token}" + // with an empty token must still hand the server an (empty) argument. + cfg := fakeWorkDirReader{workDir: "/work-path"} + server := newTestServerForVars( + []GameModVarTemplate{{Key: "server_token", DefaultValue: ""}}, + nil, + nil, + ) + + args, err := BuildCommandArgs( + cfg, server, + "{command}", + "./srcds_run +sv_setsteamaccount {server_token} +map de_dust2", + ) + + require.NoError(t, err) + assert.Equal(t, []string{"./srcds_run", "+sv_setsteamaccount", "", "+map", "de_dust2"}, args) +} diff --git a/internal/app/domain/commands_test.go b/internal/app/domain/commands_test.go index 643f50b..e00b4f9 100644 --- a/internal/app/domain/commands_test.go +++ b/internal/app/domain/commands_test.go @@ -169,3 +169,52 @@ func TestMakeFullCommand_InvalidWorkDirReturnsError(t *testing.T) { assert.Contains(t, err.Error(), `invalid work_dir "/abs" from server vars`) assert.Contains(t, err.Error(), "must be a path relative to the server directory") } + +func TestReplaceShortCodes_GamePlaceholder(t *testing.T) { + cfg := fakeWorkDirReader{workDir: "/work-path"} + + tests := []struct { + name string + game Game + gameModVars []GameModVarTemplate + settings Settings + expected string + }{ + { + name: "mod_variable_wins_over_start_code", + game: Game{StartCode: "q2"}, + gameModVars: []GameModVarTemplate{{Key: "game", DefaultValue: "baseq2"}}, + expected: "+set game baseq2", + }, + { + name: "server_setting_overrides_the_variable_default", + game: Game{StartCode: "q2"}, + gameModVars: []GameModVarTemplate{{Key: "game", DefaultValue: "baseq2"}}, + settings: Settings{"game": "ctf"}, + expected: "+set game ctf", + }, + { + name: "start_code_is_the_fallback", + game: Game{StartCode: "cstrike"}, + expected: "+set game cstrike", + }, + { + // The panel sends no start code over gRPC, so a mod without a game + // variable renders an empty value. + name: "empty_without_start_code_and_variable", + game: Game{}, + expected: "+set game ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := newTestServerForVarsWithGame(tt.game, tt.gameModVars, nil, tt.settings) + + result, err := ReplaceShortCodes("+set game {game}", cfg, server) + + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/internal/app/domain/game.go b/internal/app/domain/game.go index d2d977d..8f1a487 100644 --- a/internal/app/domain/game.go +++ b/internal/app/domain/game.go @@ -54,43 +54,8 @@ type SteamSettings struct { } type GameModVarTemplate struct { - Key string - DefaultValue string -} - -func (g *GameModVarTemplate) UnmarshalJSON(bytes []byte) error { - v := struct { - Var string `json:"var"` - Default interface{} `json:"default"` - Info string `json:"info"` - AdminVar bool `json:"admin_var"` - }{} - - err := json.Unmarshal(bytes, &v) - if err != nil { - return err - } - - g.Key = v.Var - - switch defaultValue := v.Default.(type) { - case string: - g.DefaultValue = defaultValue - case int: - g.DefaultValue = strconv.Itoa(defaultValue) - case float64: - g.DefaultValue = strconv.Itoa(int(defaultValue)) - case bool: - if v.Default.(bool) { - g.DefaultValue = "1" - } else { - g.DefaultValue = "0" - } - default: - g.DefaultValue = "" - } - - return nil + Key string `json:"var"` + DefaultValue string `json:"default"` } type GameMod struct { diff --git a/internal/app/domain/server_test.go b/internal/app/domain/server_test.go index 4ab7e3f..ef8d63b 100644 --- a/internal/app/domain/server_test.go +++ b/internal/app/domain/server_test.go @@ -13,6 +13,15 @@ func newTestServerForVars( gameModVars []GameModVarTemplate, vars map[string]string, settings Settings, +) *Server { + return newTestServerForVarsWithGame(Game{StartCode: "game"}, gameModVars, vars, settings) +} + +func newTestServerForVarsWithGame( + game Game, + gameModVars []GameModVarTemplate, + vars map[string]string, + settings Settings, ) *Server { return NewServer( 1, @@ -22,7 +31,7 @@ func newTestServerForVars( "test", "test-uuid", "test", - Game{StartCode: "game"}, + game, GameMod{Vars: gameModVars}, "127.0.0.1", 27015, diff --git a/internal/app/grpc/converters_test.go b/internal/app/grpc/converters_test.go new file mode 100644 index 0000000..aaf101f --- /dev/null +++ b/internal/app/grpc/converters_test.go @@ -0,0 +1,112 @@ +package grpc + +import ( + "testing" + + "github.com/gameap/daemon/internal/app/domain" + pb "github.com/gameap/gameap/pkg/proto" + "github.com/stretchr/testify/assert" +) + +func TestProtoGameModToDomain_KeepsVarDefaultsVerbatim(t *testing.T) { + // The panel canonicalizes every variable before it is sent: a bool is its + // true_value/false_value text, a number is its decimal text. Nothing of it + // may be reinterpreted here. + protoVars := []*pb.GameModVar{ + {Var: "ase", Default: "1", Info: "All Seeing Eye", AdminVar: true}, + {Var: "pvp", Default: "0"}, + {Var: "debug", Default: "true"}, + {Var: "hardcore", Default: "false"}, + {Var: "code", Default: "007"}, + {Var: "ratio", Default: "1.50"}, + {Var: "password", Default: ""}, + {Var: "game", Default: "baseq2"}, + {Var: "SERVER_TOKEN", Default: "abc"}, + } + + gameMod := ProtoGameModToDomain(&pb.GameMod{Id: 10, GameCode: "q2", Name: "Default", Vars: protoVars}) + + expected := make([]domain.GameModVarTemplate, 0, len(protoVars)) + for _, v := range protoVars { + expected = append(expected, domain.GameModVarTemplate{Key: v.Var, DefaultValue: v.Default}) + } + + assert.Equal(t, expected, gameMod.Vars) +} + +func TestParseProtoSettings(t *testing.T) { + tests := []struct { + name string + settings []*pb.ServerSetting + expected domain.Settings + }{ + { + name: "nil_slice_means_no_settings", + settings: nil, + expected: nil, + }, + { + name: "empty_slice_means_no_settings", + settings: []*pb.ServerSetting{}, + expected: nil, + }, + { + name: "values_are_kept_verbatim", + settings: []*pb.ServerSetting{ + {Name: "ase", Value: "0"}, + {Name: "code", Value: "007"}, + {Name: "ratio", Value: "1.50"}, + {Name: "password", Value: ""}, + {Name: "autostart", Value: "true"}, + }, + expected: domain.Settings{ + "ase": "0", + "code": "007", + "ratio": "1.50", + "password": "", + "autostart": "true", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, parseProtoSettings(tt.settings)) + }) + } +} + +func TestParseVarsJSON(t *testing.T) { + tests := []struct { + name string + input string + expected map[string]string + }{ + { + name: "empty_input", + input: "", + expected: map[string]string{}, + }, + { + name: "values_are_kept_verbatim", + input: `{"map":"q2dm1","code":"007","empty":""}`, + expected: map[string]string{"map": "q2dm1", "code": "007", "empty": ""}, + }, + { + name: "invalid_json_yields_no_vars", + input: `{"map":`, + expected: map[string]string{}, + }, + { + name: "non_string_value_yields_no_vars", + input: `{"maxplayers": 32}`, + expected: map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, parseVarsJSON(tt.input)) + }) + } +} diff --git a/internal/app/grpc/server_handler_test.go b/internal/app/grpc/server_handler_test.go new file mode 100644 index 0000000..0ff4295 --- /dev/null +++ b/internal/app/grpc/server_handler_test.go @@ -0,0 +1,231 @@ +package grpc + +import ( + "context" + "testing" + + "github.com/gameap/daemon/internal/app/config" + "github.com/gameap/daemon/internal/app/domain" + "github.com/gameap/daemon/test/mocks" + pb "github.com/gameap/gameap/pkg/proto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testServerID = 1 + testGameModID = 10 +) + +func newTestServerHandler(vars []*pb.GameModVar) (*GRPCServerHandler, *mocks.ServerRepository) { + store := NewGameStore() + store.UpdateGames([]*pb.Game{{Code: "q2", Name: "Quake 2"}}) + store.UpdateGameMods([]*pb.GameMod{{Id: testGameModID, GameCode: "q2", Name: "Default", Vars: vars}}) + + repo := mocks.NewServerRepository() + + return NewGRPCServerHandler(repo, store), repo +} + +func newTestProtoServer(startCommand, varsJSON string) *pb.Server { + srv := &pb.Server{ + Id: testServerID, + Uuid: "759b875e-d910-11eb-aff7-d796d7fcf7ef", + UuidShort: "759b875e", + Enabled: true, + Installed: pb.ServerInstalledStatus_SERVER_INSTALLED_STATUS_INSTALLED, + Name: "Quake 2 server", + GameId: "q2", + GameModId: testGameModID, + ServerIp: "127.0.0.1", + ServerPort: 27910, + Dir: "servers/q2", + StartCommand: new(startCommand), + } + + if varsJSON != "" { + srv.Vars = new(varsJSON) + } + + return srv +} + +func handledServer( + t *testing.T, + handler *GRPCServerHandler, + repo *mocks.ServerRepository, + srv *pb.Server, + settings []*pb.ServerSetting, +) *domain.Server { + t.Helper() + + require.NoError(t, handler.HandleServerConfigUpdate(context.Background(), srv, settings)) + + server, found := repo.FindByIDFromCache(testServerID) + require.True(t, found, "server must be cached after a config update") + + return server +} + +func TestHandleServerConfigUpdate_ValuesAreUsedVerbatim(t *testing.T) { + cfg := &config.Config{WorkPath: "/work-path"} + + tests := []struct { + name string + modVars []*pb.GameModVar + varsJSON string + settings []*pb.ServerSetting + startCommand string + wantRendered string + wantArgs []string + wantEnv map[string]string + }{ + { + name: "bool_true_value_text", + modVars: []*pb.GameModVar{{Var: "ase", Default: "0"}}, + settings: []*pb.ServerSetting{{Name: "ase", Value: "1"}}, + startCommand: "./run.sh --ase={ase}", + wantRendered: "./run.sh --ase=1", + wantArgs: []string{"./run.sh", "--ase=1"}, + wantEnv: map[string]string{"ASE": "1"}, + }, + { + name: "bool_false_value_text_is_not_parsed_as_truthiness", + modVars: []*pb.GameModVar{{Var: "ase", Default: "1"}}, + settings: []*pb.ServerSetting{{Name: "ase", Value: "0"}}, + startCommand: "./run.sh --ase={ase}", + wantRendered: "./run.sh --ase=0", + wantArgs: []string{"./run.sh", "--ase=0"}, + wantEnv: map[string]string{"ASE": "0"}, + }, + { + name: "empty_setting_overrides_a_non_empty_default", + modVars: []*pb.GameModVar{{Var: "map", Default: "q2dm1"}}, + settings: []*pb.ServerSetting{{Name: "map", Value: ""}}, + startCommand: "./run.sh +map {map}", + wantRendered: "./run.sh +map ", + wantArgs: []string{"./run.sh", "+map", ""}, + wantEnv: map[string]string{"MAP": ""}, + }, + { + name: "numeric_and_boolean_looking_text_is_untouched", + modVars: []*pb.GameModVar{ + {Var: "code", Default: "1"}, + {Var: "ratio", Default: "1"}, + {Var: "debug", Default: "false"}, + }, + settings: []*pb.ServerSetting{ + {Name: "code", Value: "007"}, + {Name: "ratio", Value: "1.50"}, + {Name: "debug", Value: "true"}, + }, + startCommand: "./run.sh -code {code} -ratio {ratio} -debug {debug}", + wantRendered: "./run.sh -code 007 -ratio 1.50 -debug true", + wantArgs: []string{"./run.sh", "-code", "007", "-ratio", "1.50", "-debug", "true"}, + wantEnv: map[string]string{"CODE": "007", "RATIO": "1.50", "DEBUG": "true"}, + }, + { + name: "numeric_text_from_server_vars_is_untouched", + modVars: []*pb.GameModVar{{Var: "code", Default: "1"}}, + varsJSON: `{"code":"007"}`, + startCommand: "./run.sh -code {code}", + wantRendered: "./run.sh -code 007", + wantArgs: []string{"./run.sh", "-code", "007"}, + wantEnv: map[string]string{"CODE": "007"}, + }, + { + name: "uppercase_pelican_variable_name", + modVars: []*pb.GameModVar{{Var: "SERVER_TOKEN", Default: ""}}, + settings: []*pb.ServerSetting{{Name: "SERVER_TOKEN", Value: "abc"}}, + startCommand: "./run.sh +token {SERVER_TOKEN} +token {server_token}", + wantRendered: "./run.sh +token abc +token abc", + wantArgs: []string{"./run.sh", "+token", "abc", "+token", "abc"}, + wantEnv: map[string]string{"SERVER_TOKEN": "abc"}, + }, + { + name: "empty_default_stays_an_empty_argument", + modVars: []*pb.GameModVar{{Var: "server_token", Default: ""}}, + startCommand: "./srcds_run +sv_setsteamaccount {server_token} +map de_dust2", + wantRendered: "./srcds_run +sv_setsteamaccount +map de_dust2", + wantArgs: []string{"./srcds_run", "+sv_setsteamaccount", "", "+map", "de_dust2"}, + wantEnv: map[string]string{"SERVER_TOKEN": ""}, + }, + { + name: "quake2_game_variable_is_not_shadowed_by_the_game_placeholder", + modVars: []*pb.GameModVar{{Var: "game", Default: "baseq2"}}, + startCommand: "./run.sh +set game {game} +map q2dm1", + wantRendered: "./run.sh +set game baseq2 +map q2dm1", + wantArgs: []string{"./run.sh", "+set", "game", "baseq2", "+map", "q2dm1"}, + wantEnv: map[string]string{"GAME": "baseq2"}, + }, + { + name: "password_with_spaces_stays_a_single_argument", + modVars: []*pb.GameModVar{{Var: "password", Default: ""}}, + settings: []*pb.ServerSetting{{Name: "password", Value: "p@ss w0rd"}}, + startCommand: `./run.sh --password="{password}"`, + wantRendered: `./run.sh --password="p@ss w0rd"`, + wantArgs: []string{"./run.sh", "--password=p@ss w0rd"}, + wantEnv: map[string]string{"PASSWORD": "p@ss w0rd"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler, repo := newTestServerHandler(tt.modVars) + + server := handledServer(t, handler, repo, newTestProtoServer(tt.startCommand, tt.varsJSON), tt.settings) + + rendered, err := domain.ReplaceShortCodes(server.StartCommand(), cfg, server) + require.NoError(t, err) + assert.Equal(t, tt.wantRendered, rendered) + + args, err := domain.BuildCommandArgs(cfg, server, "{command}", server.StartCommand()) + require.NoError(t, err) + assert.Equal(t, tt.wantArgs, args) + + env, err := server.EnvironmentVars(cfg) + require.NoError(t, err) + for key, want := range tt.wantEnv { + assert.Equal(t, want, env[key], "env %s", key) + } + }) + } +} + +func TestHandleServerConfigUpdate_SettingsWinOverVarsWinOverDefaults(t *testing.T) { + handler, repo := newTestServerHandler([]*pb.GameModVar{ + {Var: "from_default", Default: "default"}, + {Var: "from_vars", Default: "default"}, + {Var: "from_settings", Default: "default"}, + }) + srv := newTestProtoServer("./run.sh", `{"from_vars":"vars","from_settings":"vars"}`) + + server := handledServer(t, handler, repo, srv, []*pb.ServerSetting{{Name: "from_settings", Value: "settings"}}) + + assert.Equal(t, map[string]string{ + "from_default": "default", + "from_vars": "vars", + "from_settings": "settings", + }, server.Vars()) +} + +func TestHandleServerConfigUpdate_SettingsArePreservedWhenNotSent(t *testing.T) { + handler, repo := newTestServerHandler([]*pb.GameModVar{{Var: "map", Default: "q2dm1"}}) + srv := newTestProtoServer("./run.sh", "") + initial := []*pb.ServerSetting{{Name: "map", Value: "q2dm2"}} + + server := handledServer(t, handler, repo, srv, initial) + require.Equal(t, domain.Settings{"map": "q2dm2"}, server.AllSettings()) + + require.NoError(t, handler.HandleServerUpdate(context.Background(), srv)) + assert.Equal(t, domain.Settings{"map": "q2dm2"}, server.AllSettings(), + "a server_config message without settings keeps the existing ones") + + server = handledServer(t, handler, repo, srv, []*pb.ServerSetting{}) + assert.Equal(t, domain.Settings{"map": "q2dm2"}, server.AllSettings(), + "an empty settings list keeps the existing ones") + + server = handledServer(t, handler, repo, srv, []*pb.ServerSetting{{Name: "other", Value: "x"}}) + assert.Equal(t, domain.Settings{"other": "x"}, server.AllSettings(), + "a non-empty settings list replaces the existing ones wholesale") +} diff --git a/internal/processmanager/errors.go b/internal/processmanager/errors.go index 2744409..b6a8d7f 100644 --- a/internal/processmanager/errors.go +++ b/internal/processmanager/errors.go @@ -14,6 +14,7 @@ var ( ErrServiceNotFound = errors.New("service is not registered") ErrServiceStateTimeout = errors.New("timeout waiting for the service to change state") ErrServiceStoppedOnStart = errors.New("service stopped immediately after start") + ErrSocketStopFailed = errors.New("failed to stop socket unit") ErrUserMismatch = errors.New( "server user does not match daemon user (required for systemctl --user mode)", ) diff --git a/internal/processmanager/systemd.go b/internal/processmanager/systemd.go index f419b7a..bc526df 100644 --- a/internal/processmanager/systemd.go +++ b/internal/processmanager/systemd.go @@ -309,14 +309,59 @@ func (pm *SystemD) Start(ctx context.Context, server *domain.Server, out io.Writ return domain.ErrorResult, errors.Wrap(err, "failed to close file") } - if _, err = os.Stat(pm.stdinFile(server)); err == nil { - err = os.Remove(pm.stdinFile(server)) + if err = pm.resetStdin(ctx, server, out); err != nil { + return domain.ErrorResult, errors.WithMessage(err, "failed to reset stdin FIFO") + } + + return pm.command(ctx, server, "start", out) +} + +// resetStdin drops the stdin FIFO of a previous run, so that input queued while +// the server was down is not replayed into the new process. +// +// The FIFO belongs to the socket unit: systemd keeps the descriptor open for as +// long as the socket is active and hands that same descriptor to the service as +// stdin. A server that exited on its own leaves its socket listening, so +// unlinking only the path would start the service on the orphaned inode while +// SendInput keeps opening a path that no longer exists. Stopping the socket +// first makes the start that follows recreate the FIFO. A running service is +// left alone: taking the FIFO away would cut its console input. +func (pm *SystemD) resetStdin(ctx context.Context, server *domain.Server, out io.Writer) error { + serviceStatus, err := pm.status(ctx, pm.resolveServiceName(server), out) + if err != nil { + return errors.WithMessage(err, "failed to get service status") + } + if serviceStatus == domain.SuccessResult { + return nil + } + + socketName := pm.resolveSocketName(server) + + socketStatus, err := pm.status(ctx, socketName, out) + if err != nil { + return errors.WithMessage(err, "failed to get socket status") + } + if socketStatus == domain.SuccessResult { + // A stop that did not succeed leaves the socket listening, so the FIFO + // has to stay: unlinking it now would start the service on the orphaned + // inode that SendInput can no longer reach. + result, err := pm.executor.ExecWithWriter(ctx, pm.systemctl("stop", socketName), out, pm.execOpts()) if err != nil { - return domain.ErrorResult, errors.WithMessage(err, "failed to remove file") + return errors.WithMessagef(err, "failed to stop socket %s", socketName) + } + if domain.Result(result) != domain.SuccessResult { + return errors.WithMessagef( + ErrSocketStopFailed, "systemctl stop %s exited with code %d", socketName, result, + ) } } - return pm.command(ctx, server, "start", out) + err = os.Remove(pm.stdinFile(server)) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return errors.Wrapf(err, "failed to remove stdin FIFO %s", pm.stdinFile(server)) + } + + return nil } func (pm *SystemD) Stop(ctx context.Context, server *domain.Server, out io.Writer) (domain.Result, error) { diff --git a/internal/processmanager/systemd_internal_test.go b/internal/processmanager/systemd_internal_test.go index 76829d9..9857019 100644 --- a/internal/processmanager/systemd_internal_test.go +++ b/internal/processmanager/systemd_internal_test.go @@ -3,6 +3,8 @@ package processmanager import ( + "context" + "io" "os" "os/user" "path/filepath" @@ -11,6 +13,7 @@ import ( "time" "github.com/gameap/daemon/internal/app/config" + "github.com/gameap/daemon/internal/app/contracts" "github.com/gameap/daemon/internal/app/domain" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -662,3 +665,166 @@ func Test_buildServiceConfig_homeDir(t *testing.T) { assert.Contains(t, err.Error(), `invalid home_dir "../escape" from server vars`) }) } + +// unitStateExecutor is a contracts.Executor double that answers `systemctl show` +// with a per-unit ActiveState and records every other command it is given. +type unitStateExecutor struct { + activeStates map[string]string + commands []string + onCommand func(command string) + // execResult is the exit code ExecWithWriter reports for every command. + execResult int +} + +func (e *unitStateExecutor) Exec(_ context.Context, command string, _ contracts.ExecutorOptions) ([]byte, int, error) { + if !strings.Contains(command, " show ") { + e.record(command) + + return nil, 0, nil + } + + for unit, state := range e.activeStates { + if strings.Contains(command, " "+unit+" ") { + return []byte("LoadState=loaded\nActiveState=" + state + "\nSubState=x\nResult=success\n"), 0, nil + } + } + + return []byte("LoadState=not-found\nActiveState=inactive\nSubState=dead\nResult=success\n"), 0, nil +} + +func (e *unitStateExecutor) ExecWithWriter( + _ context.Context, command string, _ io.Writer, _ contracts.ExecutorOptions, +) (int, error) { + e.record(command) + + return e.execResult, nil +} + +func (e *unitStateExecutor) ExecArgs(_ context.Context, args []string, _ contracts.ExecutorOptions) ([]byte, int, error) { + e.record(strings.Join(args, " ")) + + return nil, 0, nil +} + +func (e *unitStateExecutor) ExecWithWriterArgs( + _ context.Context, args []string, _ io.Writer, _ contracts.ExecutorOptions, +) (int, error) { + e.record(strings.Join(args, " ")) + + return 0, nil +} + +func (e *unitStateExecutor) record(command string) { + e.commands = append(e.commands, command) + + if e.onCommand != nil { + e.onCommand(command) + } +} + +func Test_resetStdin(t *testing.T) { + cur, err := user.Current() + require.NoError(t, err) + + setup := func(t *testing.T, activeStates map[string]string) (*SystemD, *unitStateExecutor, *domain.Server, string) { + t.Helper() + + workDir := t.TempDir() + server := makeServerWithUser(cur.Username, workDir) + + executor := &unitStateExecutor{activeStates: map[string]string{}} + + cfg := makeConfigWithScope("") + cfg.WorkPath = workDir + + pm := NewSystemD(cfg, nil, executor) + pm.servicesDir = t.TempDir() + + for unit, state := range activeStates { + switch unit { + case "service": + executor.activeStates[pm.serviceName(server)] = state + case "socket": + executor.activeStates[pm.socketName(server)] = state + } + } + + stdinFile := pm.stdinFile(server) + require.NoError(t, os.MkdirAll(filepath.Dir(stdinFile), 0o755)) + require.NoError(t, os.WriteFile(stdinFile, nil, 0o600)) + + return pm, executor, server, stdinFile + } + + t.Run("a listening socket left by a server that exited on its own is stopped before the FIFO goes", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, map[string]string{"service": "inactive", "socket": "active"}) + + fifoPresentAtStop := false + executor.onCommand = func(string) { + _, statErr := os.Stat(stdinFile) + fifoPresentAtStop = statErr == nil + } + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + + assert.Equal(t, []string{"systemctl stop " + pm.socketName(server)}, executor.commands) + assert.True(t, fifoPresentAtStop, "the socket must be stopped while it still owns the FIFO path") + assert.NoFileExists(t, stdinFile) + }) + + t.Run("a failed service counts as down", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, map[string]string{"service": "failed", "socket": "active"}) + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + + assert.Equal(t, []string{"systemctl stop " + pm.socketName(server)}, executor.commands) + assert.NoFileExists(t, stdinFile) + }) + + t.Run("an inactive socket is not stopped again", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, map[string]string{"service": "inactive", "socket": "inactive"}) + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + + assert.Empty(t, executor.commands) + assert.NoFileExists(t, stdinFile) + }) + + t.Run("units that were never created only lose a stale FIFO", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, nil) + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + + assert.Empty(t, executor.commands) + assert.NoFileExists(t, stdinFile) + }) + + for _, state := range []string{"active", "activating"} { + t.Run("a service that is "+state+" keeps its FIFO", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, map[string]string{"service": state, "socket": "active"}) + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + + assert.Empty(t, executor.commands) + assert.FileExists(t, stdinFile) + }) + } + + t.Run("a socket that refuses to stop keeps its FIFO", func(t *testing.T) { + pm, executor, server, stdinFile := setup(t, map[string]string{"service": "inactive", "socket": "active"}) + executor.execResult = 1 + + err := pm.resetStdin(context.Background(), server, io.Discard) + + require.ErrorIs(t, err, ErrSocketStopFailed) + assert.Equal(t, []string{"systemctl stop " + pm.socketName(server)}, executor.commands) + assert.FileExists(t, stdinFile) + }) + + t.Run("a missing FIFO is not an error", func(t *testing.T) { + pm, _, server, stdinFile := setup(t, map[string]string{"service": "inactive", "socket": "inactive"}) + require.NoError(t, os.Remove(stdinFile)) + + require.NoError(t, pm.resetStdin(context.Background(), server, io.Discard)) + }) +}