From 1908f382171abc30c88600bb4beefb2ac3b06946 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 12 Aug 2022 20:11:44 +0900 Subject: [PATCH] all: permit to read config from yaml not from env --- cmd/dagu.go | 3 +- cmd/server_test.go | 8 +- internal/admin/config.go | 212 +++++++++++++++-------------- internal/admin/config_test.go | 8 +- internal/admin/definition.go | 2 + internal/admin/handlers/dag.go | 4 +- internal/admin/handlers/html.go | 48 ++++--- internal/admin/handlers/list.go | 4 +- internal/admin/loader.go | 12 +- internal/admin/loader_test.go | 3 +- internal/admin/routes.go | 24 ++-- internal/settings/settings.go | 44 +++--- internal/settings/settings_test.go | 9 +- 13 files changed, 202 insertions(+), 179 deletions(-) diff --git a/cmd/dagu.go b/cmd/dagu.go index 0e6a2a9ee..0211c4c5b 100644 --- a/cmd/dagu.go +++ b/cmd/dagu.go @@ -43,8 +43,7 @@ func loadGlobalConfig(c *cli.Context) (cfg *admin.Config, err error) { cf := utils.StringWithFallback(c.String("config"), settings.MustGet(settings.SETTING__ADMIN_CONFIG)) cfg, err = l.LoadAdminConfig(cf) if err == admin.ErrConfigNotFound { - cfg = admin.DefaultConfig() - err = nil + cfg, err = admin.DefaultConfig() } if err != nil { return nil, fmt.Errorf("loading admin config failed: %w", err) diff --git a/cmd/server_test.go b/cmd/server_test.go index 9976339aa..2c2202f5b 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -18,10 +18,10 @@ func Test_serverCommand(t *testing.T) { app := makeApp() dir := utils.MustTempDir("dagu_test_server") os.Setenv("HOME", dir) + settings.ChangeHomeDir(dir) port := findPort(t) - os.Setenv(settings.SETTING__ADMIN_PORT, port) - settings.ChangeHomeDir(dir) + settings.Set(settings.SETTING__ADMIN_PORT, port) done := make(chan struct{}) go func() { @@ -34,7 +34,9 @@ func Test_serverCommand(t *testing.T) { time.Sleep(time.Millisecond * 100) - cfg := admin.DefaultConfig() + cfg, err := admin.DefaultConfig() + require.NoError(t, err) + res, err := http.Post( fmt.Sprintf("http://%s:%s/shutdown", cfg.Host, cfg.Port), "application/json", diff --git a/internal/admin/config.go b/internal/admin/config.go index f34a0dcf3..ffbdb04d5 100644 --- a/internal/admin/config.go +++ b/internal/admin/config.go @@ -24,46 +24,135 @@ type Config struct { LogEncodingCharset string LogDir string BaseConfig string + NavbarColor string + NavbarTitle string } -func (cfg *Config) Init() { - if cfg.Env == nil { - cfg.Env = []string{} +func newConfig() *Config { + return &Config{ + Env: []string{}, } } -func (cfg *Config) setup() { - cfg.Command = utils.StringWithFallback(cfg.Command, "dagu") - cfg.DAGs = utils.StringWithFallback(cfg.DAGs, - settings.MustGet(settings.SETTING__ADMIN_DAGS_DIR)) - cfg.LogDir = utils.StringWithFallback(cfg.LogDir, - settings.MustGet(settings.SETTING__ADMIN_LOGS_DIR)) - cfg.Host = utils.StringWithFallback(cfg.Host, "127.0.0.1") - cfg.Port = utils.StringWithFallback(cfg.Port, - settings.MustGet(settings.SETTING__ADMIN_PORT)) +func DefaultConfig() (*Config, error) { + cfg := newConfig() + err := cfg.setup() + return cfg, err +} + +func defaultConfig() *Config { + return &Config{ + Command: "dagu", + DAGs: settings.MustGet(settings.SETTING__ADMIN_DAGS_DIR), + LogDir: settings.MustGet(settings.SETTING__ADMIN_LOGS_DIR), + Host: "127.0.0.1", + Port: settings.MustGet(settings.SETTING__ADMIN_PORT), + NavbarColor: settings.MustGet(settings.SETTING__ADMIN_NAVBAR_COLOR), + NavbarTitle: settings.MustGet(settings.SETTING__ADMIN_NAVBAR_TITLE), + } +} + +func (cfg *Config) setup() error { + // TODO: refactor to avoid loading unnecessary configs + def := defaultConfig() + setDef := func(val *string, def string) { + *val = utils.StringWithFallback(*val, def) + } + setDef(&cfg.Command, def.Command) + setDef(&cfg.DAGs, def.DAGs) + setDef(&cfg.LogDir, def.LogDir) + setDef(&cfg.Host, def.Host) + setDef(&cfg.Port, def.Port) + setDef(&cfg.NavbarColor, def.NavbarColor) + setDef(&cfg.NavbarTitle, def.NavbarTitle) + if len(cfg.Env) == 0 { env := utils.DefaultEnv() env, err := loadVariables(env) if err != nil { - panic(err) + return err } cfg.Env = buildConfigEnv(env) } + return nil } func buildFromDefinition(def *configDefinition) (cfg *Config, err error) { - cfg = &Config{} - cfg.Init() + cfg = newConfig() for _, fn := range []func(cfg *Config, def *configDefinition) error{ - buildEnvs, - buildHostPort, - buildDAGsDir, - buildCommand, - buildWorkDir, - buildBasicAuthOpts, - buidEncodingOpts, - buildBaseConfig, + func(cfg *Config, def *configDefinition) error { + env, err := loadVariables(def.Env) + if err != nil { + return err + } + cfg.Env = buildConfigEnv(env) + return nil + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.Host, err = utils.ParseVariable(def.Host) + if def.Port != 0 { + cfg.Port = strconv.Itoa(def.Port) + } + return err + }, + func(cfg *Config, def *configDefinition) (err error) { + val, err := utils.ParseVariable(def.Dags) + if err == nil && len(val) > 0 { + if !filepath.IsAbs(val) { + return fmt.Errorf("DAGs directory should be absolute path. was %s", val) + } + cfg.DAGs, err = filepath.Abs(val) + if err != nil { + return fmt.Errorf("failed to resolve DAGs directory: %w", err) + } + } + return err + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.Command, err = utils.ParseVariable(def.Command) + return err + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.WorkDir, err = utils.ParseVariable(def.WorkDir) + if err == nil && strings.TrimSpace(cfg.WorkDir) == "" { + cfg.WorkDir, err = os.Getwd() + if err != nil { + return fmt.Errorf("failed to resolve working directory: %w", err) + } + } + return err + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.BasicAuthUsername, err = utils.ParseVariable(def.BasicAuthUsername) + if err != nil { + return err + } + cfg.BasicAuthPassword, err = utils.ParseVariable(def.BasicAuthPassword) + if err != nil { + return err + } + return nil + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.LogEncodingCharset, err = utils.ParseVariable(def.LogEncodingCharset) + return err + }, + func(cfg *Config, def *configDefinition) (err error) { + cfg.BaseConfig, err = utils.ParseVariable(strings.TrimSpace(def.BaseConfig)) + if err != nil { + return err + } + if cfg.BaseConfig == "" { + cfg.BaseConfig = settings.MustGet(settings.SETTING__BASE_CONFIG) + } + return nil + }, + func(cfg *Config, def *configDefinition) error { + cfg.NavbarColor = def.NavbarColor + cfg.NavbarTitle = def.NavbarTitle + return nil + }, } { if err := fn(cfg, def); err != nil { return nil, err @@ -76,83 +165,6 @@ func buildFromDefinition(def *configDefinition) (cfg *Config, err error) { return cfg, nil } -func buildEnvs(cfg *Config, def *configDefinition) error { - env, err := loadVariables(def.Env) - if err != nil { - return err - } - cfg.Env = buildConfigEnv(env) - return nil -} - -func buildHostPort(cfg *Config, def *configDefinition) (err error) { - cfg.Host, err = utils.ParseVariable(def.Host) - if def.Port == 0 { - cfg.Port = settings.MustGet(settings.SETTING__ADMIN_PORT) - } else { - cfg.Port = strconv.Itoa(def.Port) - } - return err -} - -func buildDAGsDir(cfg *Config, def *configDefinition) (err error) { - val, err := utils.ParseVariable(def.Dags) - if err == nil && len(val) > 0 { - if !filepath.IsAbs(val) { - return fmt.Errorf("DAGs directory should be absolute path. was %s", val) - } - cfg.DAGs, err = filepath.Abs(val) - if err != nil { - return fmt.Errorf("failed to resolve DAGs directory: %w", err) - } - } - return err -} - -func buildBaseConfig(cfg *Config, def *configDefinition) (err error) { - cfg.BaseConfig, err = utils.ParseVariable(strings.TrimSpace(def.BaseConfig)) - if err != nil { - return err - } - if cfg.BaseConfig == "" { - cfg.BaseConfig = settings.MustGet(settings.SETTING__BASE_CONFIG) - } - return nil -} - -func buildCommand(cfg *Config, def *configDefinition) (err error) { - cfg.Command, err = utils.ParseVariable(def.Command) - return err -} - -func buildWorkDir(cfg *Config, def *configDefinition) (err error) { - cfg.WorkDir, err = utils.ParseVariable(def.WorkDir) - if err == nil && strings.TrimSpace(cfg.WorkDir) == "" { - cfg.WorkDir, err = os.Getwd() - if err != nil { - return fmt.Errorf("failed to resolve working directory: %w", err) - } - } - return err -} - -func buildBasicAuthOpts(cfg *Config, def *configDefinition) (err error) { - cfg.BasicAuthUsername, err = utils.ParseVariable(def.BasicAuthUsername) - if err != nil { - return err - } - cfg.BasicAuthPassword, err = utils.ParseVariable(def.BasicAuthPassword) - if err != nil { - return err - } - return nil -} - -func buidEncodingOpts(cfg *Config, def *configDefinition) (err error) { - cfg.LogEncodingCharset, err = utils.ParseVariable(def.LogEncodingCharset) - return err -} - func buildConfigEnv(vars map[string]string) []string { ret := []string{} for k, v := range vars { diff --git a/internal/admin/config_test.go b/internal/admin/config_test.go index 67a3f9a73..4d607acfd 100644 --- a/internal/admin/config_test.go +++ b/internal/admin/config_test.go @@ -20,6 +20,8 @@ basicAuthPassword: password logEncodingCharset: utf-8 logDir: /var/log/dagu baseConfig: /dagu/config.yaml +navbarColor: red +navbarTitle: Dagu test ` func TestLoadConfig(t *testing.T) { @@ -42,6 +44,8 @@ func TestLoadConfig(t *testing.T) { Env: []string{}, LogDir: "/var/log/dagu", BaseConfig: "/dagu/config.yaml", + NavbarColor: "red", + NavbarTitle: "Dagu test", }, }, { @@ -59,7 +63,9 @@ func TestLoadConfig(t *testing.T) { Env: []string{}, LogDir: settings.MustGet( settings.SETTING__ADMIN_LOGS_DIR), - BaseConfig: settings.MustGet(settings.SETTING__BASE_CONFIG), + BaseConfig: settings.MustGet(settings.SETTING__BASE_CONFIG), + NavbarColor: "", + NavbarTitle: "Dagu", }, }, } { diff --git a/internal/admin/definition.go b/internal/admin/definition.go index 539f90ac2..7a9ffdf69 100644 --- a/internal/admin/definition.go +++ b/internal/admin/definition.go @@ -13,4 +13,6 @@ type configDefinition struct { BasicAuthUsername string BasicAuthPassword string LogEncodingCharset string + NavbarColor string + NavbarTitle string } diff --git a/internal/admin/handlers/dag.go b/internal/admin/handlers/dag.go index 61549888f..bedc16ac0 100644 --- a/internal/admin/handlers/dag.go +++ b/internal/admin/handlers/dag.go @@ -89,8 +89,8 @@ type DAGHandlerConfig struct { LogEncodingCharset string } -func HandleGetDAG(hc *DAGHandlerConfig) http.HandlerFunc { - renderFunc := useTemplate("index.gohtml", "dag") +func HandleGetDAG(hc *DAGHandlerConfig, tc *TemplateConfig) http.HandlerFunc { + renderFunc := useTemplate("index.gohtml", "dag", tc) return func(w http.ResponseWriter, r *http.Request) { dn, err := getPathParameter(r) diff --git a/internal/admin/handlers/html.go b/internal/admin/handlers/html.go index 2adb0904e..f8f8c79a8 100644 --- a/internal/admin/handlers/html.go +++ b/internal/admin/handlers/html.go @@ -10,37 +10,41 @@ import ( "text/template" "github.com/yohamta/dagu/internal/constants" - "github.com/yohamta/dagu/internal/settings" ) -var defaultFuncs = template.FuncMap{ - "defTitle": func(ip interface{}) string { - v, ok := ip.(string) - if !ok || (ok && v == "") { - return "" - } - return v - }, - "version": func() string { - return constants.Version - }, - "navbarColor": func() string { - c, _ := settings.Get(settings.SETTING__ADMIN_NAVBAR_COLOR) - return c - }, - "navbarTitle": func() string { - val, _ := settings.Get(settings.SETTING__ADMIN_NAVBAR_TITLE) - return val - }, +type TemplateConfig struct { + NavbarColor string + NavbarTitle string +} + +func defaultFuncs(tc *TemplateConfig) template.FuncMap { + return template.FuncMap{ + "defTitle": func(ip interface{}) string { + v, ok := ip.(string) + if !ok || (ok && v == "") { + return "" + } + return v + }, + "version": func() string { + return constants.Version + }, + "navbarColor": func() string { + return tc.NavbarColor + }, + "navbarTitle": func() string { + return tc.NavbarTitle + }, + } } //go:embed web/templates/* web/assets/* var assets embed.FS var templatePath = "web/templates/" -func useTemplate(layout string, name string) func(http.ResponseWriter, interface{}) { +func useTemplate(layout string, name string, tc *TemplateConfig) func(http.ResponseWriter, interface{}) { files := append(baseTemplates(), path.Join(templatePath, layout)) - tmpl, err := template.New(name).Funcs(defaultFuncs).ParseFS(assets, files...) + tmpl, err := template.New(name).Funcs(defaultFuncs(tc)).ParseFS(assets, files...) if err != nil { panic(err) } diff --git a/internal/admin/handlers/list.go b/internal/admin/handlers/list.go index 795635152..e1c1359aa 100644 --- a/internal/admin/handlers/list.go +++ b/internal/admin/handlers/list.go @@ -22,8 +22,8 @@ type DAGListHandlerConfig struct { DAGsDir string } -func HandleGetList(hc *DAGListHandlerConfig) http.HandlerFunc { - renderFunc := useTemplate("index.gohtml", "index") +func HandleGetList(hc *DAGListHandlerConfig, tc *TemplateConfig) http.HandlerFunc { + renderFunc := useTemplate("index.gohtml", "index", tc) return func(w http.ResponseWriter, r *http.Request) { dir := filepath.Join(hc.DAGsDir) dags, errs, err := controller.GetDAGs(dir) diff --git a/internal/admin/loader.go b/internal/admin/loader.go index 9146c739c..b762a1f0a 100644 --- a/internal/admin/loader.go +++ b/internal/admin/loader.go @@ -13,13 +13,6 @@ import ( type Loader struct{} -func DefaultConfig() *Config { - c := &Config{} - c.Init() - c.setup() - return c -} - func (cl *Loader) LoadAdminConfig(file string) (*Config, error) { if !utils.FileExists(file) { return nil, ErrConfigNotFound @@ -56,13 +49,16 @@ func (cl *Loader) LoadAdminConfig(file string) (*Config, error) { cfg, err = buildFromDefinition(def) return err }, + func() (err error) { + err = cfg.setup() + return err + }, } { if err := fn(); err != nil { return nil, err } } - cfg.setup() return cfg, nil } diff --git a/internal/admin/loader_test.go b/internal/admin/loader_test.go index 95be91dbf..b3b02fe2c 100644 --- a/internal/admin/loader_test.go +++ b/internal/admin/loader_test.go @@ -11,7 +11,8 @@ import ( var testsConfig = path.Join(testsDir, "admin.yaml") func TestDefaultConfig(t *testing.T) { - cfg := DefaultConfig() + cfg, err := DefaultConfig() + require.NoError(t, err) testConfig(t, cfg, &testWant{ Host: "127.0.0.1", Port: "8080", diff --git a/internal/admin/routes.go b/internal/admin/routes.go index cc13c8b37..2999b05b8 100644 --- a/internal/admin/routes.go +++ b/internal/admin/routes.go @@ -13,32 +13,30 @@ type route struct { } func defaultRoutes(cfg *Config) []*route { + tc := &handlers.TemplateConfig{ + NavbarColor: cfg.NavbarColor, + NavbarTitle: cfg.NavbarTitle, + } return []*route{ {http.MethodGet, `^/?$`, handlers.HandleGetList( - &handlers.DAGListHandlerConfig{ - DAGsDir: cfg.DAGs, - }, + &handlers.DAGListHandlerConfig{DAGsDir: cfg.DAGs}, + tc, )}, {http.MethodPost, `^/?$`, handlers.HandlePostList( - &handlers.DAGListHandlerConfig{ - DAGsDir: cfg.DAGs, - }, + &handlers.DAGListHandlerConfig{DAGsDir: cfg.DAGs}, )}, {http.MethodGet, `^/dags/?$`, handlers.HandleGetList( - &handlers.DAGListHandlerConfig{ - DAGsDir: cfg.DAGs, - }, + &handlers.DAGListHandlerConfig{DAGsDir: cfg.DAGs}, + tc, )}, {http.MethodPost, `^/dags/?$`, handlers.HandlePostList( - &handlers.DAGListHandlerConfig{ - DAGsDir: cfg.DAGs, - }, + &handlers.DAGListHandlerConfig{DAGsDir: cfg.DAGs}, )}, {http.MethodGet, `^/dags/([^/]+)$`, handlers.HandleGetDAG( &handlers.DAGHandlerConfig{ DAGsDir: cfg.DAGs, LogEncodingCharset: cfg.LogEncodingCharset, - }, + }, tc, )}, {http.MethodPost, `^/dags/([^/]+)$`, handlers.HandlePostDAG( &handlers.PostDAGHandlerConfig{ diff --git a/internal/settings/settings.go b/internal/settings/settings.go index a3897f925..6a6258394 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -13,19 +13,18 @@ var ( ) const ( - SETTING__HOME = "DAGU_HOME" - - // TODO: consider declaring these consts as enum when changed not to read from env - SETTING__DATA_DIR = "DAGU__DATA" - SETTING__LOGS_DIR = "DAGU__LOGS" - SETTING__SUSPEND_FLAGS_DIR = "DAGU__SUSPEND_FLAGS_DIR" - SETTING__BASE_CONFIG = "DAGU__BASE_CONFIG" - SETTING__ADMIN_CONFIG = "DAGU__ADMIN_CONFIG" - SETTING__ADMIN_PORT = "DAGU__ADMIN_PORT" + SETTING__HOME = "DAGU_HOME" SETTING__ADMIN_NAVBAR_COLOR = "DAGU__ADMIN_NAVBAR_COLOR" SETTING__ADMIN_NAVBAR_TITLE = "DAGU__ADMIN_NAVBAR_TITLE" - SETTING__ADMIN_LOGS_DIR = "DAGU__ADMIN_LOGS_DIR" - SETTING__ADMIN_DAGS_DIR = "DAGU__ADMIN_DAGS_DIR" + + SETTING__ADMIN_PORT = "DAGU__ADMIN_PORT" + SETTING__DATA_DIR = "DAGU__DATA" + SETTING__LOGS_DIR = "DAGU__LOGS" + SETTING__SUSPEND_FLAGS_DIR = "DAGU__SUSPEND_FLAGS_DIR" + SETTING__BASE_CONFIG = "DAGU__BASE_CONFIG" + SETTING__ADMIN_CONFIG = "DAGU__ADMIN_CONFIG" + SETTING__ADMIN_LOGS_DIR = "DAGU__ADMIN_LOGS_DIR" + SETTING__ADMIN_DAGS_DIR = "DAGU__ADMIN_DAGS_DIR" ) // MustGet returns the value of the setting or @@ -46,6 +45,11 @@ func Get(name string) (string, error) { return "", ErrSettingNotFound } +// Set sets the value of the setting. +func Set(key, val string) { + cache[key] = val +} + // ChangeHomeDir changes the home directory and reloads // the settings. func ChangeHomeDir(homeDir string) { @@ -67,16 +71,14 @@ func load() { cache[SETTING__ADMIN_CONFIG] = path.Join(dh, "admin.yaml") cache[SETTING__BASE_CONFIG] = path.Join(dh, "config.yaml") - - // TODO: consider reading these settings from env - cacheEnv(SETTING__DATA_DIR, path.Join(dh, "/data")) - cacheEnv(SETTING__LOGS_DIR, path.Join(dh, "/logs")) - cacheEnv(SETTING__SUSPEND_FLAGS_DIR, path.Join(dh, "/suspend")) - cacheEnv(SETTING__ADMIN_NAVBAR_COLOR, "") - cacheEnv(SETTING__ADMIN_NAVBAR_TITLE, "Dagu admin") - cacheEnv(SETTING__ADMIN_PORT, "8080") - cacheEnv(SETTING__ADMIN_LOGS_DIR, path.Join(dh, "/logs/admin")) - cacheEnv(SETTING__ADMIN_DAGS_DIR, path.Join(dh, "/dags")) + cache[SETTING__DATA_DIR] = path.Join(dh, "/data") + cache[SETTING__LOGS_DIR] = path.Join(dh, "/logs") + cache[SETTING__SUSPEND_FLAGS_DIR] = path.Join(dh, "/suspend") + cache[SETTING__ADMIN_LOGS_DIR] = path.Join(dh, "/logs/admin") + cache[SETTING__ADMIN_DAGS_DIR] = path.Join(dh, "/dags") + cache[SETTING__ADMIN_PORT] = "8080" + cache[SETTING__ADMIN_NAVBAR_COLOR] = "" + cache[SETTING__ADMIN_NAVBAR_TITLE] = "Dagu" } func cacheEnv(key, def string) { diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index 0b8e251b9..bab289a84 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -42,17 +42,18 @@ func TestReadSetting(t *testing.T) { } // read from enviroment variables + _ = os.Setenv(SETTING__HOME, "/tmp/dagu/") for _, test := range []struct { Name string Want string }{ { Name: SETTING__DATA_DIR, - Want: "/home/dagu/data", + Want: "/tmp/dagu/data", }, { Name: SETTING__LOGS_DIR, - Want: "/home/dagu/logs", + Want: "/tmp/dagu/logs", }, } { _ = os.Setenv(test.Name, test.Want) @@ -60,10 +61,10 @@ func TestReadSetting(t *testing.T) { val, err := Get(test.Name) require.NoError(t, err) - require.Equal(t, val, test.Want) + require.Equal(t, test.Want, val) val = MustGet(test.Name) - require.Equal(t, val, test.Want) + require.Equal(t, test.Want, val) } _, err := Get("Invalid_Name")