From 1db3be532bd527debe3cab416f348cb75aa9402f Mon Sep 17 00:00:00 2001 From: Hugo Rosnet Date: Mon, 13 Jun 2016 15:32:19 +0200 Subject: [PATCH] dir-feat: Make loading from directory recursively possible The commit adds the following modifications: * Possibility to load hooks recursively from a directory * Extraction of the loadinghook method in webhook.go * Tests done on a fairly complex tree of directory Allowing hooks to be loaded from a directory is a nice feature, as it allows a better organisation/readibility of the hooks that we have. The LoadFromDir method rely on LoadFromFile and is meant to be very permissive, and will return errors only when no hooks have been loaded. Otherwise it will simply return 'nil' and the potential issues/warnings that happen during the loading of hooks (invalid file, invalid path, issue opening, etc). --- hook/hook.go | 43 ++++++++++ hook/hook_test.go | 34 ++++++++ .../depth2/depth3/depth4/empty_file.example | 0 .../depth1/depth2/hooks.json.example | 79 +++++++++++++++++++ test/hooks_dir.test/depth1/hooks.json.example | 79 +++++++++++++++++++ .../depth1b/hooks-invalid.json.example | 74 +++++++++++++++++ webhook.go | 54 ++++++++----- 7 files changed, 345 insertions(+), 18 deletions(-) create mode 100644 test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example create mode 100644 test/hooks_dir.test/depth1/depth2/hooks.json.example create mode 100644 test/hooks_dir.test/depth1/hooks.json.example create mode 100644 test/hooks_dir.test/depth1b/hooks-invalid.json.example diff --git a/hook/hook.go b/hook/hook.go index d0115e8..93101f6 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -12,6 +12,7 @@ import ( "regexp" "strconv" "strings" + "os" ) // Constants used to specify the parameter source @@ -400,6 +401,48 @@ func (h *Hooks) LoadFromFile(path string) error { return e } +// LoadFromDir attempts to load hooks from specified Dir +func (h *Hooks) LoadFromDir(path string) ([]string, error) { + warnings := []string{} + loaded := fmt.Errorf("no single hooks loaded from directory %s", path) + + if _, e := os.Stat(path); os.IsNotExist(e) { + if path == "" {return []string{"path '" + path + "' is unspecified"}, nil} + return []string{"path '" + path + "' is invalid"}, loaded + } + + files, e := ioutil.ReadDir(path) + if e != nil { + return []string{path + " issue while readdir"}, e + } else if len(files) == 0 { + return []string{path + " is empty"}, loaded + } + + // add '/' to path if missing + if path[len(path)-1] != '/' {path += "/"} + for _, file := range files { + tmp_hooks := Hooks{} + if file.IsDir() == true { + msg, e := tmp_hooks.LoadFromDir(path + file.Name()) + if len(msg) != 0 { + warnings = append(warnings, msg...) + } + if e == nil { + *h = append(*h, tmp_hooks...) + } + } else { + e := tmp_hooks.LoadFromFile(path + file.Name()) + if e != nil { + warnings = append(warnings, fmt.Sprintf("%s%s (%+v)", path, file.Name(), e)) + } else { + *h = append(*h, tmp_hooks...) + } + } + } + if len(*h) == 0 {return warnings, loaded} + return warnings, nil +} + // Match iterates through Hooks and returns first one that matches the given ID, // if no hook matches the given ID, nil is returned func (h *Hooks) Match(id string) *Hook { diff --git a/hook/hook_test.go b/hook/hook_test.go index 472787a..2736df1 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -3,6 +3,7 @@ package hook import ( "reflect" "testing" + "strings" ) var checkPayloadSignatureTests = []struct { @@ -217,6 +218,39 @@ func TestHooksLoadFromFile(t *testing.T) { } } +var hooksLoadFromDirTests = []struct { + path string + warn string + ok bool +}{ + // ok - because at least one hook is loaded - exception for "" path + {"", "path '' is unspecified", true}, + {"../test/hooks_dir.test", "../test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example (unexpected end of JSON input),../test/hooks_dir.test/depth1/depth2/depth3b is empty,../test/hooks_dir.test/depth1b/hooks-invalid.json.example (invalid character ':' after array element)", true}, + {"../test/hooks_dir.test/depth1", "../test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example (unexpected end of JSON input),../test/hooks_dir.test/depth1/depth2/depth3b is empty", true}, + {"../test/hooks_dir.test//depth1/depth2/", "../test/hooks_dir.test//depth1/depth2/depth3/depth4/empty_file.example (unexpected end of JSON input),../test/hooks_dir.test//depth1/depth2/depth3b is empty", true}, + // failures - because no hook has been loaded + {"../test/hooks_dir.test///depth1b//", "../test/hooks_dir.test///depth1b//hooks-invalid.json.example (invalid character ':' after array element)", false}, + {"../test/hooks_dir.test//depth1/depth2///depth3", "../test/hooks_dir.test//depth1/depth2///depth3/depth4/empty_file.example (unexpected end of JSON input)", false}, + {"../test/hooks_dir.test/depth1/depth2/depth3b////", "../test/hooks_dir.test/depth1/depth2/depth3b//// is empty", false}, + {"../test/hooks_dir.test/depth1/depth2/depth3/depth4/", "../test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example (unexpected end of JSON input)", false}, + {"../test/hooks_dir.test/non-existing-dir", "path '../test/hooks_dir.test/non-existing-dir' is invalid", false}, +} + +func TestHooksLoadFromDir(t *testing.T) { + for _, tt := range hooksLoadFromDirTests { + h := &Hooks{} + warnings, err := h.LoadFromDir(tt.path) + // to simplify the comparison from the slice we received + warnings_string := strings.Join(warnings, ",") + if (err == nil) != tt.ok { + t.Errorf(err.Error()) + } + if warnings_string != tt.warn { + t.Errorf("recevied: [%s]\nexpected [%s]", warnings_string, tt.warn) + } + } +} + var hooksMatchTests = []struct { id string hooks Hooks diff --git a/test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example b/test/hooks_dir.test/depth1/depth2/depth3/depth4/empty_file.example new file mode 100644 index 0000000..e69de29 diff --git a/test/hooks_dir.test/depth1/depth2/hooks.json.example b/test/hooks_dir.test/depth1/depth2/hooks.json.example new file mode 100644 index 0000000..8324ac4 --- /dev/null +++ b/test/hooks_dir.test/depth1/depth2/hooks.json.example @@ -0,0 +1,79 @@ +[ + { + "id": "webhook-d2", + "execute-command": "/home/adnan/redeploy-go-webhook.sh", + "command-working-directory": "/home/adnan/go", + "response-message": "I got the payload!", + "response-headers": + [ + { + "name": "Access-Control-Allow-Origin", + "value": "*" + } + ], + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "pusher.name" + }, + { + "source": "payload", + "name": "pusher.email" + } + ], + "trigger-rule": + { + "and": + [ + { + "match": + { + "type": "payload-hash-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + }, + { + "match": + { + "type": "value", + "value": "refs/heads/master", + "parameter": + { + "source": "payload", + "name": "ref" + } + } + } + ] + } + }, + { + "id": "simple-webhook-d2", + "execute-command": "/tmp/script1.sh", + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "head_commit.author.name" + }, + { + "source": "payload", + "name": "head_commit.author.email" + } + ] + } +] diff --git a/test/hooks_dir.test/depth1/hooks.json.example b/test/hooks_dir.test/depth1/hooks.json.example new file mode 100644 index 0000000..11a8cc3 --- /dev/null +++ b/test/hooks_dir.test/depth1/hooks.json.example @@ -0,0 +1,79 @@ +[ + { + "id": "webhook", + "execute-command": "/home/adnan/redeploy-go-webhook.sh", + "command-working-directory": "/home/adnan/go", + "response-message": "I got the payload!", + "response-headers": + [ + { + "name": "Access-Control-Allow-Origin", + "value": "*" + } + ], + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "pusher.name" + }, + { + "source": "payload", + "name": "pusher.email" + } + ], + "trigger-rule": + { + "and": + [ + { + "match": + { + "type": "payload-hash-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + }, + { + "match": + { + "type": "value", + "value": "refs/heads/master", + "parameter": + { + "source": "payload", + "name": "ref" + } + } + } + ] + } + }, + { + "id": "simple-webhook", + "execute-command": "/tmp/script1.sh", + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "head_commit.author.name" + }, + { + "source": "payload", + "name": "head_commit.author.email" + } + ] + } +] diff --git a/test/hooks_dir.test/depth1b/hooks-invalid.json.example b/test/hooks_dir.test/depth1b/hooks-invalid.json.example new file mode 100644 index 0000000..047410f --- /dev/null +++ b/test/hooks_dir.test/depth1b/hooks-invalid.json.example @@ -0,0 +1,74 @@ +[ + { + "id": "webhook", + "execute-command": "/home/adnan/redeploy-go-webhook.sh", + "command-working-directory": "/home/adnan/go", + "response-message": "I got the payload!", + "response-headers": + [ + "value": "*" + } + ], + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "pusher.name" + }, + { + "source": "payload", + "name": "pusher.email" + }, + ], + "trigger-rule": + { + "and": + [ + { + "match": + { + "type": "payload-hash-sha1", + "secret": "mysecret", + "parameter": + { +# +# +# + "source": "header", + "name": "X-Hub-Signature" + } + } + }, + { + "match": + { + "type": "value", + "value": "refs/heads/master", + "parameter": + { + "source": "payload", + "name": "ref" + { + "id": "simple-webhook", + "execute-command": "/tmp/script1.sh", + "pass-arguments-to-command": + [ + { + "source": "payload", + "name": "head_commit.id" + }, + { + "source": "payload", + "name": "head_commit.author.name" + }, + { + "source": "payload", + "name": "head_commit.author.email" + } + ] + } +]]]]; diff --git a/webhook.go b/webhook.go index 3c45532..b721a1c 100644 --- a/webhook.go +++ b/webhook.go @@ -31,6 +31,7 @@ var ( noPanic = flag.Bool("nopanic", false, "do not panic if hooks cannot be loaded when webhook is not running in verbose mode") hotReload = flag.Bool("hotreload", false, "watch hooks file for changes and reload them automatically") hooksFilePath = flag.String("hooks", "hooks.json", "path to the json file containing defined hooks the webhook should serve") + hooksDirPath = flag.String("hooksdir", "", "path to the json directory containing defined hooks the webhook should serve") hooksURLPrefix = flag.String("urlprefix", "hooks", "url prefix to use for served hooks (protocol://yourserver:port/PREFIX/:hook-id)") secure = flag.Bool("secure", false, "use HTTPS instead of HTTP") cert = flag.String("cert", "cert.pem", "path to the HTTPS certificate pem file") @@ -64,24 +65,7 @@ func main() { setupSignals() // load and parse hooks - log.Printf("attempting to load hooks from %s\n", *hooksFilePath) - - err := hooks.LoadFromFile(*hooksFilePath) - - if err != nil { - if !*verbose && !*noPanic { - log.SetOutput(os.Stdout) - log.Fatalf("couldn't load any hooks from file! %+v\naborting webhook execution since the -verbose flag is set to false.\nIf, for some reason, you want webhook to start without the hooks, either use -verbose flag, or -nopanic", err) - } - - log.Printf("couldn't load hooks from file! %+v\n", err) - } else { - log.Printf("loaded %d hook(s) from file\n", len(hooks)) - - for _, hook := range hooks { - log.Printf("\t> %s\n", hook.ID) - } - } + hooks = loadHooks() if *hotReload { // set up file watcher @@ -137,7 +121,41 @@ func main() { log.Printf("starting insecure (http) webhook on %s:%d", *ip, *port) log.Fatal(http.ListenAndServe(fmt.Sprintf("%s:%d", *ip, *port), n)) } +} +func loadHooks() hook.Hooks { + log.Printf("attempting to load hooks from file %s\n", *hooksFilePath) + file_hooks := hook.Hooks{} + err_loadfile := file_hooks.LoadFromFile(*hooksFilePath) + + log.Printf("attempting to load hooks from dir %s\n", *hooksDirPath) + dir_hooks := hook.Hooks{} + warnings, err_loaddir := dir_hooks.LoadFromDir(*hooksDirPath) + + if *hooksDirPath != "" && len(warnings) != 0 { + log.Printf("faced issues while loading from %s:\n", *hooksDirPath) + for _, warning := range warnings { + log.Printf("\t> %s\n", warning) + } + } + if err_loadfile != nil && err_loaddir != nil { + if !*verbose && !*noPanic { + log.SetOutput(os.Stdout) + log.Printf("couldn't load any hooks from file and/or dir!\n") + log.Printf("if, for some reason, you want webhook to start without the hooks, either use -verbose flag, or -nopanic") + log.Fatal("aborting webhook execution since the -verbose flag is set to false.\n") + } + } else { + log.Printf("loaded %d hook(s) from file\n", len(file_hooks)) + for _, hook := range file_hooks { + log.Printf("\t> %s\n", hook.ID) + } + log.Printf("loaded %d hook(s) from directory\n", len(dir_hooks)) + for _, hook := range dir_hooks { + log.Printf("\t> %s\n", hook.ID) + } + } + return append(dir_hooks, file_hooks...) } func hookHandler(w http.ResponseWriter, r *http.Request) {