Skip to content

Run alternate qmail-remote by setting QMAILREMOTE. - #46

Merged
mbhangui merged 1 commit into
masterfrom
alternate-qmailremote
Aug 27, 2019
Merged

Run alternate qmail-remote by setting QMAILREMOTE.#46
mbhangui merged 1 commit into
masterfrom
alternate-qmailremote

Conversation

@schmonz

@schmonz schmonz commented Jul 24, 2019

Copy link
Copy Markdown
Member

This is like the QMAILQUEUE patch, but for qmail-remote(8).

With this change, on a system wherein qmail-start's environment...

  • has QMAILREMOTE set: it's invoked by qmail-rspawn(8) instead of
    "qmail-remote"

  • has QMAILREMOTE unset: no change in behavior

A QMAILREMOTE program must take care to adhere to the qmail-remote(8)
interface. It can wrap the original, concluding its work by running
qmail-remote, or it can be a full replacement.

Supports goals: "Adding interfaces and seams, where needed, to make extensions possible"
Broaches non-goals: none known
Risks: none known

@schmonz

schmonz commented Jul 24, 2019

Copy link
Copy Markdown
Member Author

This one is intended for our "Introduce new programming interfaces for use by extensions" release. The implementation may change depending on how we implement the "FHS-strict" option of #2 (no problem, that's scheduled for 1.08).

Comment thread qmail-rspawn.c Outdated
Comment thread qmail-rspawn.c Outdated
@mbhangui

Copy link
Copy Markdown
Contributor

I have done something similar in indimail-mta in qmail-lspawn.c and qmail-rspawn.c by setting QMAILLOCAL and QMAILREMOTE. Instead of calling qmail-local / qmail-remote, I call a program called spawn-filter which can run user defined filters and the output of that gets passed to qmail-local, qmail-remote. @schmonz, take a look at qmail-rspawn.c, qmail-lspawn.c and spawn-filter.c. I use it mainly for DKIM.
https://github.com/mbhangui/indimail-mta/tree/master/indimail-mta-2.8

@schmonz

schmonz commented Jul 26, 2019

Copy link
Copy Markdown
Member Author

@mbhangui, that sounds more generalized than what I've done here, and might be preferable. Would you be willing to submit a PR with it? No hurry whatsoever, this would be for a couple releases after 1.07.

@mbhangui

mbhangui commented Jul 26, 2019

Copy link
Copy Markdown
Contributor

@mbhangui, that sounds more generalized than what I've done here, and might be preferable. Would you be willing to submit a PR with it? No hurry whatsoever, this would be for a couple releases after 1.07.

Sure. I will also make the change that @DerDakon suggested and remove all indimail related changes. The change will add a control file named /var/qmail/control/filterargs where one can specify filters
The format of this file is of the form domain:args for both local and remote mails. domain:remote:args for remote mails and domain:local:args for local mails. Here domain refers to the recipient domain. If there are multiple lines which match domain, the first line that matches domain will be used. e.g. a line in the control file filterargs

indimail.org:remote:/usr/bin/dk-filter

will cause all emails sent to indimail.org, get filtered through the the program /usr/bin/dk-filter. The program /usr/bin/dk-filter will just take the original email on stdin and print the email on stdout with the DKIM-Signature added. This output will be sent to qmail-remote.

@DerDakon

Copy link
Copy Markdown
Member

If this becomes code like this:

const char *qrprogram()
{
  static const char *qrprog;
  if (qrprog != NULL)
    return qrprog;
 …
}

then it does not need to be in main as it will be only on if call per scheduled mail, regardless which binary will finally be called.

@schmonz schmonz added the enhancement New feature or request label Jul 29, 2019
@mbhangui

Copy link
Copy Markdown
Contributor

@schmonz I have done the changes. spawn-filter.c and wildmat.c will require a good review. How do I put it here? Do I use this same branch alternate-qmailremote and add changes (this is what I have done now, but I have not committed or pushed the changes)
or
do I create a new pull request after your changes are merged?

Following will be the changes

  1. No change to qmail-rspawn.c. Your change is perfect and it assigns qrargs[0] just once
  2. Same change added to qmail-lspawn.c using env variable QMAILLOCAL
  3. Modification of man pages qmail-rspawn.8, qmail-lspawn.8 for QMAILREMOTE, QMAILLOCAL
  4. Modification of man page for qmail-control (for new control file filterargs)
  5. addition of a new program spawn-filter (which can be called by setting QMAILREMOTE or QMAILLOCAL
  6. Addition of a C file wildmat.c for wildmat_internal() function
  7. modification of Makefile, TARGETS
  8. modification of hier.c for installing spawn-filter and spawn-filter man page

@schmonz

schmonz commented Jul 29, 2019

Copy link
Copy Markdown
Member Author

@mbhangui, I think we'll want to be able to look at all our options for how to solve this problem. (We did this for DESTDIR. One PR got merged, and the rest closed.) So please create a new PR on a new branch, with the name not being terribly important because it won't stick around forever. When I see the PR land, I'll reference it from this one. Thanks!

Update: @mbhangui's approach is in #50

@DerDakon
DerDakon requested a review from mbhangui August 26, 2019 18:49
This is like the QMAILQUEUE patch, but for qmail-remote(8).

With this change, on a system wherein qmail-start's environment...

- has QMAILREMOTE set: it's invoked by qmail-rspawn(8) instead of
  "qmail-remote"

- has QMAILREMOTE unset: no change in behavior

A QMAILREMOTE program must take care to adhere to the qmail-remote(8)
interface. It can wrap the original, concluding its work by running
qmail-remote, or it can be a full replacement.

Supports goals: "Adding interfaces and seams, where needed, to make extensions possible"
Broaches non-goals: none known
Risks: none known
@schmonz
schmonz force-pushed the alternate-qmailremote branch from e751849 to 175529d Compare August 26, 2019 19:02
@mbhangui

Copy link
Copy Markdown
Contributor

Should I click on the "Rebase and Merge". The branch has no conflicts.

@schmonz

schmonz commented Aug 27, 2019

Copy link
Copy Markdown
Member Author

That would work, but I'd like to see some comparative opinions on your #50 before we decide what to merge for 1.08. :-)

@mbhangui

mbhangui commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

That would work, but I'd like to see some comparative opinions on your #50 before we decide what to merge for 1.08. :-)
The difference are the following

  1. This adds spawn-filter binary for filtering message passed to qmail-remote/qmail-local #50 can run alternate qmail-remote as well as alternate qmail-local (QMAILREMOTE, QMAILLOCAL env variables)
  2. It has an additional binary spawn-filter which can run either qmail-local or qmail-remote before calling the actual qmail-remote / qmail-local
  3. Additional complexity by introducing a new binary named spawn-filter

@schmonz

schmonz commented Aug 27, 2019

Copy link
Copy Markdown
Member Author

If you agree #50 is better for later (perhaps 1.9), then I'm happy having you merge this one!

@mbhangui

Copy link
Copy Markdown
Contributor

We can consider #50 after spawn-filter.c is thouroughly reviewed. Merging #46 for now.

@mbhangui
mbhangui merged commit 143784b into master Aug 27, 2019
@schmonz
schmonz deleted the alternate-qmailremote branch August 27, 2019 04:21
@DerDakon DerDakon modified the milestones: 1.09, 1.08 Dec 28, 2019
Comment thread qmail-rspawn.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants