Problems invoking amavis from postfix

[Robert Moskowitz]

On 2/8/19 2:08 PM, Wietse Venema wrote:

> Robert Moskowitz:
>>
>> On 2/8/19 1:44 PM, Wietse Venema wrote:
>>> Viktor Dukhovni:
>>>>> pickup         unix   n             -             n             60           1             pickup
>>>>> pickup         unix   n             -             n             60           1             pickup
>>>>>           -o content_filter=
>>>> The "pickup" service is defined twice in master.c, the second
>>>> instance (last one wins) disables content filtering for mail submitted
>>>> locally via sendmail(1).
>>> That was easy enough to fix:
>> When I was working on this 2 years ago, I thought it was kind of cool
>> that instead of editing master.cf entries to fix them, I could just
>> append a whole new entry with the 'right' content.
>>
>> Much easier to automate changes (as we had nothing like postconf -e for
>> changing master.cf).? If I read the patch right, you are providing a
>> warning of the double entry.? Perhaps a better patch would warn and drop
>> all but the last entry?
> Why do you think it was keeping both pickup entries?

Well, I am not sure.  From Viktor's earlier note, it seems that the last
wins and the earlier ones are just ignored.  Maybe it is that Viktor
said, "master.c" and I don't know what "master.c" different from
"master.cf" that is in /etc/postfix.

I kind of assumed (and we know what that is an abbreviation for) that
"master.c" is an internal entry in postfix built from processing
master.cf.  Thus why keep all but the last in the internal table?

[Viktor Dukhovni]

> On Feb 8, 2019, at 2:15 PM, Robert Moskowitz <[hidden email]> wrote:
>
> Well, I am not sure.  From Viktor's earlier note, it seems that the last wins and the earlier ones are just ignored.  Maybe it is that Viktor said, "master.c"

A simple typo.  I meant master.cf.

--
        Viktor.

[Robert Moskowitz]

On 2/8/19 2:10 PM, Viktor Dukhovni wrote:
>> On Feb 8, 2019, at 2:07 PM, Robert Moskowitz <[hidden email]> wrote:
>>
>> Much easier to automate changes (as we had nothing like postconf -e for changing master.cf).  If I read the patch right, you are providing a warning of the double entry.  Perhaps a better patch would warn and drop all but the last entry?
> It is not the job of master(8) to edit master.cf.  Indeed that file
> might reside in read-only storage.
>
> If you meant "use only the last one", as Wietse also notes, that's the
> current behaviour.
>
Ah, so it is my muddled reading.

I did not think that postfix should edit master.cf.  Only its internal
processes would use the last entry found.

[Wietse Venema]

Robert Moskowitz:

>
>
> On 2/8/19 2:10 PM, Viktor Dukhovni wrote:
> >> On Feb 8, 2019, at 2:07 PM, Robert Moskowitz <[hidden email]> wrote:
> >>
> >> Much easier to automate changes (as we had nothing like postconf -e for changing master.cf).  If I read the patch right, you are providing a warning of the double entry.  Perhaps a better patch would warn and drop all but the last entry?
> > It is not the job of master(8) to edit master.cf.  Indeed that file
> > might reside in read-only storage.
> >
> > If you meant "use only the last one", as Wietse also notes, that's the
> > current behaviour.
> >
> Ah, so it is my muddled reading.
>
> I did not think that postfix should edit master.cf. Only its internal
> processes would use the last entry found.

To make this abundantly clear, adding this warning does not change program behavior.

        Wietse

[Robert Moskowitz]

On 2/8/19 2:31 PM, Wietse Venema wrote:

> Robert Moskowitz:
>>
>> On 2/8/19 2:10 PM, Viktor Dukhovni wrote:
>>>> On Feb 8, 2019, at 2:07 PM, Robert Moskowitz <[hidden email]> wrote:
>>>>
>>>> Much easier to automate changes (as we had nothing like postconf -e for changing master.cf).  If I read the patch right, you are providing a warning of the double entry.  Perhaps a better patch would warn and drop all but the last entry?
>>> It is not the job of master(8) to edit master.cf.  Indeed that file
>>> might reside in read-only storage.
>>>
>>> If you meant "use only the last one", as Wietse also notes, that's the
>>> current behaviour.
>>>
>> Ah, so it is my muddled reading.
>>
>> I did not think that postfix should edit master.cf. Only its internal
>> processes would use the last entry found.
> To make this abundantly clear, adding this warning does not change program behavior.

I did see that, and because i assumed Viktor's typo (and I am a master
at making typos) was a 'new' insight (for me) into postfix that there
was something more at work here.

Got it all now.

I hope.

:)

[Wietse Venema]

Wietse Venema:

> Robert Moskowitz:
> >
> >
> > On 2/8/19 2:10 PM, Viktor Dukhovni wrote:
> > >> On Feb 8, 2019, at 2:07 PM, Robert Moskowitz <[hidden email]> wrote:
> > >>
> > >> Much easier to automate changes (as we had nothing like postconf -e for changing master.cf).  If I read the patch right, you are providing a warning of the double entry.  Perhaps a better patch would warn and drop all but the last entry?
> > > It is not the job of master(8) to edit master.cf.  Indeed that file
> > > might reside in read-only storage.
> > >
> > > If you meant "use only the last one", as Wietse also notes, that's the
> > > current behaviour.
> > >
> > Ah, so it is my muddled reading.
> >
> > I did not think that postfix should edit master.cf. Only its internal
> > processes would use the last entry found.
>
> To make this abundantly clear, adding this warning does not change program behavior.

Well it is not supposed to. This is a revised version of the patch,

        Wietse

diff -ur src/master/master_conf.c- src/master/master_conf.c
--- src/master/master_conf.c- 2011-09-07 12:58:45.000000000 -0400
+++ src/master/master_conf.c 2019-02-08 17:30:15.000000000 -0500
@@ -123,7 +123,11 @@
  * settings.
  */
  else {
-    serv->flags &= ~MASTER_FLAG_MARK;
+    if ((serv->flags & MASTER_FLAG_MARK) == 0)
+ msg_warn("duplicate master.cf entry for service \"%s\" (%s) "
+     "-- using the last entry", serv->ext_name, serv->name);
+    else
+ serv->flags &= ~MASTER_FLAG_MARK;
     if (entry->flags & MASTER_FLAG_CONDWAKE)
  serv->flags |= MASTER_FLAG_CONDWAKE;
     else