diff --git a/cmd/logger.go b/cmd/logger.go index 6c12e4bb3..694e944f1 100644 --- a/cmd/logger.go +++ b/cmd/logger.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/dagu-org/dagu/internal/cmdutil" "github.com/dagu-org/dagu/internal/config" "github.com/dagu-org/dagu/internal/fileutil" "github.com/dagu-org/dagu/internal/logger" @@ -52,17 +53,23 @@ type logFileSettings struct { // openLogFile creates and opens a log file based on the provided settings. // It creates the necessary directory structure and returns the file handle. func openLogFile(config logFileSettings) (*os.File, error) { + logDir, err := cmdutil.SubstituteCommands(os.ExpandEnv(config.LogDir)) + if err != nil { + return nil, fmt.Errorf("failed to expand log directory: %w", err) + } + config.LogDir = logDir + if err := validateSettings(config); err != nil { return nil, fmt.Errorf("invalid log settings: %w", err) } - logDir, err := setupLogDirectory(config) + outputDir, err := setupLogDirectory(config) if err != nil { return nil, fmt.Errorf("failed to setup log directory: %w", err) } filename := buildLogFilename(config) - return createLogFile(filepath.Join(logDir, filename)) + return createLogFile(filepath.Join(outputDir, filename)) } // validateSettings ensures all required fields are properly set diff --git a/go.mod b/go.mod index f1255418e..df54bbb80 100644 --- a/go.mod +++ b/go.mod @@ -98,6 +98,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/firefart/nonamedreturns v1.0.5 // indirect github.com/fzipp/gocyclo v0.6.0 // indirect + github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/ghostiam/protogetter v0.3.8 // indirect github.com/go-critic/go-critic v0.11.5 // indirect github.com/go-logr/logr v1.4.2 // indirect @@ -106,6 +107,9 @@ require ( github.com/go-openapi/inflect v0.21.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect + github.com/go-playground/locales v0.14.1 // indirect + github.com/go-playground/universal-translator v0.18.1 // indirect + github.com/go-playground/validator/v10 v10.23.0 // indirect github.com/go-toolsmith/astcast v1.1.0 // indirect github.com/go-toolsmith/astcopy v1.1.0 // indirect github.com/go-toolsmith/astequal v1.2.0 // indirect @@ -158,6 +162,7 @@ require ( github.com/lasiar/canonicalheader v1.1.2 // indirect github.com/ldez/gomoddirectives v0.2.4 // indirect github.com/ldez/tagliatelle v0.5.0 // indirect + github.com/leodido/go-urn v1.4.0 // indirect github.com/leonklingele/grouper v1.1.2 // indirect github.com/macabu/inamedparam v0.1.3 // indirect github.com/magiconair/properties v1.8.7 // indirect diff --git a/go.sum b/go.sum index bb4771b18..d06fca4af 100644 --- a/go.sum +++ b/go.sum @@ -191,6 +191,8 @@ github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nos github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/fzipp/gocyclo v0.6.0 h1:lsblElZG7d3ALtGMx9fmxeTKZaLLpU8mET09yN4BBLo= github.com/fzipp/gocyclo v0.6.0/go.mod h1:rXPyn8fnlpa0R2csP/31uerbiVBugk5whMdlyaLkLoA= +github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0= +github.com/gabriel-vasile/mimetype v1.4.3/go.mod h1:d8uq/6HKRL6CGdk+aubisF/M5GcPfT7nKyLpA0lbSSk= github.com/ghostiam/protogetter v0.3.8 h1:LYcXbYvybUyTIxN2Mj9h6rHrDZBDwZloPoKctWrFyJY= github.com/ghostiam/protogetter v0.3.8/go.mod h1:WZ0nw9pfzsgxuRsPOFQomgDVSWtDLJRfQJEhsGbmQMA= github.com/go-chi/chi/v5 v5.0.8 h1:lD+NLqFcAi1ovnVZpsnObHGW4xb4J8lNmoYVfECH1Y0= @@ -233,6 +235,12 @@ github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+Gr github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58= github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ= +github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= +github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= +github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= +github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= +github.com/go-playground/validator/v10 v10.23.0 h1:/PwmTwZhS0dPkav3cdK9kV1FsAmrL8sThn8IHr/sO+o= +github.com/go-playground/validator/v10 v10.23.0/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI= github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow= github.com/go-resty/resty/v2 v2.7.0 h1:me+K9p3uhSmXtrBZ4k9jcEAfJmuC8IivWHwaLZwPrFY= @@ -455,6 +463,8 @@ github.com/ldez/gomoddirectives v0.2.4 h1:j3YjBIjEBbqZ0NKtBNzr8rtMHTOrLPeiwTkfUJ github.com/ldez/gomoddirectives v0.2.4/go.mod h1:oWu9i62VcQDYp9EQ0ONTfqLNh+mDLWWDO+SO0qSQw5g= github.com/ldez/tagliatelle v0.5.0 h1:epgfuYt9v0CG3fms0pEgIMNPuFf/LpPIfjk4kyqSioo= github.com/ldez/tagliatelle v0.5.0/go.mod h1:rj1HmWiL1MiKQuOONhd09iySTEkUuE/8+5jtPYz9xa4= +github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= +github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/leonklingele/grouper v1.1.2 h1:o1ARBDLOmmasUaNDesWqWCIFH3u7hoFlM84YrjT3mIY= github.com/leonklingele/grouper v1.1.2/go.mod h1:6D0M/HVkhs2yRKRFZUoGjeDy7EZTfFBE9gl4kjmIGkA= github.com/macabu/inamedparam v0.1.3 h1:2tk/phHkMlEL/1GNe/Yf6kkR/hkcUdAEY3L0hjYV1Mk= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index af8d5630c..2984f2ac6 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -17,6 +17,7 @@ import ( "time" "github.com/dagu-org/dagu/internal/client" + "github.com/dagu-org/dagu/internal/cmdutil" "github.com/dagu-org/dagu/internal/digraph" "github.com/dagu-org/dagu/internal/digraph/scheduler" "github.com/dagu-org/dagu/internal/logger" @@ -308,19 +309,23 @@ func (a *Agent) HandleHTTP(ctx context.Context) sock.HTTPHandlerFunc { // setup the agent instance for DAG execution. func (a *Agent) setup(ctx context.Context) error { - // Lock to prevent race condition. a.lock.Lock() defer a.lock.Unlock() a.scheduler = a.newScheduler() - a.reporter = newReporter( - mailer.New(&mailer.NewMailerArgs{ - Host: a.dag.SMTP.Host, - Port: a.dag.SMTP.Port, - Username: a.dag.SMTP.Username, - Password: a.dag.SMTP.Password, - }), - ) + + mailerConfig, err := cmdutil.SubstituteStringFields(mailer.Config{ + Host: a.dag.SMTP.Host, + Port: a.dag.SMTP.Port, + Username: a.dag.SMTP.Username, + Password: a.dag.SMTP.Password, + }) + if err != nil { + return fmt.Errorf("failed to substitute mailer options: %w", err) + } + + mailer := mailer.New(mailerConfig) + a.reporter = newReporter(mailer) return a.setupGraph(ctx) } diff --git a/internal/cmdutil/cmdutil.go b/internal/cmdutil/cmdutil.go index 73a9ec22d..b705d5a40 100644 --- a/internal/cmdutil/cmdutil.go +++ b/internal/cmdutil/cmdutil.go @@ -236,8 +236,6 @@ func GetShellCommand(configuredShell string) string { // modified struct value. func SubstituteStringFields[T any](obj T) (T, error) { v := reflect.ValueOf(obj) - k := v.Kind() - println(k) if v.Kind() != reflect.Struct { return obj, fmt.Errorf("input must be a struct, got %T", obj) } diff --git a/internal/digraph/builder_dag.go b/internal/digraph/builder_dag.go index 6a64f6110..bf1d57d25 100644 --- a/internal/digraph/builder_dag.go +++ b/internal/digraph/builder_dag.go @@ -8,8 +8,6 @@ import ( "os" "strings" "time" - - "github.com/dagu-org/dagu/internal/cmdutil" ) var builderRegistry = []builderEntry{ @@ -109,12 +107,8 @@ func buildSteps(ctx BuildContext, spec *definition, dag *DAG) error { // buildLogDir builds the log directory for the DAG. func buildLogDir(_ BuildContext, spec *definition, dag *DAG) (err error) { - logDir, err := cmdutil.SubstituteCommands(os.ExpandEnv(spec.LogDir)) - if err != nil { - return err - } - dag.LogDir = logDir - return err + dag.LogDir = spec.LogDir + return nil } // buildHandlers builds the handlers for the DAG. diff --git a/internal/digraph/dag.go b/internal/digraph/dag.go index 45651d4e0..6a81d6759 100644 --- a/internal/digraph/dag.go +++ b/internal/digraph/dag.go @@ -45,9 +45,6 @@ type DAG struct { // replaced with '_'). LogDir string `json:"LogDir"` - // LogFormat is the format of the log (text or json). - LogFormat string `json:"LogFormat"` - // Parameters configuration. // The DAG definition contains only DefaultParams. Params are automatically // set by the DAG loader. diff --git a/internal/digraph/executors/mail.go b/internal/digraph/executors/mail.go index dc82d4777..c7b19f599 100644 --- a/internal/digraph/executors/mail.go +++ b/internal/digraph/executors/mail.go @@ -9,11 +9,15 @@ import ( "io" "os" + "github.com/dagu-org/dagu/internal/cmdutil" "github.com/dagu-org/dagu/internal/digraph" "github.com/dagu-org/dagu/internal/mailer" + govalidator "github.com/go-playground/validator/v10" "github.com/mitchellh/mapstructure" ) +var validate = govalidator.New() + type mail struct { stdout io.Writer stderr io.Writer @@ -22,35 +26,47 @@ type mail struct { } type mailConfig struct { - From string `mapstructure:"from"` - To string `mapstructure:"to"` - Subject string `mapstructure:"subject"` + From string `mapstructure:"from" validate:"required,email"` + To string `mapstructure:"to" validate:"required,email"` + Subject string `mapstructure:"subject" validate:"required"` Message string `mapstructure:"message"` } func newMail(ctx context.Context, step digraph.Step) (Executor, error) { - var cfg mailConfig - if err := decodeMailConfig(step.ExecutorConfig.Config, &cfg); err != nil { + var mailConfig mailConfig + if err := decodeMailConfig(step.ExecutorConfig.Config, &mailConfig); err != nil { return nil, err } - cfg.From = os.ExpandEnv(cfg.From) - cfg.To = os.ExpandEnv(cfg.To) - cfg.Subject = os.ExpandEnv(cfg.Subject) - cfg.Message = os.ExpandEnv(cfg.Message) + mailConfig, err := cmdutil.SubstituteStringFields(mailConfig) + if err != nil { + return nil, fmt.Errorf("failed to substitute mail config: %w", err) + } + + // Validate the configuration. + if err := validate.Struct(mailConfig); err != nil { + return nil, fmt.Errorf("invalid mail configuration: %w", err) + } - exec := &mail{cfg: &cfg} + exec := &mail{cfg: &mailConfig} dagCtx, err := digraph.GetContext(ctx) if err != nil { return nil, err } - m := mailer.New(&mailer.NewMailerArgs{ + + mailerConfig, err := cmdutil.SubstituteStringFields(mailer.Config{ Host: dagCtx.DAG.SMTP.Host, Port: dagCtx.DAG.SMTP.Port, Username: dagCtx.DAG.SMTP.Username, Password: dagCtx.DAG.SMTP.Password, }) + + if err != nil { + return nil, fmt.Errorf("failed to substitute mailer options: %w", err) + } + + m := mailer.New(mailerConfig) exec.mailer = m return exec, nil diff --git a/internal/digraph/executors/mail_test.go b/internal/digraph/executors/mail_test.go new file mode 100644 index 000000000..5015b2149 --- /dev/null +++ b/internal/digraph/executors/mail_test.go @@ -0,0 +1,93 @@ +package executors + +import ( + "context" + "os" + "testing" + + "github.com/dagu-org/dagu/internal/digraph" + "github.com/stretchr/testify/assert" +) + +func TestMail(t *testing.T) { + t.Parallel() + + os.Setenv("MAIL_SUBJECT", "Test Subject") + t.Cleanup(func() { + os.Unsetenv("MAIL_SUBJECT") + }) + + t.Run("NewMail", func(t *testing.T) { + tests := []struct { + name string + step digraph.Step + expectedError bool + }{ + { + name: "ValidConfig", + step: digraph.Step{ + ExecutorConfig: digraph.ExecutorConfig{ + Config: map[string]any{ + "from": "test@example.com", + "to": "recipient@example.com", + "subject": "Test Subject", + "message": "Test Message", + }, + }, + }, + expectedError: false, + }, + { + name: "ValidConfigWithEnv", + step: digraph.Step{ + ExecutorConfig: digraph.ExecutorConfig{ + Config: map[string]any{ + "from": "test@example.com", + "to": "recipient@example.com", + "subject": "${MAIL_SUBJECT}", + "message": "Test Message", + }, + }, + }, + expectedError: false, + }, + { + name: "InvalidConfig", + step: digraph.Step{ + ExecutorConfig: digraph.ExecutorConfig{ + Config: map[string]any{ + "from": "test@example.com", + }, + }, + }, + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ctx = digraph.NewContext(ctx, &digraph.DAG{ + SMTP: &digraph.SMTPConfig{}, + }, nil, nil, "", "") + + exec, err := newMail(ctx, tt.step) + + if tt.expectedError { + assert.Error(t, err) + assert.Nil(t, exec) + } else { + assert.NoError(t, err) + assert.NotNil(t, exec) + + mailExec, ok := exec.(*mail) + assert.True(t, ok) + assert.Equal(t, "test@example.com", mailExec.cfg.From) + assert.Equal(t, "recipient@example.com", mailExec.cfg.To) + assert.Equal(t, "Test Subject", mailExec.cfg.Subject) + assert.Equal(t, "Test Message", mailExec.cfg.Message) + } + }) + } + }) +} diff --git a/internal/mailer/mailer.go b/internal/mailer/mailer.go index 961798939..28d7fb17e 100644 --- a/internal/mailer/mailer.go +++ b/internal/mailer/mailer.go @@ -23,15 +23,15 @@ type Mailer struct { password string } -// NewMailerArgs is a config for SMTP mailer. -type NewMailerArgs struct { +// Config is a config for SMTP mailer. +type Config struct { Host string Port string Username string Password string } -func New(cfg *NewMailerArgs) *Mailer { +func New(cfg Config) *Mailer { return &Mailer{ host: cfg.Host, port: cfg.Port,