From 39ab72bb304b43837b60f0008ba7082e45178e61 Mon Sep 17 00:00:00 2001 From: Nicolas Carlier Date: Mon, 4 Mar 2024 09:13:59 +0100 Subject: [PATCH] refactor(config): small config refactoring - split config structure - improve config logic - improve test lib and fix typos --- README.md | 20 +++---- docker-compose.yml | 2 +- docker-entrypoint.sh | 8 +-- etc/default/webhookd.env | 72 ++++++++++++-------------- main.go | 23 ++++---- pkg/api/index.go | 30 ++++++----- pkg/api/routes.go | 8 +-- pkg/api/static.go | 4 +- pkg/api/test/helper_test.go | 8 +-- pkg/assert/assert.go | 12 ++--- pkg/config/config.go | 71 +++++++++++++++++-------- pkg/config/deprecated.go | 54 +++++++++++++++++++ pkg/config/flag/bind.go | 39 ++++++++------ pkg/config/flag/test/bind_test.go | 8 ++- pkg/helper/snake.go | 7 +-- pkg/hook/job.go | 4 +- pkg/notification/http/http_notifier.go | 2 +- pkg/server/server.go | 10 ++-- pkg/truststore/truststore.go | 4 +- pkg/worker/dispatcher.go | 1 - pkg/worker/worker.go | 1 - 21 files changed, 241 insertions(+), 147 deletions(-) create mode 100644 pkg/config/deprecated.go diff --git a/README.md b/README.md index 2d8cfdf..faf4094 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ All configuration variables are described in [etc/default/webhookd.env](./etc/de Webhooks are simple scripts within a directory structure. By default inside the `./scripts` directory. -You can change the default directory using the `WHD_SCRIPTS` environment variable or `-script` parameter. +You can change the default directory using the `WHD_HOOK_SCRIPTS` environment variable or `-hook-scripts` parameter. *Example:* @@ -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: @@ -218,7 +218,7 @@ $ # Retrieve logs afterwards $ curl http://localhost:8080/echo/2 ``` -If needed, you can also redirect hook logs to the server output (configured by the `WHD_LOG_HOOK_OUTPUT` environment variable). +If needed, you can also redirect hook logs to the server output (configured by the `WHD_LOG_MODULES=hook` environment variable). ### Post hook notifications @@ -327,12 +327,12 @@ Webhookd supports 2 signature methods: - [HTTP Signatures](https://www.ietf.org/archive/id/draft-cavage-http-signatures-12.txt) - [Ed25519 Signature](https://ed25519.cr.yp.to/) (used by [Discord](https://discord.com/developers/docs/interactions/receiving-and-responding#security-and-authorization)) -To activate request signature verification, you have to configure the trust store: +To activate request signature verification, you have to configure the truststore: ```bash -$ export WHD_TRUST_STORE_FILE=/etc/webhookd/pubkey.pem +$ export WHD_TRUSTSTORE_FILE=/etc/webhookd/pubkey.pem $ # or -$ webhookd --trust-store-file /etc/webhookd/pubkey.pem +$ webhookd --truststore-file /etc/webhookd/pubkey.pem ``` Public key is stored in PEM format. @@ -361,9 +361,9 @@ You can find a small HTTP client in the ["tooling" directory](./tooling/httpsig/ You can activate TLS to secure communications: ```bash -$ export WHD_TLS=true +$ export WHD_TLS_ENABLED=true $ # or -$ webhookd --tls +$ webhookd --tls-enabled ``` By default webhookd is expecting a certificate and key file (`./server.pem` and `./server.key`). @@ -373,10 +373,10 @@ Webhookd also support [ACME](https://ietf-wg-acme.github.io/acme/) protocol. You can activate ACME by setting a fully qualified domain name: ```bash -$ export WHD_TLS=true +$ export WHD_TLS_ENABLED=true $ export WHD_TLS_DOMAIN=hook.example.com $ # or -$ webhookd --tls --tls-domain=hook.example.com +$ webhookd --tls-enabled --tls-domain=hook.example.com ``` **Note:** diff --git a/docker-compose.yml b/docker-compose.yml index 482f766..4ce25ca 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,6 +9,6 @@ services: ports: - "8080:8080" environment: - - WHD_SCRIPTS=/scripts + - WHD_HOOK_SCRIPTS=/scripts volumes: - ./scripts:/scripts diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 61565da..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_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_SCRIPTS + mkdir -p $WHD_HOOK_SCRIPTS - echo "Cloning $WHD_SCRIPTS_GIT_URL into $WHD_SCRIPTS ..." - ssh-agent sh -c 'ssh-add ${WHD_SCRIPTS_GIT_KEY}; git clone --depth 1 --single-branch ${WHD_SCRIPTS_GIT_URL} ${WHD_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_HOOK_SCRIPTS}' [ $? != 0 ] && die "Unable to clone repository" fi diff --git a/etc/default/webhookd.env b/etc/default/webhookd.env index d6c989d..3dde73c 100644 --- a/etc/default/webhookd.env +++ b/etc/default/webhookd.env @@ -2,30 +2,37 @@ # Webhookd configuration ### -# Hook execution logs location, default is OS temporary directory -#WHD_HOOK_LOG_DIR="/tmp" - -# Maximum hook execution time in second, default is 10 -#WHD_HOOK_TIMEOUT=10 - # HTTP listen address, default is ":8080" # Example: `localhost:8080` or `:8080` for all interfaces #WHD_LISTEN_ADDR=":8080" # Log level (debug, info, warn or error), default is "info" #WHD_LOG_LEVEL=info - # Log format (text or json), default is "text" #WHD_LOG_FORMAT=text +# Logging modules to activate (http, hook) +# - `http`: HTTP access logs +# - `hook`: Hook execution logs +# Example: `http` or `http,hook` +#WHD_LOG_MODULES= -# Log HTTP request, default is false -#WHD_LOG_HTTP_REQUEST=false - -# Log hook execution output, default is false -#WHD_LOG_HOOK_OUTPUT=false +# Default extension for hook scripts, default is "sh" +#WHD_HOOK_DEFAULT_EXT=sh +# Maximum hook execution time in second, default is 10 +#WHD_HOOK_TIMEOUT=10 +# Scripts location, default is "scripts" +#WHD_HOOK_SCRIPTS="scripts" +# Hook execution logs location, default is OS temporary directory +#WHD_HOOK_LOG_DIR="/tmp" # Number of workers to start, default is 2 -#WHD_NB_WORKERS=2 +#WHD_HOOK_WORKERS=2 + +# Static file directory to serve on /static path, disabled by default +# Example: `./var/www` +#WHD_STATIC_DIR= +# Path to serve static file directory, default is "/static" +#WHD_STATIC_PATH=/static # Notification URI, disabled by default # Example: `http://requestb.in/v9b229v9` or `mailto:foo@bar.com?smtp=smtp-relay-localnet:25` @@ -34,37 +41,26 @@ # Password file for HTTP basic authentication, default is ".htpasswd" #WHD_PASSWD_FILE=".htpasswd" -# Scripts location, default is "scripts" -#WHD_SCRIPTS="scripts" +# Truststore URI, disabled by default +# Enable HTTP signature verification if set. +# Example: `/etc/webhookd/pubkey.pem` +#WHD_TRUSTSTORE_FILE= + +# Activate TLS, default is false +#WHD_TLS_ENABLED=false +# TLS key file, default is "./server.key" +#WHD_TLS_KEY_FILE="./server.key" +# TLS certificate file, default is "./server.crt" +#WHD_TLS_CERT_FILE="./server.pem" +# TLS domain name used by ACME, key and cert files are ignored if set +# Example: `hook.example.org` +#WHD_TLS_DOMAIN= # GIT repository that contains scripts # Note: this is only used by the Docker image or by using the Docker entrypoint script # Example: `git@github.com:ncarlier/webhookd.git` #WHD_SCRIPTS_GIT_URL= - # GIT SSH private key used to clone the repository # Note: this is only used by the Docker image or by using the Docker entrypoint script # Example: `/etc/webhookd/github_deploy_key.pem` #WHD_SCRIPTS_GIT_KEY= - -# Static file directory to serve on /static path, disabled by default -# Example: `./var/www` -#WHD_STATIC_DIR= - -# Trust store URI, disabled by default -# Enable HTTP signature verification if set. -# Example: `/etc/webhookd/pubkey.pem` -#WHD_TRUST_STORE_FILE= - -# Activate TLS, default is false -#WHD_TLS=false - -# TLS key file, default is "./server.key" -#WHD_TLS_KEY_FILE="./server.key" - -# TLS certificate file, default is "./server.crt" -#WHD_TLS_CERT_FILE="./server.pem" - -# TLS domain name used by ACME, key and cert files are ignored if set -# Example: `hook.example.org` -#WHD_TLS_DOMAIN= diff --git a/main.go b/main.go index 3d7bf94..6df1173 100644 --- a/main.go +++ b/main.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "os/signal" + "slices" "syscall" "time" @@ -22,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() @@ -33,30 +36,32 @@ func main() { os.Exit(0) } - if conf.HookLogDir == "" { - conf.HookLogDir = os.TempDir() + if conf.Hook.LogDir == "" { + conf.Hook.LogDir = os.TempDir() } if err := conf.Validate(); err != nil { log.Fatal("invalid configuration:", err) } - logger.Configure(conf.LogFormat, conf.LogLevel) - logger.HookOutputEnabled = conf.LogHookOutput - logger.RequestOutputEnabled = conf.LogHTTPRequest + logger.Configure(conf.Log.Format, conf.Log.Level) + logger.HookOutputEnabled = slices.Contains(conf.Log.Modules, "hook") + logger.RequestOutputEnabled = slices.Contains(conf.Log.Modules, "http") + + conf.ManageDeprecatedFlags(envPrefix) slog.Debug("starting webhookd server...") srv := server.NewServer(conf) // Configure notification - if err := notification.Init(conf.NotificationURI); err != nil { + if err := notification.Init(conf.Notification.URI); err != nil { slog.Error("unable to create notification channel", "err", err) } // Start the dispatcher. - slog.Debug("starting the dispatcher...", "workers", conf.NbWorkers) - worker.StartDispatcher(conf.NbWorkers) + slog.Debug("starting the dispatcher...", "workers", conf.Hook.Workers) + worker.StartDispatcher(conf.Hook.Workers) done := make(chan bool) quit := make(chan os.Signal, 1) diff --git a/pkg/api/index.go b/pkg/api/index.go index 9b1115b..aa8e54b 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -32,10 +32,10 @@ func atoiFallback(str string, fallback int) int { // index is the main handler of the API. func index(conf *config.Config) http.Handler { - defaultTimeout = conf.HookTimeout - defaultExt = conf.HookDefaultExt - scriptDir = conf.ScriptDir - outputDir = conf.HookLogDir + defaultTimeout = conf.Hook.Timeout + defaultExt = conf.Hook.DefaultExt + scriptDir = conf.Hook.ScriptsDir + outputDir = conf.Hook.LogDir return http.HandlerFunc(webhookHandler) } @@ -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/api/routes.go b/pkg/api/routes.go index 70c7192..8e1156d 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -18,14 +18,14 @@ var commonMiddlewares = middleware.Middlewares{ func buildMiddlewares(conf *config.Config) middleware.Middlewares { var middlewares = commonMiddlewares - if conf.TLS { + if conf.TLS.Enabled { middlewares = middlewares.UseAfter(middleware.HSTS) } // Load trust store... - ts, err := truststore.New(conf.TrustStoreFile) + ts, err := truststore.New(conf.TruststoreFile) if err != nil { - slog.Warn("unable to load trust store", "filename", conf.TrustStoreFile, "err", err) + slog.Warn("unable to load trust store", "filename", conf.TruststoreFile, "err", err) } if ts != nil { middlewares = middlewares.UseAfter(middleware.Signature(ts)) @@ -44,7 +44,7 @@ func buildMiddlewares(conf *config.Config) middleware.Middlewares { func routes(conf *config.Config) Routes { middlewares := buildMiddlewares(conf) - staticPath := conf.StaticPath + "/" + staticPath := conf.Static.Path + "/" return Routes{ route( "/", diff --git a/pkg/api/static.go b/pkg/api/static.go index 2a610dd..f1c754d 100644 --- a/pkg/api/static.go +++ b/pkg/api/static.go @@ -8,8 +8,8 @@ import ( func static(prefix string) HandlerFunc { return func(conf *config.Config) http.Handler { - if conf.StaticDir != "" { - fs := http.FileServer(http.Dir(conf.StaticDir)) + if conf.Static.Dir != "" { + fs := http.FileServer(http.Dir(conf.Static.Dir)) return http.StripPrefix(prefix, fs) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/api/test/helper_test.go b/pkg/api/test/helper_test.go index 55a26f5..4b2339d 100644 --- a/pkg/api/test/helper_test.go +++ b/pkg/api/test/helper_test.go @@ -15,8 +15,8 @@ func TestQueryParamsToShellVars(t *testing.T) { "list": []string{"foo", "bar"}, } values := api.HTTPParamsToShellVars(tc) - assert.ContainsStr(t, "string=foo", values, "") - assert.ContainsStr(t, "list=foo,bar", values, "") + assert.Contains(t, "string=foo", values, "") + assert.Contains(t, "list=foo,bar", values, "") } func TestHTTPHeadersToShellVars(t *testing.T) { @@ -25,6 +25,6 @@ func TestHTTPHeadersToShellVars(t *testing.T) { "X-Foo-Bar": []string{"foo", "bar"}, } values := api.HTTPParamsToShellVars(tc) - assert.ContainsStr(t, "content_type=text/plain", values, "") - assert.ContainsStr(t, "x_foo_bar=foo,bar", values, "") + assert.Contains(t, "content_type=text/plain", values, "") + assert.Contains(t, "x_foo_bar=foo,bar", values, "") } diff --git a/pkg/assert/assert.go b/pkg/assert/assert.go index 1edab0b..a7f667a 100644 --- a/pkg/assert/assert.go +++ b/pkg/assert/assert.go @@ -25,27 +25,27 @@ func NotNil(t *testing.T, actual interface{}, message string) { } // Equal assert that an object is equal to an expected value -func Equal(t *testing.T, expected, actual interface{}, message string) { +func Equal[K comparable](t *testing.T, expected, actual K, message string) { if message == "" { message = "Equal assertion failed" } if actual != expected { - t.Fatalf("%s - expected: %s, actual: %s", message, expected, actual) + t.Fatalf("%s - expected: %v, actual: %v", message, expected, actual) } } // NotEqual assert that an object is not equal to an expected value -func NotEqual(t *testing.T, expected, actual interface{}, message string) { +func NotEqual[K comparable](t *testing.T, expected, actual K, message string) { if message == "" { message = "Not equal assertion failed" } if actual == expected { - t.Fatalf("%s - unexpected: %s, actual: %s", message, expected, actual) + t.Fatalf("%s - unexpected: %v, actual: %v", message, expected, actual) } } // ContainsStr assert that an array contains an expected value -func ContainsStr(t *testing.T, expected string, array []string, message string) { +func Contains[K comparable](t *testing.T, expected K, array []K, message string) { if message == "" { message = "Array don't contains expected value" } @@ -54,7 +54,7 @@ func ContainsStr(t *testing.T, expected string, array []string, message string) return } } - t.Fatalf("%s - array: %v, expected value: %s", message, array, expected) + t.Fatalf("%s - array: %v, expected value: %v", message, array, expected) } // True assert that an expression is true diff --git a/pkg/config/config.go b/pkg/config/config.go index ad3623c..7094197 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,33 +5,58 @@ import ( "regexp" ) -// Config contain global configuration +// Config store root configuration type Config struct { - ListenAddr string `flag:"listen-addr" desc:"HTTP listen address" default:":8080"` - TLS bool `flag:"tls" desc:"Activate TLS" default:"false"` - TLSCertFile string `flag:"tls-cert-file" desc:"TLS certificate file" default:"server.pem"` - TLSKeyFile string `flag:"tls-key-file" desc:"TLS key file" default:"server.key"` - TLSDomain string `flag:"tls-domain" desc:"TLS domain name used by ACME"` - NbWorkers int `flag:"nb-workers" desc:"Number of workers to start" default:"2"` - HookDefaultExt string `flag:"hook-default-ext" desc:"Default extension for hook scripts" default:"sh"` - HookTimeout int `flag:"hook-timeout" desc:"Maximum hook execution time in second" default:"10"` - HookLogDir string `flag:"hook-log-dir" desc:"Hook execution logs location" default:""` - ScriptDir string `flag:"scripts" desc:"Scripts location" default:"scripts"` - PasswdFile string `flag:"passwd-file" desc:"Password file for basic HTTP authentication" default:".htpasswd"` - LogLevel string `flag:"log-level" desc:"Log level (debug, info, warn, error)" default:"info"` - LogFormat string `flag:"log-format" desc:"Log format (json, text)" default:"text"` - LogHookOutput bool `flag:"log-hook-output" desc:"Log hook execution output" default:"false"` - LogHTTPRequest bool `flag:"log-http-request" desc:"Log HTTP request" default:"false"` - StaticDir string `flag:"static-dir" desc:"Static file directory to serve on /static path" default:""` - StaticPath string `flag:"static-path" desc:"Path to serve static file directory" default:"/static"` - NotificationURI string `flag:"notification-uri" desc:"Notification URI"` - TrustStoreFile string `flag:"trust-store-file" desc:"Trust store used by HTTP signature verifier (.pem or .p12)"` + 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"` + TruststoreFile string `flag:"truststore-file" desc:"Truststore used by HTTP signature verifier (.pem or .p12)"` + Hook HookConfig `flag:"hook"` + Log LogConfig `flag:"log"` + Notification NotificationConfig `flag:"notification"` + Static StaticConfig `flag:"static"` + TLS TLSConfig `flag:"tls"` + OldConfig `flag:""` } -// Validate 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"` + ScriptsDir string `flag:"scripts" desc:"Scripts location" default:"scripts"` + LogDir string `flag:"log-dir" desc:"Hook execution logs location" default:""` + Workers int `flag:"workers" desc:"Number of workers to start" default:"2"` +} + +// 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 store notification configuration +type NotificationConfig struct { + URI string `flag:"uri" desc:"Notification URI"` +} + +// 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 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"` + KeyFile string `flag:"key-file" desc:"TLS key file (unused if ACME used)" default:"server.key"` + Domain string `flag:"domain" desc:"TLS domain name used by ACME"` +} + +// Validate the configuration func (c *Config) Validate() error { - if matched, _ := regexp.MatchString(`^/\w+$`, c.StaticPath); !matched { - return fmt.Errorf("invalid static path: %s", c.StaticPath) + if matched, _ := regexp.MatchString(`^/\w+$`, c.Static.Path); !matched { + return fmt.Errorf("invalid static path: %s", c.Static.Path) } return nil } diff --git a/pkg/config/deprecated.go b/pkg/config/deprecated.go new file mode 100644 index 0000000..632f36f --- /dev/null +++ b/pkg/config/deprecated.go @@ -0,0 +1,54 @@ +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"` + Scripts string `flag:"scripts" desc:"Scripts location [DEPRECATED]" default:"scripts"` +} + +// ManageDeprecatedFlags manage legacy configuration +func (c *Config) ManageDeprecatedFlags(prefix string) { + if isUsingDeprecatedConfigParam(prefix, "nb-workers") { + c.Hook.Workers = c.NbWorkers + } + 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) { + if f.Name == name { + found = true + } + }) + return found +} diff --git a/pkg/config/flag/bind.go b/pkg/config/flag/bind.go index 51ccc56..9238dd3 100644 --- a/pkg/config/flag/bind.go +++ b/pkg/config/flag/bind.go @@ -13,7 +13,11 @@ import ( ) // Bind conf struct tags with flags -func Bind(conf interface{}, prefix string) error { +func Bind(conf interface{}, envPrefix string) error { + return bind(conf, envPrefix, "") +} + +func bind(conf interface{}, envPrefix, keyPrefix string) error { rv := reflect.ValueOf(conf) for rv.Kind() == reflect.Ptr || rv.Kind() == reflect.Interface { rv = rv.Elem() @@ -40,15 +44,14 @@ func Bind(conf interface{}, prefix string) error { val = tag } - // Get field value and description from environment variable - if fieldType.Type.Kind() == reflect.Slice { - val = getEnvValue(prefix, key+"s", val) - desc = getEnvDesc(prefix, key+"s", desc) - } else { - val = getEnvValue(prefix, key, val) - desc = getEnvDesc(prefix, key, desc) + if keyPrefix != "" { + key = keyPrefix + "-" + key } + // Get field value and description from environment variable + val = getEnvValue(envPrefix, key, val) + desc = getEnvDesc(envPrefix, key, desc) + // Get field value by reflection from struct definition // And bind value to command line flag switch fieldType.Type.Kind() { @@ -82,18 +85,20 @@ func Bind(conf interface{}, prefix string) error { ptr, _ := field.Addr().Interface().(*int) flag.IntVar(ptr, key, int(i64Val), desc) } + case reflect.Struct: + if err := bind(field.Addr().Interface(), envPrefix, key); err != nil { + return fmt.Errorf("invalid struct value for %s: %v", key, err) + } case reflect.Slice: sliceType := field.Type().Elem() if sliceType.Kind() == reflect.String { - if strings.TrimSpace(val) != "" { - vals := strings.Split(val, ",") - sl := make([]string, len(vals)) - copy(sl, vals) - field.Set(reflect.ValueOf(sl)) - ptr, _ := field.Addr().Interface().(*[]string) - af := newArrayFlags(ptr) - flag.Var(af, key, desc) - } + vals := strings.Split(val, ",") + sl := make([]string, len(vals)) + copy(sl, vals) + field.Set(reflect.ValueOf(sl)) + ptr, _ := field.Addr().Interface().(*[]string) + af := newArrayFlags(ptr) + flag.Var(af, key, desc) } } } diff --git a/pkg/config/flag/test/bind_test.go b/pkg/config/flag/test/bind_test.go index e50e2f0..616d1a9 100644 --- a/pkg/config/flag/test/bind_test.go +++ b/pkg/config/flag/test/bind_test.go @@ -17,12 +17,17 @@ type sampleConfig struct { Timer time.Duration `flag:"timer" desc:"Duration parameter" default:"30s"` Array []string `flag:"array" desc:"Array parameter" default:"foo,bar"` OverrideArray []string `flag:"override-array" desc:"Array parameter to override" default:"foo"` + Obj objConfig `flag:"obj"` +} + +type objConfig struct { + Name string `flag:"name" desc:"Object name" default:"none"` } func TestFlagBinding(t *testing.T) { conf := &sampleConfig{} err := configflag.Bind(conf, "FOO") - flag.CommandLine.Parse([]string{"-override", "test", "-override-array", "a", "-override-array", "b"}) + flag.CommandLine.Parse([]string{"-override", "test", "-override-array", "a", "-override-array", "b", "-obj-name", "foo"}) assert.Nil(t, err, "error should be nil") assert.Equal(t, "foo", conf.Label, "") assert.Equal(t, "test", conf.Override, "") @@ -33,4 +38,5 @@ func TestFlagBinding(t *testing.T) { assert.Equal(t, "foo", conf.Array[0], "") assert.Equal(t, 2, len(conf.OverrideArray), "") assert.Equal(t, "a", conf.OverrideArray[0], "") + assert.Equal(t, "foo", conf.Obj.Name, "") } 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/hook/job.go b/pkg/hook/job.go index 10b39e0..7aa50bc 100644 --- a/pkg/hook/job.go +++ b/pkg/hook/job.go @@ -94,7 +94,7 @@ func (job *Job) Terminate(err error) error { "id", job.ID(), "status", "error", "err", err, - "took", time.Since(job.start).Microseconds(), + "took", time.Since(job.start).Milliseconds(), ) return err } @@ -103,7 +103,7 @@ func (job *Job) Terminate(err error) error { "hook", job.Name(), "id", job.ID(), "status", "success", - "took", time.Since(job.start).Microseconds(), + "took", time.Since(job.start).Milliseconds(), ) return nil } diff --git a/pkg/notification/http/http_notifier.go b/pkg/notification/http/http_notifier.go index a46bdab..303114c 100644 --- a/pkg/notification/http/http_notifier.go +++ b/pkg/notification/http/http_notifier.go @@ -27,7 +27,7 @@ type httpNotifier struct { } func newHTTPNotifier(uri *url.URL) (notification.Notifier, error) { - slog.Info("using HTTP notification system ", "üri", uri.Opaque) + slog.Info("using HTTP notification system ", "uri", uri.Opaque) return &httpNotifier{ URL: uri, PrefixFilter: helper.GetValueOrAlt(uri.Query(), "prefix", "notify:"), diff --git a/pkg/server/server.go b/pkg/server/server.go index 73a4835..407584a 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -50,7 +50,7 @@ func (s *Server) Shutdown(ctx context.Context) error { func NewServer(cfg *config.Config) *Server { logger := slog.NewLogLogger(slog.Default().Handler(), slog.LevelError) server := &Server{ - tls: cfg.TLS, + tls: cfg.TLS.Enabled, self: &http.Server{ Addr: cfg.ListenAddr, Handler: api.NewRouter(cfg), @@ -59,14 +59,14 @@ func NewServer(cfg *config.Config) *Server { } if server.tls { // HTTPs server - if cfg.TLSDomain == "" { - server.certFile = cfg.TLSCertFile - server.keyFile = cfg.TLSKeyFile + if cfg.TLS.Domain == "" { + server.certFile = cfg.TLS.CertFile + server.keyFile = cfg.TLS.KeyFile } else { m := &autocert.Manager{ Cache: autocert.DirCache(cacheDir()), Prompt: autocert.AcceptTOS, - HostPolicy: autocert.HostWhitelist(cfg.TLSDomain), + HostPolicy: autocert.HostWhitelist(cfg.TLS.Domain), } server.self.TLSConfig = m.TLSConfig() server.certFile = "" 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" )