From 073ac38b476dd70d802cd51d2f46dd2b0850178a Mon Sep 17 00:00:00 2001 From: Nicolas Carlier Date: Sun, 31 Dec 2023 14:49:37 +0100 Subject: [PATCH] refactor(config): small config refactoring - split config structure - improve config logic --- README.md | 2 +- docker-entrypoint.sh | 4 ++-- etc/default/webhookd.env | 2 +- main.go | 6 ++++-- pkg/api/index.go | 22 +++++++++++--------- pkg/config/config.go | 13 ++++++------ pkg/config/deprecated.go | 39 ++++++++++++++++++++++++++---------- pkg/helper/snake.go | 7 ++++--- pkg/truststore/truststore.go | 4 ++-- pkg/worker/dispatcher.go | 1 - pkg/worker/worker.go | 1 - 11 files changed, 62 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 44bd44f..faf4094 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ The directory structure define the webhook URL. You can omit the script extension. If you do, webhookd will search by default for a `.sh` file. You can change the default extension using the `WHD_HOOK_DEFAULT_EXT` environment variable or `-hook-default-ext` parameter. -If the script exists, the output the will be streamed to the HTTP response. +If the script exists, the output will be streamed to the HTTP response. The streaming technology depends on the HTTP request: diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index a36d26d..17ee8f5 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -7,13 +7,13 @@ if [ ! -z "$WHD_SCRIPTS_GIT_URL" ] then [ ! -f "$WHD_SCRIPTS_GIT_KEY" ] && die "Git clone key not found." - export WHD_HOOK_SCRIPTS=${WHD_SCRIPTS:-/opt/scripts-git} + export WHD_HOOK_SCRIPTS=${WHD_HOOK_SCRIPTS:-/opt/scripts-git} export GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" mkdir -p $WHD_HOOK_SCRIPTS echo "Cloning $WHD_SCRIPTS_GIT_URL into $WHD_HOOK_SCRIPTS ..." - ssh-agent sh -c 'ssh-add ${WHD_SCRIPTS_GIT_KEY}; git clone --depth 1 --single-branch ${WHD_SCRIPTS_GIT_URL} ${WHD_SCRIPTS}' + ssh-agent sh -c 'ssh-add ${WHD_SCRIPTS_GIT_KEY}; git clone --depth 1 --single-branch ${WHD_SCRIPTS_GIT_URL} ${WHD_HOOK_SCRIPTS}' [ $? != 0 ] && die "Unable to clone repository" fi diff --git a/etc/default/webhookd.env b/etc/default/webhookd.env index 0427be0..3dde73c 100644 --- a/etc/default/webhookd.env +++ b/etc/default/webhookd.env @@ -11,7 +11,7 @@ # Log format (text or json), default is "text" #WHD_LOG_FORMAT=text # Logging modules to activate (http, hook) -# - `http`: HTPP access logs +# - `http`: HTTP access logs # - `hook`: Hook execution logs # Example: `http` or `http,hook` #WHD_LOG_MODULES= diff --git a/main.go b/main.go index 485951a..6df1173 100644 --- a/main.go +++ b/main.go @@ -23,9 +23,11 @@ import ( "github.com/ncarlier/webhookd/pkg/worker" ) +const envPrefix = "WHD" + func main() { conf := &config.Config{} - configflag.Bind(conf, "WHD") + configflag.Bind(conf, envPrefix) flag.Parse() @@ -46,7 +48,7 @@ func main() { logger.HookOutputEnabled = slices.Contains(conf.Log.Modules, "hook") logger.RequestOutputEnabled = slices.Contains(conf.Log.Modules, "http") - conf.ManageDeprecatedFlags() + conf.ManageDeprecatedFlags(envPrefix) slog.Debug("starting webhookd server...") diff --git a/pkg/api/index.go b/pkg/api/index.go index c793336..aa8e54b 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -65,14 +65,16 @@ func triggerWebhook(w http.ResponseWriter, r *http.Request) { } script, err := hook.ResolveScript(scriptDir, hookName, defaultExt) if err != nil { - slog.Error("hooke not found", "err", err.Error()) - http.Error(w, "hook not found", http.StatusNotFound) + msg := "hook not found" + slog.Error(msg, "err", err.Error()) + http.Error(w, msg, http.StatusNotFound) return } if err = r.ParseForm(); err != nil { - slog.Error("error reading from-data", "err", err) - http.Error(w, "unable to parse request form", http.StatusBadRequest) + msg := "unable to parse form-data" + slog.Error(msg, "err", err) + http.Error(w, msg, http.StatusBadRequest) return } @@ -84,8 +86,9 @@ func triggerWebhook(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(mediatype, "text/") || mediatype == "application/json" { body, err = io.ReadAll(r.Body) if err != nil { - slog.Error("error reading body", "err", err) - http.Error(w, "unable to read request body", http.StatusBadRequest) + msg := "unable to read request body" + slog.Error(msg, "err", err) + http.Error(w, msg, http.StatusBadRequest) return } } @@ -106,8 +109,9 @@ func triggerWebhook(w http.ResponseWriter, r *http.Request) { OutputDir: outputDir, }) if err != nil { - slog.Error("error creating hook job", "err", err) - http.Error(w, "unable to create hook job", http.StatusInternalServerError) + msg := "unable to create hook execution job" + slog.Error(msg, "err", err) + http.Error(w, msg, http.StatusInternalServerError) return } @@ -163,7 +167,7 @@ func getWebhookLog(w http.ResponseWriter, r *http.Request) { return } if logFile == nil { - http.Error(w, "job not found", http.StatusNotFound) + http.Error(w, "hook execution log not found", http.StatusNotFound) return } defer logFile.Close() diff --git a/pkg/config/config.go b/pkg/config/config.go index f6a360d..7094197 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,6 +5,7 @@ import ( "regexp" ) +// Config store root configuration type Config struct { ListenAddr string `flag:"listen-addr" desc:"HTTP listen address" default:":8080"` PasswdFile string `flag:"passwd-file" desc:"Password file for basic HTTP authentication" default:".htpasswd"` @@ -17,7 +18,7 @@ type Config struct { OldConfig `flag:""` } -// HookConfig manage Hook execution configuration +// HookConfig store Hook execution configuration type HookConfig struct { DefaultExt string `flag:"default-ext" desc:"Default extension for hook scripts" default:"sh"` Timeout int `flag:"timeout" desc:"Maximum hook execution time in second" default:"10"` @@ -26,25 +27,25 @@ type HookConfig struct { Workers int `flag:"workers" desc:"Number of workers to start" default:"2"` } -// LogConfig manage the logger configuration +// LogConfig store logger configuration type LogConfig struct { Level string `flag:"level" desc:"Log level (debug, info, warn or error)" default:"info"` Format string `flag:"format" desc:"Log format (json or text)" default:"text"` Modules []string `flag:"modules" desc:"Logging modules to activate (http,hook)" default:""` } -// NotificationConfig manage notification configuration +// NotificationConfig store notification configuration type NotificationConfig struct { URI string `flag:"uri" desc:"Notification URI"` } -// StaticConfig manage static assets configuration +// StaticConfig store static assets configuration type StaticConfig struct { Dir string `flag:"dir" desc:"Static file directory to serve on /static path" default:""` Path string `flag:"path" desc:"Path to serve static file directory" default:"/static"` } -// TLSConfig manage TLS configuration +// TLSConfig store TLS configuration type TLSConfig struct { Enabled bool `flag:"enabled" desc:"Enable TLS" default:"false"` CertFile string `flag:"cert-file" desc:"TLS certificate file (unused if ACME used)" default:"server.pem"` @@ -52,7 +53,7 @@ type TLSConfig struct { Domain string `flag:"domain" desc:"TLS domain name used by ACME"` } -// Validate configuration +// Validate the configuration func (c *Config) Validate() error { if matched, _ := regexp.MatchString(`^/\w+$`, c.Static.Path); !matched { return fmt.Errorf("invalid static path: %s", c.Static.Path) diff --git a/pkg/config/deprecated.go b/pkg/config/deprecated.go index 02a48b1..632f36f 100644 --- a/pkg/config/deprecated.go +++ b/pkg/config/deprecated.go @@ -3,29 +3,46 @@ package config import ( "flag" "log/slog" + "os" + + "github.com/ncarlier/webhookd/pkg/helper" ) // OldConfig contain global configuration type OldConfig struct { NbWorkers int `flag:"nb-workers" desc:"Number of workers to start [DEPRECATED]" default:"2"` - ScriptDir string `flag:"scripts" desc:"Scripts location [DEPRECATED]" default:"scripts"` + Scripts string `flag:"scripts" desc:"Scripts location [DEPRECATED]" default:"scripts"` } -// ManageDeprecatedFlags manage configuration legacy -func (c *Config) ManageDeprecatedFlags() { - // TODO check env variable - // TODO other legacy parameters? - // TODO code factorization - if isFlagPassed("nb-workers") { - slog.Warn("using deprecated configuration flag", "flag", "nb-workers") +// ManageDeprecatedFlags manage legacy configuration +func (c *Config) ManageDeprecatedFlags(prefix string) { + if isUsingDeprecatedConfigParam(prefix, "nb-workers") { c.Hook.Workers = c.NbWorkers } - if isFlagPassed("scripts") { - slog.Warn("using deprecated configuration flag", "flag", "scripts") - c.Hook.ScriptsDir = c.ScriptDir + if isUsingDeprecatedConfigParam(prefix, "scripts") { + c.Hook.ScriptsDir = c.Scripts } } +func isUsingDeprecatedConfigParam(prefix, flagName string) bool { + envVar := helper.ToScreamingSnake(prefix + "_" + flagName) + switch { + case isFlagPassed(flagName): + slog.Warn("using deprecated configuration flag", "flag", flagName) + return true + case isEnvExists(envVar): + slog.Warn("using deprecated configuration environment variable", "variable", envVar) + return true + default: + return false + } +} + +func isEnvExists(name string) bool { + _, exists := os.LookupEnv(name) + return exists +} + func isFlagPassed(name string) bool { found := false flag.Visit(func(f *flag.Flag) { diff --git a/pkg/helper/snake.go b/pkg/helper/snake.go index 3b6a42c..7c9c788 100644 --- a/pkg/helper/snake.go +++ b/pkg/helper/snake.go @@ -68,17 +68,18 @@ func ToScreamingDelimited(s string, del uint8, screaming bool) string { } } - if i > 0 && n[len(n)-1] != del && nextCaseIsChanged { + switch { + case i > 0 && n[len(n)-1] != del && nextCaseIsChanged: // add underscore if next letter case type is changed if v >= 'A' && v <= 'Z' { n += string(del) + string(v) } else if v >= 'a' && v <= 'z' { n += string(v) + string(del) } - } else if v == ' ' || v == '_' || v == '-' || v == '/' { + case v == ' ' || v == '_' || v == '-' || v == '/': // replace spaces/underscores with delimiters n += string(del) - } else { + default: n += string(v) } } diff --git a/pkg/truststore/truststore.go b/pkg/truststore/truststore.go index a590a77..829e5e3 100644 --- a/pkg/truststore/truststore.go +++ b/pkg/truststore/truststore.go @@ -31,14 +31,14 @@ func New(filename string) (store TrustStore, err error) { return nil, nil } - slog.Debug("loading trust store...", "filname", filename) + slog.Debug("loading truststore...", "filname", filename) switch filepath.Ext(filename) { case ".pem": store, err = newPEMTrustStore(filename) case ".p12": store, err = newP12TrustStore(filename) default: - err = fmt.Errorf("unsupported trust store file format: %s", filename) + err = fmt.Errorf("unsupported truststore file format: %s", filename) } return diff --git a/pkg/worker/dispatcher.go b/pkg/worker/dispatcher.go index 85f3aec..bc14710 100644 --- a/pkg/worker/dispatcher.go +++ b/pkg/worker/dispatcher.go @@ -30,7 +30,6 @@ func StartDispatcher(nworkers int) { slog.Debug("dispatching hook request", "hook", work.Name(), "id", work.ID()) worker <- work }() - } }() } diff --git a/pkg/worker/worker.go b/pkg/worker/worker.go index e902d2a..e26ed84 100644 --- a/pkg/worker/worker.go +++ b/pkg/worker/worker.go @@ -5,7 +5,6 @@ import ( "log/slog" "github.com/ncarlier/webhookd/pkg/metric" - "github.com/ncarlier/webhookd/pkg/notification" )