dagu-org / dagu

Developer-friendly, minimalism Cron alternative, but with much more capabilities. It aims to solve greater problems.
https://dagu.readthedocs.io
GNU General Public License v3.0
1.63k stars 143 forks source link

[ISSUE #674] feat: Add support for env variables in executor config #677

Closed halalala222 closed 1 month ago

halalala222 commented 2 months ago

export env

export DAGU_SSH_CONFIG_PASSWORD="password"
export DAGU_SSH_CONFIG_IP="ip"
export DAGU_SSH_CONFIG_USER="user"
export DAGU_SSH_CONFIG_PORT="22"

func newSSHExec(_ context.Context, step dag.Step) (Executor, error) { // expend env if err = expendExecConfigEnv(step.ExecutorConfig.Config); err != nil { return nil, err }

if err = md.Decode(step.ExecutorConfig.Config); err != nil {
    return nil, err
}

} 75d2aff794bde8915dd250eb16684a0 74cd5158848c2808e1149058db0257e

yohamta commented 2 months ago

Hi @halalala222, thank you very much for your great work! After reviewing the code, I noticed that it's replacing specific environment variables. I apologize for not being clear about this earlier, but we need to expand all environment variables included in the Config. For example, it should work like this:


steps:
  - name: step1
    executor:
      type: ssh
      config:
        user: "${SSH_USER}"
        ip: "${SSH_HOST}"
        port: "${SSH_PORT}"
        key: "${HOME}/.ssh/id_rsa"
    command: /usr/sbin/ifconfig

I think we can achieve this using the following approach:

// expandEnvHook is a mapstructure decode hook that expands environment variables in string fields
func expandEnvHook(f reflect.Type, t reflect.Type, data interface{}) (interface{}, error) {
    if f.Kind() != reflect.String || t.Kind() != reflect.String {
        return data, nil
    }
    return os.ExpandEnv(data.(string)), nil
}

func newSSHExec(_ context.Context, step dag.Step) (Executor, error) {
    cfg := new(sshExecConfig)
    decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
        Result: cfg,
        DecodeHook: expandEnvHook,
    })
    // ... omitted ...
}

Additionally, if possible, it would be great if you could add some tests for this functionality. Thank you very much for your help. It's really appreciated!

halalala222 commented 2 months ago

Hi ! @yohamta Thank you ! I know ! I will try to work on it.

yohamta commented 1 month ago

Hi @halalala222, I understand you might be busy right now. Would you mind if I take over the task? If you'd prefer to continue working on it, that's great too.

halalala222 commented 1 month ago

Hi @yohamta ,I'm really sorry, but I have some matters to attend to recently and may not be able to cover this task anymore. I apologize for the inconvenience.

yohamta commented 1 month ago

Hi @halalala222, no worries at all! Thank you so much for all the wonderful work you've done on developing Dagu, it was incredibly helpful. I'm closing this for now.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.68%. Comparing base (3c4f636) to head (b20c3ab). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/dagu-org/dagu/pull/677/graphs/tree.svg?width=650&height=150&src=pr&token=CODZQP61J2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org)](https://app.codecov.io/gh/dagu-org/dagu/pull/677?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org) ```diff @@ Coverage Diff @@ ## main #677 +/- ## ======================================= Coverage 64.68% 64.68% ======================================= Files 53 53 Lines 4315 4315 ======================================= Hits 2791 2791 Misses 1269 1269 Partials 255 255 ``` ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/dagu-org/dagu/pull/677?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/dagu-org/dagu/pull/677?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org). Last update [3c4f636...b20c3ab](https://app.codecov.io/gh/dagu-org/dagu/pull/677?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dagu-org).