From b4fb05127933965bfc1757f8a2b0507238a7b7a6 Mon Sep 17 00:00:00 2001 From: Daniel Jagszent Date: Mon, 13 Mar 2023 20:07:27 +0100 Subject: [PATCH] chore: make integration tests more robust --- Makefile | 8 +++--- integration/docker/syslog-ng.conf | 1 + integration/go.mod | 4 +-- integration/mta/mock/mta.go | 18 ++++++++------ integration/mta/mock/mta.sh | 2 -- integration/mta/postfix/mta.sh | 2 -- integration/mta/script.sh | 1 + integration/mta/sendmail/mta.sh | 8 +++--- integration/mta/sendmail/sendmail.cf | 18 +++++++------- integration/runner/config.go | 23 +++++++++++++++--- integration/runner/main.go | 6 ++++- integration/runner/mta.go | 18 ++++++++------ integration/runner/runner.go | 27 +++++++++------------ integration/testcase.go | 8 +++--- integration/tests/body/test.go | 2 -- integration/tests/header/test.go | 2 -- integration/tests/mail-from/change.testcase | 3 +++ integration/tests/mail-from/test.go | 9 +++++-- integration/tests/rcpt-to/add.testcase | 4 +++ integration/tests/rcpt-to/change.testcase | 3 +++ integration/tests/rcpt-to/test.go | 10 ++++++-- 21 files changed, 107 insertions(+), 70 deletions(-) create mode 100644 integration/tests/mail-from/change.testcase create mode 100644 integration/tests/rcpt-to/add.testcase create mode 100644 integration/tests/rcpt-to/change.testcase diff --git a/Makefile b/Makefile index 76e862a..ae8ed8a 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,8 @@ -GO_MILTER_DIR := $(shell go list -f '{{.Dir}}' github.com/d--j/go-milter) +GO_MILTER_INTEGRATION_DIR := $(shell cd integration && go list -f '{{.Dir}}' github.com/d--j/go-milter/integration) integration: - docker build -q --progress=plain -t go-milter-integration "$(GO_MILTER_DIR)/integration/docker" && \ - docker run --rm -w /usr/src/root/integration -v $(PWD):/usr/src/root go-milter-integration \ - go run github.com/d--j/go-milter/integration/runner -filter '.*' ./tests + docker build -q --progress=plain -t go-milter-integration "$(GO_MILTER_INTEGRATION_DIR)/docker" && \ + docker run --rm --hostname=mx.example.com -w /usr/src/root/integration -v $(PWD):/usr/src/root go-milter-integration \ + go run github.com/d--j/go-milter/integration/runner -filter '.*' -mtaFilter '.*' ./tests .PHONY: integration diff --git a/integration/docker/syslog-ng.conf b/integration/docker/syslog-ng.conf index 63a99a3..217f0f4 100644 --- a/integration/docker/syslog-ng.conf +++ b/integration/docker/syslog-ng.conf @@ -2,6 +2,7 @@ @include "scl.conf" source s_local { + unix-stream("/dev/log"); internal(); }; diff --git a/integration/go.mod b/integration/go.mod index a4ff759..2106e72 100644 --- a/integration/go.mod +++ b/integration/go.mod @@ -3,17 +3,17 @@ module github.com/d--j/go-milter/integration go 1.18 require ( - github.com/d--j/go-milter v0.6.0 + github.com/d--j/go-milter v0.6.5 github.com/emersion/go-message v0.16.0 github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21 github.com/emersion/go-smtp v0.16.0 + golang.org/x/text v0.7.0 golang.org/x/tools v0.1.12 ) require ( github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594 // indirect golang.org/x/net v0.7.0 // indirect - golang.org/x/text v0.7.0 // indirect ) replace github.com/d--j/go-milter => ../ diff --git a/integration/mta/mock/mta.go b/integration/mta/mock/mta.go index 0609174..31a8fcd 100644 --- a/integration/mta/mock/mta.go +++ b/integration/mta/mock/mta.go @@ -46,8 +46,9 @@ type Backend struct { func (bkd *Backend) NewSession(conn *smtp.Conn) (smtp.Session, error) { macros := milter.NewMacroBag() - macros.Set(milter.MacroMTAFullyQualifiedDomainName, "localhost.local") - macros.Set(milter.MacroDaemonName, "localhost.local") + macros.Set(milter.MacroMTAVersion, "MOCK-SMTP 0.0.0") + macros.Set(milter.MacroMTAFQDN, "localhost.local") + macros.Set(milter.MacroDaemonName, "mock-smtp") macros.Set(milter.MacroIfName, "eth99") addr, _, err := net.SplitHostPort(conn.Conn().LocalAddr().String()) if err != nil { @@ -289,21 +290,22 @@ func (s *Session) Data(r io.Reader) error { for _, act := range modActions { switch act.Type { case milter.ActionChangeFrom: - log.Printf("[%s] ACT = ActionChangeFrom <%s> %s", s.queueId, act.From, act.FromArgs) - s.MailFrom = act.From + log.Printf("[%s] ACT = ActionChangeFrom %s %s", s.queueId, act.From, act.FromArgs) + s.MailFrom = milter.RemoveAngle(act.From) s.MailFromArgs = act.FromArgs case milter.ActionDelRcpt: - log.Printf("[%s] ACT = ActionDelRcpt <%s>", s.queueId, act.Rcpt) + log.Printf("[%s] ACT = ActionDelRcpt %s", s.queueId, act.Rcpt) + rcpt := milter.RemoveAngle(act.Rcpt) again: for i, r := range s.Recipients { - if act.Rcpt == r.Addr { + if rcpt == r.Addr { s.Recipients = append(s.Recipients[:i], s.Recipients[i+1:]...) goto again } } case milter.ActionAddRcpt: - log.Printf("[%s] ACT = ActionAddRcpt <%s> %s", s.queueId, act.Rcpt, act.RcptArgs) - s.Recipients = append(s.Recipients, Rcpt{Addr: act.Rcpt, Args: act.RcptArgs}) + log.Printf("[%s] ACT = ActionAddRcpt %s %s", s.queueId, act.Rcpt, act.RcptArgs) + s.Recipients = append(s.Recipients, Rcpt{Addr: milter.RemoveAngle(act.Rcpt), Args: act.RcptArgs}) case milter.ActionReplaceBody: log.Printf("[%s] ACT = ActionReplaceBody %q", s.queueId, act.Body) replacedBody = append(replacedBody, act.Body...) diff --git a/integration/mta/mock/mta.sh b/integration/mta/mock/mta.sh index cf285f2..40b79f5 100755 --- a/integration/mta/mock/mta.sh +++ b/integration/mta/mock/mta.sh @@ -16,14 +16,12 @@ if [ "tags" = "$1" ]; then fi if [ "start" = "$1" ]; then - shift parse_args "$@" go build -o "$SCRATCH_DIR/mta.exe" -v "$SCRIPT_DIR" exec "$SCRATCH_DIR/mta.exe" -mta ":$MTA_PORT" -next ":$RECEIVER_PORT" -milter ":$MILTER_PORT" -cert "$SCRATCH_DIR/../cert.pem" -key "$SCRATCH_DIR/../key.pem" fi if [ "stop" = "$1" ]; then - shift parse_args "$@" exit 0 fi diff --git a/integration/mta/postfix/mta.sh b/integration/mta/postfix/mta.sh index 88a837b..f0a52ad 100755 --- a/integration/mta/postfix/mta.sh +++ b/integration/mta/postfix/mta.sh @@ -137,7 +137,6 @@ setup_chroot() { } if [ "start" = "$1" ]; then - shift parse_args "$@" mkdir "$SCRATCH_DIR/conf" "$SCRATCH_DIR/conf/sasl" "$SCRATCH_DIR/data" "$SCRATCH_DIR/queue" || die "could not create $SCRATCH_DIR/{conf,data,queue}" render_template <"$SCRIPT_DIR/main.cf" >"$SCRATCH_DIR/conf/main.cf" || die "could not create $SCRATCH_DIR/conf/main.cf" @@ -156,7 +155,6 @@ if [ "start" = "$1" ]; then fi if [ "stop" = "$1" ]; then - shift parse_args "$@" sudo -n -- postfix -v -c "$SCRATCH_DIR/conf" stop exit 0 diff --git a/integration/mta/script.sh b/integration/mta/script.sh index 51072f9..b5324f2 100644 --- a/integration/mta/script.sh +++ b/integration/mta/script.sh @@ -12,6 +12,7 @@ usage() { } parse_args() { + shift while [ $# -gt 0 ]; do case $1 in -mtaPort) diff --git a/integration/mta/sendmail/mta.sh b/integration/mta/sendmail/mta.sh index 6c2e778..c3e3855 100755 --- a/integration/mta/sendmail/mta.sh +++ b/integration/mta/sendmail/mta.sh @@ -14,24 +14,24 @@ if [ "tags" = "$1" ]; then echo "auth-no" #echo "auth-plain" echo "tls-no" - #echo "tls-starttls" + echo "tls-starttls" exit 0 fi if [ "start" = "$1" ]; then - shift parse_args "$@" + cp "$SCRATCH_DIR/../cert.pem" "$SCRATCH_DIR/cert.pem" || die "could not create $SCRATCH_DIR/cert.pem" + cp "$SCRATCH_DIR/../key.pem" "$SCRATCH_DIR/key.pem" || die "could not create $SCRATCH_DIR/key.pem" render_template <"$SCRIPT_DIR/sendmail.cf" >"$SCRATCH_DIR/sendmail.cf" || die "could not create $SCRATCH_DIR/sendmail.cf" mkdir "${SCRATCH_DIR}/mqueue" || die "could not create $SCRATCH_DIR/mqueue" sudo -n -- chown smmta:smmsp "${SCRATCH_DIR}/mqueue" || die "could not chown $SCRATCH_DIR/mqueue" sudo -n -- chmod u=rwx,g=rs,o= "${SCRATCH_DIR}/mqueue" || die "could not chmod $SCRATCH_DIR/mqueue" - sudo -n -- syslog-ng || die "could not start syslog-ng" + sudo -n -- syslog-ng --no-caps || die "could not start syslog-ng" sudo -n -- /usr/libexec/sendmail/sendmail -bd "-C${SCRATCH_DIR}/sendmail.cf" exit 0 fi if [ "stop" = "$1" ]; then - shift parse_args "$@" # echo "SHUTDOWN" > "${SCRATCH_DIR}/smcontrol" kill "$(head -n1 "${SCRATCH_DIR}/sendmail.pid")" diff --git a/integration/mta/sendmail/sendmail.cf b/integration/mta/sendmail/sendmail.cf index d668d7e..127c716 100644 --- a/integration/mta/sendmail/sendmail.cf +++ b/integration/mta/sendmail/sendmail.cf @@ -101,7 +101,7 @@ Fw/etc/mail/local-host-names %[^\#] # my official domain name # ... define this only if sendmail cannot automatically determine your domain -#Dj$w.Foo.COM +Djmx.example.com # host/domain names ending with a token in class P are canonical CP. @@ -253,7 +253,7 @@ O UseErrorsTo=False #O UseCompressedIPv6Addresses # log level -O LogLevel=9 +O LogLevel=50 # send to me too, even in an alias expansion? O MeToo=True @@ -299,8 +299,8 @@ O MaxRunnersPerQueue=5 # shall we sort the queue by hostname first? #O QueueSortOrder=priority -# increase milter log level (breaks sendmail?) -#O Milter.LogLevel=99 +# increase milter log level +O Milter.LogLevel=50 # minimum time in queue before retry #O MinQueueAge=30m @@ -570,19 +570,19 @@ O ProcessTitlePrefix=MTA # CA directory -#O CACertPath +O CACertPath=/etc/ssl/certs/ # CA file -#O CACertFile +O CACertFile=%{SCRATCH_DIR}/cert.pem # Server Cert -#O ServerCertFile +O ServerCertFile=%{SCRATCH_DIR}/cert.pem # Server private key -#O ServerKeyFile +O ServerKeyFile=%{SCRATCH_DIR}/key.pem # Client Cert #O ClientCertFile # Client private key #O ClientKeyFile # File containing certificate revocation lists -#O CRLFile +O CRLFile=/dev/null # DHParameters (only required if DSA/DH is used) #O DHParameters # Random data source (required for systems without /dev/urandom under OpenSSL) diff --git a/integration/runner/config.go b/integration/runner/config.go index fb14edd..ea3cf97 100644 --- a/integration/runner/config.go +++ b/integration/runner/config.go @@ -49,6 +49,8 @@ func ParseConfig() *Config { flag.UintVar(&milterPort, "milterPort", 34126, "`port` for test milter servers (1024 < port < 65536") filter := "" flag.StringVar(&filter, "filter", "", "regexp `pattern` to filter testcases") + mtaFilter := "" + flag.StringVar(&mtaFilter, "mtaFilter", "", "regexp `pattern` to filter MTAs") flag.Usage = func() { fmt.Fprintf(flag.CommandLine.Output(), "Usage of %s:\n", os.Args[0]) flag.PrintDefaults() @@ -62,6 +64,13 @@ func ParseConfig() *Config { if err != nil { LevelOneLogger.Fatal(err) } + if mtaFilter == "" { + mtaFilter = ".*" + } + mtaFilterRe, err := regexp.Compile(mtaFilter) + if err != nil { + LevelOneLogger.Fatal(err) + } config := Config{ MtaStartPort: uint16(mtaPort), ReceiverPort: uint16(receiverPort), @@ -97,13 +106,19 @@ func ParseConfig() *Config { if mtas == nil { LevelOneLogger.Fatalf("did not find any MTAs") } - if mtaPort+uint(len(mtas)) > math.MaxUint16 { + var filteredMtas []string + for _, m := range mtas { + if mtaFilterRe.MatchString(m) { + filteredMtas = append(filteredMtas, m) + } + } + if mtaPort+uint(len(filteredMtas)) > math.MaxUint16 { LevelOneLogger.Fatal("too many MTAs, pick a lower -mtaPort") } - if overlap(receiverPort, receiverPort, mtaPort, mtaPort+uint(len(mtas))) { + if overlap(receiverPort, receiverPort, mtaPort, mtaPort+uint(len(filteredMtas))) { LevelOneLogger.Fatal("-receiverPort and -mtaPort overlap") } - if overlap(milterPort, milterPort, mtaPort, mtaPort+uint(len(mtas))) { + if overlap(milterPort, milterPort, mtaPort, mtaPort+uint(len(filteredMtas))) { LevelOneLogger.Fatal("-milterPort and -mtaPort overlap") } if overlap(receiverPort, receiverPort, milterPort, milterPort) { @@ -111,7 +126,7 @@ func ParseConfig() *Config { } var dirs []*TestDir var tests []*TestCase - for _, p := range mtas { + for _, p := range filteredMtas { mta, err := NewMTA(p, uint16(mtaPort), &config) mtaPort++ if err != nil { diff --git a/integration/runner/main.go b/integration/runner/main.go index 845a0b0..0055e3f 100644 --- a/integration/runner/main.go +++ b/integration/runner/main.go @@ -18,5 +18,9 @@ func main() { } defer receiver.Cleanup() runner := NewRunner(config, &receiver) - runner.Run() + if !runner.Run() { + receiver.Cleanup() + config.Cleanup() + os.Exit(1) + } } diff --git a/integration/runner/mta.go b/integration/runner/mta.go index 6da20c6..e2ca5f8 100644 --- a/integration/runner/mta.go +++ b/integration/runner/mta.go @@ -65,8 +65,12 @@ func (m *MTA) MarkFailedTest() { } func (m *MTA) Start() error { + err := os.Chmod(m.path, 0755) + if err != nil { + return err + } m.dir = path.Join(m.config.ScratchDir, fmt.Sprintf("mta-%d", m.Port)) - err := os.Mkdir(m.dir, 0755) + err = os.Mkdir(m.dir, 0755) if err != nil && !os.IsExist(err) { return err } @@ -92,13 +96,13 @@ func (m *MTA) Start() error { b, err := m.cmd.CombinedOutput() failed := !IsExpectedExitErr(err) if failed { - LevelTwoLogger.Print(err) + LevelOneLogger.Print(err) } m.m.Lock() failedTest := m.failedTest m.m.Unlock() - if failed || failedTest { - LevelTwoLogger.Printf("MTA %s\n%s", m.path, b) + if failed || failedTest && len(b) > 0 { + LevelOneLogger.Printf("MTA %s output\n%s", m.path, b) } m.wg.Done() cancel() @@ -109,7 +113,7 @@ func (m *MTA) Start() error { m.Stop() return err } - LevelTwoLogger.Printf("MTA %s ready", m.path) + LevelOneLogger.Printf("MTA %s ready", m.path) return nil } @@ -131,8 +135,8 @@ func (m *MTA) Stop() { m.m.Lock() failedTest := m.failedTest m.m.Unlock() - if failedTest { - LevelTwoLogger.Printf("MTA %s stop\n%s", m.path, b) + if failedTest && len(b) > 0 { + LevelOneLogger.Printf("MTA %s stop output\n%s", m.path, b) } }) } diff --git a/integration/runner/runner.go b/integration/runner/runner.go index db1146e..25851dc 100644 --- a/integration/runner/runner.go +++ b/integration/runner/runner.go @@ -1,8 +1,6 @@ package main import ( - "os" - "github.com/d--j/go-milter/integration" ) @@ -18,7 +16,7 @@ func NewRunner(config *Config, receiver *Receiver) *Runner { } } -func (r *Runner) Run() { +func (r *Runner) Run() bool { var prevMta *MTA var prevDir *TestDir defer func() { @@ -39,7 +37,8 @@ func (r *Runner) Run() { LevelOneLogger.Print(dir.MTA) prevMta = dir.MTA if err := dir.MTA.Start(); err != nil { - LevelTwoLogger.Fatal(err) + LevelTwoLogger.Printf("ERR starting MTA %v", err) + return false } } prevDir = dir @@ -53,7 +52,8 @@ func (r *Runner) Run() { } continue } - LevelTwoLogger.Fatal(err) + LevelTwoLogger.Printf("ERR starting milter %v", err) + return false } for _, t := range dir.Tests { i++ @@ -63,13 +63,12 @@ func (r *Runner) Run() { } code, message, step, err := t.Send(t.TestCase.InputSteps, dir.MTA.Port) if err != nil { - prevMta.MarkFailedTest() - prevMta.Stop() - LevelThreeLogger.Fatal(err) + t.MarkFailed("ERR %v", err) + return false } if !t.TestCase.Decision.Compare(code, message, step) { r.receiver.IgnoreMessages() - t.MarkFailed("%03d/%03d NOK DECISION %s != %d %s @%s", i, tests, t.TestCase.Decision, code, message, step) + t.MarkFailed("NOK DECISION %s != %d %s @%s", t.TestCase.Decision, code, message, step) continue } if t.TestCase.ExpectsOutput() { @@ -79,15 +78,15 @@ func (r *Runner) Run() { if !ok { if t.parent.MTA.HasTag("mta-sendmail") { if integration.CompareOutputSendmail(t.TestCase.Output, output) { - t.MarkOk("%03d/%03d OK (sendmail) %s", i, tests, diff) + t.MarkOk("OK (sendmail) %s", diff) continue } } - t.MarkFailed("%03d/%03d NOK OUTPUT %s", i, tests, diff) + t.MarkFailed("NOK OUTPUT %sRECEIVED OUTPUT\n%s", diff, output) continue } } - t.MarkOk("%03d/%03d OK", i, tests) + t.MarkOk("OK") } prevDir.Stop() } @@ -103,7 +102,5 @@ func (r *Runner) Run() { } } LevelOneLogger.Printf("%d tests done: %d OK %d skipped %d failed", len(r.config.Tests), numOk, numSkipped, numFailed) - if numFailed > 0 { - os.Exit(1) - } + return numFailed == 0 } diff --git a/integration/testcase.go b/integration/testcase.go index 20bfcca..dbc28e8 100644 --- a/integration/testcase.go +++ b/integration/testcase.go @@ -131,22 +131,22 @@ func (o *Output) String() string { var b strings.Builder if o.From != nil { b.WriteString("FROM\n") - b.WriteString(fmt.Sprintf("- <%s> %s\n", o.From.Addr, o.From.Arg)) + b.WriteString(fmt.Sprintf("<%s> %s\n", o.From.Addr, o.From.Arg)) } if o.To != nil { b.WriteString("TO\n") for _, t := range o.To { - b.WriteString(fmt.Sprintf("- <%s> %s\n", t.Addr, t.Arg)) + b.WriteString(fmt.Sprintf("<%s> %s\n", t.Addr, t.Arg)) } } if o.Header != nil { b.WriteString("HEADER\n") - b.WriteString(fmt.Sprintf("- %q\n", o.Header)) + b.WriteString(fmt.Sprintf("%q\n", o.Header)) } if o.Body != nil { b.WriteString("BODY\n") - b.WriteString(fmt.Sprintf("- %q\n", o.Body)) + b.WriteString(fmt.Sprintf("%q\n", o.Body)) } return b.String() } diff --git a/integration/tests/body/test.go b/integration/tests/body/test.go index 4931ad4..8bba731 100644 --- a/integration/tests/body/test.go +++ b/integration/tests/body/test.go @@ -1,5 +1,3 @@ -//go:build: auth-no - package main import ( diff --git a/integration/tests/header/test.go b/integration/tests/header/test.go index a4c14ca..0e48b85 100644 --- a/integration/tests/header/test.go +++ b/integration/tests/header/test.go @@ -1,5 +1,3 @@ -//go:build: auth-no - package main import ( diff --git a/integration/tests/mail-from/change.testcase b/integration/tests/mail-from/change.testcase new file mode 100644 index 0000000..ff623a1 --- /dev/null +++ b/integration/tests/mail-from/change.testcase @@ -0,0 +1,3 @@ +FROM +DECISION ACCEPT +FROM * diff --git a/integration/tests/mail-from/test.go b/integration/tests/mail-from/test.go index 8520a66..bcf6a89 100644 --- a/integration/tests/mail-from/test.go +++ b/integration/tests/mail-from/test.go @@ -1,5 +1,3 @@ -//go:build: auth-no - package main import ( @@ -26,6 +24,13 @@ func main() { if trx.MailFrom.Addr == "quarantine@example.com" { return mailfilter.QuarantineResponse("test"), nil } + if trx.MailFrom.Addr == "change@example.com" { + trx.MailFrom.Addr = "another@example.com" + // Sendmail might break when you do not null this out + if trx.MTA.IsSendmail() { + trx.MailFrom.Args = "" + } + } return mailfilter.Accept, nil }, mailfilter.WithDecisionAt(mailfilter.DecisionAtMailFrom)) } diff --git a/integration/tests/rcpt-to/add.testcase b/integration/tests/rcpt-to/add.testcase new file mode 100644 index 0000000..db49663 --- /dev/null +++ b/integration/tests/rcpt-to/add.testcase @@ -0,0 +1,4 @@ +TO +DECISION ACCEPT +TO * +TO * diff --git a/integration/tests/rcpt-to/change.testcase b/integration/tests/rcpt-to/change.testcase new file mode 100644 index 0000000..983e0b0 --- /dev/null +++ b/integration/tests/rcpt-to/change.testcase @@ -0,0 +1,3 @@ +TO +DECISION ACCEPT +TO * diff --git a/integration/tests/rcpt-to/test.go b/integration/tests/rcpt-to/test.go index 4da98ac..fc7f424 100644 --- a/integration/tests/rcpt-to/test.go +++ b/integration/tests/rcpt-to/test.go @@ -1,5 +1,3 @@ -//go:build: auth-no - package main import ( @@ -26,6 +24,14 @@ func main() { if trx.HasRcptTo("quarantine@example.com") { return mailfilter.QuarantineResponse("test"), nil } + if trx.HasRcptTo("add@example.com") { + trx.AddRcptTo("another@example.com", "") + } + if trx.HasRcptTo("change@example.com") { + trx.DelRcptTo("change@example.com") + // Sendmail does not like setting ESMTP args, so we do not set any + trx.AddRcptTo("another@example.com", "") + } return mailfilter.Accept, nil // the decision is done at the DATA command but the mock server sends the DATA response before we can intercept // so the testcases allow the rejection at any step, not only at "DATA".