treasure-data / digdag

Workload Automation System
https://www.digdag.io/
Apache License 2.0
1.31k stars 222 forks source link

mail operator does not use secret "mail.port" #1383

Closed fukasawah closed 1 year ago

fukasawah commented 4 years ago

Related issue: #948

I also encountered this problem. And i guessed the cause by this problem, so I'd like to propose a fix.

Problem

https://github.com/treasure-data/digdag/blob/v0.9.41/digdag-standards/src/main/java/io/digdag/standards/operator/MailOperatorFactory.java#L291

                .port(secrets.getSecretOptional("port").transform(Integer::parseInt).or(params.get("port", int.class)))

This code was always get mail operator config "port". if not defined mail operator config "port", throw exception regardless of secrets.

Proposed fix

-                 .port(secrets.getSecretOptional("port").transform(Integer::parseInt).or(params.get("port", int.class)))
+                 .port(secrets.getSecretOptional("port").transform(Integer::parseInt).or(() -> params.get("port", int.class)))

It works by lazy get data from mail operator config using Supplier. Or i think that can set the default value 587.

Reproduction

files

digdag-test-mail.dig

_export:
  mail:
    from: "fukasawah@hoge.example"

+step1:
  mail>: email-body.txt
  subject: workflow started
  to: [fukasawah@hoge.example]

.mail-secret.json

{
  "mail.host": "smtp.*********.com",
  "mail.port": "587",
  "mail.username": "fukasawah@hoge.example",
  "mail.password": "**************",
  "mail.tls": "true"
}

command

digdag push email-test
digdag secret --project email-test --set @.mail-secret.json

task log

2020-03-26 12:55:51.618 +0900 [INFO] (0044@[0:email-test]+digdag-test-mail+step1) io.digdag.core.agent.OperatorManager: mail>: email-body.txt 2020-03-26 12:55:51.996 +0900 [ERROR] (0044@[0:email-test]+digdag-test-mail+step1) io.digdag.core.agent.OperatorManager: Configuration error at task +digdag-test-mail+step1: Parameter 'port' is required but not set (config)

yoyama commented 1 year ago

I am not sure port should be credential and should be set in secret. I close the ticket. If you think still need, please re-open or file a new one.