From 19e747f1669d8ae64619686569929a59d5722b8c Mon Sep 17 00:00:00 2001 From: Julien Dessaux Date: Wed, 13 Jan 2021 15:38:19 +0100 Subject: Finished implementing config tests --- config/app_test.go | 26 ++++++- config/config.go | 8 +- config/config_test.go | 24 +++--- config/game.go | 36 ++++++++- config/game_test.go | 103 +++++++++++++++++++++++++- config/menu.go | 7 +- config/test_data/duplicate_game.yaml | 30 ++++++++ config/test_data/fake_nethack_directory/.keep | 0 config/test_data/invalid_game.yaml | 21 ++++++ config/test_data/unreachable_game.yaml | 4 + example/complete.yaml | 5 +- 11 files changed, 228 insertions(+), 36 deletions(-) create mode 100644 config/test_data/duplicate_game.yaml create mode 100644 config/test_data/fake_nethack_directory/.keep create mode 100644 config/test_data/invalid_game.yaml diff --git a/config/app_test.go b/config/app_test.go index cda891d..5a6cf41 100644 --- a/config/app_test.go +++ b/config/app_test.go @@ -55,15 +55,35 @@ func TestAppvalidate(t *testing.T) { t.Fatal("Negative or zero MenuMaxIdleTime should not be valid.") } - //PostLoginCommands is tested from command.go - - // A valid App + //PostLoginCommands are mostly tested from command_test.go app = App{ WorkingDirectory: "var/", MaxUsers: 512, MaxNickLen: 15, MenuMaxIdleTime: 60, } + if err := app.validate(); err != nil { + t.Fatal("Empty PostLoginCommands list should be valid") + } + app = App{ + WorkingDirectory: "var/", + MaxUsers: 512, + MaxNickLen: 15, + MenuMaxIdleTime: 60, + PostLoginCommands: []string{"invalid"}, + } + if err := app.validate(); err == nil { + t.Fatal("Invalid command in PostLoginCommands should not be valid") + } + + // A valid App + app = App{ + WorkingDirectory: "var/", + MaxUsers: 512, + MaxNickLen: 15, + MenuMaxIdleTime: 60, + PostLoginCommands: []string{"wait"}, + } if err := app.validate(); err != nil { t.Fatal("A valid app should pass") } diff --git a/config/config.go b/config/config.go index aa1672a..9a161ed 100644 --- a/config/config.go +++ b/config/config.go @@ -44,20 +44,20 @@ func LoadFile(path string) (config Config, err error) { var f *os.File f, err = os.Open(path) if err != nil { - return + return config, errors.Wrapf(err, "Failed to open configuration file %s", path) } defer f.Close() decoder := yaml.NewDecoder(f) if err = decoder.Decode(&config); err != nil { - return + return config, errors.Wrap(err, "Failed to decode configuration file") } if err = config.validate(); err != nil { - return + return config, errors.Wrap(err, "Failed to validate configuration") } // If all looks good we validate menu consistency for _, v := range config.Menus { if err = v.validateConsistency(&config); err != nil { - return + return config, errors.Wrap(err, "Failed menu consistency checks") } } return diff --git a/config/config_test.go b/config/config_test.go index 44df5ed..2bc5489 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -54,16 +54,6 @@ func TestLoadFile(t *testing.T) { t.Fatalf("minimal example failed:\nerror %v\nwant:%+v\ngot: %+v", err, want, config) } - // TODO test non existant game in play actions, and duplicate - //menuEntry = MenuEntry{ - //Key: "p", - //Label: "play non existant game", - //Action: "play nonexistant", - //} - //if err := menuEntry.validate(); err == nil { - //t.Fatal("An inexistant game cannot be played") - //} - t.Cleanup(func() { os.RemoveAll("var/") }) // Invalid App if _, err := LoadFile("test_data/invalid_app.yaml"); err == nil { @@ -101,10 +91,18 @@ func TestLoadFile(t *testing.T) { if _, err := LoadFile("test_data/unreachable_menu.yaml"); err == nil { t.Fatal("unreachable menu should fail to load") } + // invalid game + if _, err := LoadFile("test_data/invalid_game.yaml"); err == nil { + t.Fatal("invalid game should fail to load") + } // unreachable game if _, err := LoadFile("test_data/unreachable_game.yaml"); err == nil { t.Fatal("unreachable game should fail to load") } + // duplicate game + if _, err := LoadFile("test_data/duplicate_game.yaml"); err == nil { + t.Fatal("unreachable game should fail to load") + } // Complexe example want = Config{ @@ -205,12 +203,8 @@ func TestLoadFile(t *testing.T) { }, Games: map[string]Game{ "nethack3.7": Game{ - ChrootPath: "/opt/nethack", + ChrootPath: "test_data/fake_nethack_directory", FileMode: "0666", - ScoreCommands: []string{ - "exec /games/nethack -s all", - "wait", - }, Commands: []string{ "cp /games/var/save/%u%n.gz /games/var/save/%u%n.gz.bak", "exec /games/nethack -u %n", diff --git a/config/game.go b/config/game.go index c34c697..b5f4d39 100644 --- a/config/game.go +++ b/config/game.go @@ -1,11 +1,15 @@ package config import ( - "errors" "regexp" + + "github.com/pkg/errors" + "golang.org/x/sys/unix" ) var reValidGameName = regexp.MustCompile(`^[\w\._]+$`) +var reValidFileMode = regexp.MustCompile(`^0[\d]{3}$`) +var reSpace = regexp.MustCompile(`^\s$`) // Game struct containers the configuration for a game type Game struct { @@ -15,8 +19,6 @@ type Game struct { FileMode string `yaml:"FileMode"` // Commands is the command list Commands []string `yaml:"Commands"` - // ScoreFile is relative to the chroot path for the game - ScoreCommands []string `yaml:"ScoreCommands"` // Env is the environment in which to exec the commands Env map[string]string `yaml:"Env"` } @@ -27,9 +29,35 @@ func (g *Game) validate(name string) error { return errors.New("Invalid Game name, must match regex `^[\\w\\._]+$` : " + name) } // ChrootPath TODO + if err := unix.Access(g.ChrootPath, unix.R_OK|unix.X_OK); err != nil { + return errors.Wrapf(err, "Invalid ChrootPath : %s", g.ChrootPath) + } // FileMode + if ok := reValidFileMode.MatchString(g.FileMode); !ok { + return errors.New("Invalid File Mode, must match regex `^0[\\d]{3}$` : " + name) + } // Commands - // ScoreFile + if len(g.Commands) == 0 { + return errors.New("Invalid game " + name + " has no commands") + } + for i := 0; i < len(g.Commands); i++ { + if err := validateCommand(g.Commands[i]); err != nil { + return errors.Wrapf(err, "Failed to validate Commands for game %s", name) + } + } // Env + for k, _ := range g.Env { + for _, c := range k { + switch c { + case '=': + return errors.New("Environment variable key must not contain equal sign") + case '\000': + return errors.New("Environment variable key must not contain null character") + } + if reSpace.MatchString(string(c)) { + return errors.New("Environment variable key must not contain spaces") + } + } + } return nil } diff --git a/config/game_test.go b/config/game_test.go index d295ce2..b3bf107 100644 --- a/config/game_test.go +++ b/config/game_test.go @@ -3,8 +3,105 @@ package config import "testing" func TestGameValidate(t *testing.T) { - empty := Game{} - if err := empty.validate("invalid game name because of spaces"); err == nil { - t.Fatal("invalid game name") + // Game name + game := Game{} + if err := game.validate("invalid game name because of spaces"); err == nil { + t.Fatal("game name with spaces should not be valid") + } + // ChrootPath + game = Game{ChrootPath: "test_data/non_existant"} + if err := game.validate("test"); err == nil { + t.Fatal("non_existant ChrootPath should not be valid") + } + // FileMode + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + } + if err := game.validate("test"); err == nil { + t.Fatal("Invalid FileMode should not be valid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "abcd", + } + if err := game.validate("test"); err == nil { + t.Fatal("Invalid FileMode should not be valid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "777", + } + if err := game.validate("test"); err == nil { + t.Fatal("Invalid FileMode should not be valid") + } + // Commands are mostly tested from command_test.go + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + } + if err := game.validate("test"); err == nil { + t.Fatal("Empty Commands list should not be valid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"invalid"}, + } + if err := game.validate("test"); err == nil { + t.Fatal("Invalid command in Commands should not be valid") + } + // Env + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"wait"}, + } + if err := game.validate("test"); err != nil { + t.Fatal("Empty env list should be valid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"wait"}, + Env: map[string]string{ + "test invalid": "test", + }, + } + if err := game.validate("test"); err == nil { + t.Fatal("Spaces in environnement variable name are invalid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"wait"}, + Env: map[string]string{ + "test\000invalid": "test", + }, + } + if err := game.validate("test"); err == nil { + t.Fatal("null character in environnement variable name are invalid") + } + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"wait"}, + Env: map[string]string{ + "test=invalid": "test", + }, + } + if err := game.validate("test"); err == nil { + t.Fatal("equals symbol in environnement variable name are invalid") + } + // Valid Game entry + game = Game{ + ChrootPath: "test_data/fake_nethack_directory", + FileMode: "0777", + Commands: []string{"wait"}, + Env: map[string]string{ + "test": "test", + }, + } + if err := game.validate("test"); err != nil { + t.Fatalf("Valid game entry should pass but got error %s", err) } } diff --git a/config/menu.go b/config/menu.go index f405ec5..a8ea7b0 100644 --- a/config/menu.go +++ b/config/menu.go @@ -70,9 +70,10 @@ func (m *Menu) validateConsistency(c *Config) error { return errors.New("No logged_in menu declared") } // Validate actions - menus := make(map[string]bool) - menus["anonymous"] = true - menus["logged_in"] = true + menus := map[string]bool{ + "anonymous": true, + "logged_in": true, + } playable := make(map[string]bool) for k, v := range c.Menus { for _, e := range v.MenuEntries { diff --git a/config/test_data/duplicate_game.yaml b/config/test_data/duplicate_game.yaml new file mode 100644 index 0000000..f01a017 --- /dev/null +++ b/config/test_data/duplicate_game.yaml @@ -0,0 +1,30 @@ +App: + WorkingDirectory: var/ + MaxUsers: 1 + AllowRegistration: true + MaxNickLen: 15 + MenuMaxIdleTime: 600 + +Menus: + anonymous: + MenuEntries: + - Key: q + Label: quit + Action: quit + logged_in: + MenuEntries: + - Key: p + Label: play + Action: play test + +Games: + test: + ChrootPath: test_data/fake_nethack_directory + FileMode: 0777 + Commands: + - wait + test: + ChrootPath: test_data/fake_nethack_directory + FileMode: 0777 + Commands: + - wait diff --git a/config/test_data/fake_nethack_directory/.keep b/config/test_data/fake_nethack_directory/.keep new file mode 100644 index 0000000..e69de29 diff --git a/config/test_data/invalid_game.yaml b/config/test_data/invalid_game.yaml new file mode 100644 index 0000000..d58c3ee --- /dev/null +++ b/config/test_data/invalid_game.yaml @@ -0,0 +1,21 @@ +App: + WorkingDirectory: var/ + MaxUsers: 1 + AllowRegistration: true + MaxNickLen: 15 + MenuMaxIdleTime: 600 + +Menus: + anonymous: + MenuEntries: + - Key: q + Label: quit + Action: quit + logged_in: + MenuEntries: + - Key: p + Label: play + Action: play test + +Games: + test: diff --git a/config/test_data/unreachable_game.yaml b/config/test_data/unreachable_game.yaml index 5af610b..f2f22e2 100644 --- a/config/test_data/unreachable_game.yaml +++ b/config/test_data/unreachable_game.yaml @@ -19,3 +19,7 @@ Menus: Games: unreachable: + ChrootPath: test_data/fake_nethack_directory + FileMode: 0777 + Commands: + - wait diff --git a/example/complete.yaml b/example/complete.yaml index 2677285..18a33d7 100644 --- a/example/complete.yaml +++ b/example/complete.yaml @@ -64,11 +64,8 @@ Menus: Games: nethack3.7: - ChrootPath: /opt/nethack + ChrootPath: test_data/fake_nethack_directory FileMode: "0666" - ScoreCommands: - - exec /games/nethack -s all - - wait Commands: - cp /games/var/save/%u%n.gz /games/var/save/%u%n.gz.bak - exec /games/nethack -u %n -- cgit v1.2.3