From e1036eaaf083d3a9c57f5c2eff667d2b16b87fe2 Mon Sep 17 00:00:00 2001 From: Ivan Pesin Date: Sun, 27 Aug 2017 20:03:58 -0400 Subject: [PATCH 1/4] max-concurrency hook property support --- hook/hook.go | 1 + webhook.go | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/hook/hook.go b/hook/hook.go index 6fcda3e..a271630 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -381,6 +381,7 @@ type Hook struct { PassEnvironmentToCommand []Argument `json:"pass-environment-to-command,omitempty"` PassArgumentsToCommand []Argument `json:"pass-arguments-to-command,omitempty"` JSONStringParameters []Argument `json:"parse-parameters-as-json,omitempty"` + MaxConcurrency int `json:"max-concurrency,omiempty"` TriggerRule *Rules `json:"trigger-rule,omitempty"` TriggerRuleMismatchHttpResponseCode int `json:"trigger-rule-mismatch-http-response-code,omitempty"` } diff --git a/webhook.go b/webhook.go index 1034290..05989da 100644 --- a/webhook.go +++ b/webhook.go @@ -43,6 +43,8 @@ var ( watcher *fsnotify.Watcher signals chan os.Signal + + limits = make(map[string]chan struct{}) ) func matchLoadedHook(id string) *hook.Hook { @@ -108,7 +110,15 @@ func main() { if matchLoadedHook(hook.ID) != nil { log.Fatalf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!\n", hook.ID) } - log.Printf("\tloaded: %s\n", hook.ID) + + msg := fmt.Sprintf("\tloaded: %s", hook.ID) + + // initialize concurrency map + if hook.MaxConcurrency > 0 { + limits[hook.ID] = make(chan struct{}, hook.MaxConcurrency) + msg = fmt.Sprintf("%s (max: %d)", msg, hook.MaxConcurrency) + } + log.Println(msg) } loadedHooksFromFiles[hooksFilePath] = newHooks @@ -208,6 +218,18 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if matchedHook := matchLoadedHook(id); matchedHook != nil { log.Printf("%s got matched\n", id) + // check if we have concurrency limits + if _, ok := limits[id]; ok { + if len(limits[id]) == cap(limits[id]) { + log.Printf("reached concurrency limit for: %s (max=%d)", id, len(limits[id])) + w.WriteHeader(http.StatusTooManyRequests) + fmt.Fprintf(w, "Error occurred while evaluating hook rules.") + return + } + defer func() { <-limits[id] }() + limits[id] <- struct{}{} + } + body, err := ioutil.ReadAll(r.Body) if err != nil { log.Printf("error reading the request body. %+v\n", err) From baad906d24743ea8f550bd2434c491278f613d92 Mon Sep 17 00:00:00 2001 From: Ivan Pesin Date: Sun, 10 Sep 2017 21:36:17 -0500 Subject: [PATCH 2/4] Corrected parameter name, typo, error message, and variable naming --- hook/hook.go | 2 +- webhook.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index a271630..8689f97 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -381,7 +381,7 @@ type Hook struct { PassEnvironmentToCommand []Argument `json:"pass-environment-to-command,omitempty"` PassArgumentsToCommand []Argument `json:"pass-arguments-to-command,omitempty"` JSONStringParameters []Argument `json:"parse-parameters-as-json,omitempty"` - MaxConcurrency int `json:"max-concurrency,omiempty"` + MaxConcurrency int `json:"maximum-concurrent-executions,omitempty"` TriggerRule *Rules `json:"trigger-rule,omitempty"` TriggerRuleMismatchHttpResponseCode int `json:"trigger-rule-mismatch-http-response-code,omitempty"` } diff --git a/webhook.go b/webhook.go index 05989da..8982aa1 100644 --- a/webhook.go +++ b/webhook.go @@ -44,7 +44,7 @@ var ( watcher *fsnotify.Watcher signals chan os.Signal - limits = make(map[string]chan struct{}) + hookExecutions = make(map[string]chan struct{}) ) func matchLoadedHook(id string) *hook.Hook { @@ -115,7 +115,7 @@ func main() { // initialize concurrency map if hook.MaxConcurrency > 0 { - limits[hook.ID] = make(chan struct{}, hook.MaxConcurrency) + hookExecutions[hook.ID] = make(chan struct{}, hook.MaxConcurrency) msg = fmt.Sprintf("%s (max: %d)", msg, hook.MaxConcurrency) } log.Println(msg) @@ -219,15 +219,15 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { log.Printf("%s got matched\n", id) // check if we have concurrency limits - if _, ok := limits[id]; ok { - if len(limits[id]) == cap(limits[id]) { - log.Printf("reached concurrency limit for: %s (max=%d)", id, len(limits[id])) + if _, ok := hookExecutions[id]; ok { + if len(hookExecutions[id]) == cap(hookExecutions[id]) { + log.Printf("reached concurrency limit for: %s (max=%d)", id, len(hookExecutions[id])) w.WriteHeader(http.StatusTooManyRequests) - fmt.Fprintf(w, "Error occurred while evaluating hook rules.") + fmt.Fprintf(w, "Hook reached maximum concurrent execution limit. Try again later.") return } - defer func() { <-limits[id] }() - limits[id] <- struct{}{} + defer func() { <-hookExecutions[id] }() + hookExecutions[id] <- struct{}{} } body, err := ioutil.ReadAll(r.Body) From 0389a66cb9dea9b5219643c33b333aa29f4616fd Mon Sep 17 00:00:00 2001 From: Ivan Pesin Date: Sun, 10 Sep 2017 22:23:02 -0500 Subject: [PATCH 3/4] Simplify if block -- reduce nesting --- webhook.go | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/webhook.go b/webhook.go index 8982aa1..8cb5f0b 100644 --- a/webhook.go +++ b/webhook.go @@ -377,36 +377,36 @@ func reloadHooks(hooksFilePath string) { log.Printf("attempting to reload hooks from %s\n", hooksFilePath) err := hooksInFile.LoadFromFile(hooksFilePath) - if err != nil { log.Printf("couldn't load hooks from file! %+v\n", err) - } else { - seenHooksIds := make(map[string]bool) + return + } - log.Printf("found %d hook(s) in file\n", len(hooksInFile)) + seenHooksIds := make(map[string]bool) - for _, hook := range hooksInFile { - wasHookIDAlreadyLoaded := false + log.Printf("found %d hook(s) in file\n", len(hooksInFile)) - for _, loadedHook := range loadedHooksFromFiles[hooksFilePath] { - if loadedHook.ID == hook.ID { - wasHookIDAlreadyLoaded = true - break - } + for _, hook := range hooksInFile { + wasHookIDAlreadyLoaded := false + + for _, loadedHook := range loadedHooksFromFiles[hooksFilePath] { + if loadedHook.ID == hook.ID { + wasHookIDAlreadyLoaded = true + break } - - if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] == true { - log.Printf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!", hook.ID) - log.Println("reverting hooks back to the previous configuration") - return - } - - seenHooksIds[hook.ID] = true - log.Printf("\tloaded: %s\n", hook.ID) } - loadedHooksFromFiles[hooksFilePath] = hooksInFile + if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] == true { + log.Printf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!", hook.ID) + log.Println("reverting hooks back to the previous configuration") + return + } + + seenHooksIds[hook.ID] = true + log.Printf("\tloaded: %s\n", hook.ID) } + + loadedHooksFromFiles[hooksFilePath] = hooksInFile } func reloadAllHooks() { From c2ec2adc503cbd86d82233a763550aeb0c376316 Mon Sep 17 00:00:00 2001 From: Ivan Pesin Date: Sun, 10 Sep 2017 22:49:48 -0500 Subject: [PATCH 4/4] Update hookExecutions limits on configuration reload --- webhook.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index 8cb5f0b..18fafdc 100644 --- a/webhook.go +++ b/webhook.go @@ -403,7 +403,27 @@ func reloadHooks(hooksFilePath string) { } seenHooksIds[hook.ID] = true - log.Printf("\tloaded: %s\n", hook.ID) + msg := fmt.Sprintf("\tloaded: %s", hook.ID) + + // initialize or update concurrency map + switch { + case hook.MaxConcurrency == 0: + if _, ok := hookExecutions[hook.ID]; ok { + delete(hookExecutions, hook.ID) + } + msg = fmt.Sprintf("%s", msg) + case hook.MaxConcurrency > 0: + hookExecutions[hook.ID] = make(chan struct{}, hook.MaxConcurrency) + msg = fmt.Sprintf("%s (max: %d)", msg, hook.MaxConcurrency) + } + log.Println(msg) + } + + // clean up hookExecutions channels for removed hooks + for _, loadedHook := range loadedHooksFromFiles[hooksFilePath] { + if _, ok := seenHooksIds[loadedHook.ID]; !ok { + delete(hookExecutions, loadedHook.ID) + } } loadedHooksFromFiles[hooksFilePath] = hooksInFile @@ -418,6 +438,9 @@ func reloadAllHooks() { func removeHooks(hooksFilePath string) { for _, hook := range loadedHooksFromFiles[hooksFilePath] { log.Printf("\tremoving: %s\n", hook.ID) + if _, ok := hookExecutions[hook.ID]; ok { + delete(hookExecutions, hook.ID) + } } newHooksFiles := hooksFiles[:0]