canonical map doesn't accept underscores in destination address

gitwart commented 1 year ago

Affected Puppet, Ruby, OS and module versions/distributions

Puppet: puppet-agent-7.19.0-1.el7
Ruby:
Distribution: Scientific Linux 7, Rocky Linux 8
Module version: 3.0.0

How to reproduce (e.g Puppet code you use)

        class {'::postfix':
            inet_interfaces     => 'loopback-only',
        }
        postfix::hash {'/etc/postfix/canonical':
            ensure => 'present',
        }   
        postfix::config { "canonical_maps":
            value => "hash:/etc/postfix/canonical"
        }   
        postfix::canonical { 'root':
            ensure      => 'present',
            destination => 'sample_user@example.com',
        }

What are you seeing

The puppet agent generates an error:

Error: /Stage[main]/Profile::Smtpclient/Postfix::Canonical[root]/Augeas[Postfix 
canonical - root]: Could not evaluate: Save failed, see debug output for details

The debug output from the puppet agent:

Debug: Augeas[Postfix canonical - root](provider=augeas): Put failed on one or more files, output from /augeas//error:
Debug: Augeas[Postfix canonical - root](provider=augeas): /augeas/files/etc/postfix/canonical/error = put_failed
Debug: Augeas[Postfix canonical - root](provider=augeas): /augeas/files/etc/postfix/canonical/error/path = /files/etc/postfix/canonical/files/etc/postfix/canonical/pattern[2]
Debug: Augeas[Postfix canonical - root](provider=augeas): /augeas/files/etc/postfix/canonical/error/lens = /opt/puppetlabs/puppet/share/augeas/lenses/postfix_canonical.aug:44.15-46.15:
Debug: Augeas[Postfix canonical - root](provider=augeas): /augeas/files/etc/postfix/canonical/error/message = Failed to match tree under /files/etc/postfix/canonical/pattern[2]

     { "destination" = "sample_user@example.com" }

  with pattern

    { /destination/ = /[*+.0-9@-Z\\a-z-]+/ }

What behaviour did you expect instead

The address mapping is added to the canonical file

Output log

Any additional information you'd like to impart

Removing the underscore from the destination address (sampleuser@example.com) makes the error go away. Adding an underscore to the word definition in postfixcanonical.aug line 35 seems to fix the problem: `let word = store /[A-Za-z0-9@*.+-]+/`

ekohl commented 7 months ago

Interesting that it appeared to at least load the lens for you. I needed 194e18dcab32d2f122dda2bf43db1a6a62a655bd to load it.

Removing the underscore from the destination address (sampleuser@example.com) makes the error go away. Adding an underscore to the word definition in postfixcanonical.aug line 35 seems to fix the problem: `let word = store /[A-Za-z0-9@*.+-]+/`

Can you submit a patch for that? Ideally with tests.

giacomd commented 5 months ago

Hi, from RFC 3696 (informational only but referring to RFC 2821 and RFC 2822):

   Without quotes, local-parts may consist of any combination of
   alphabetic characters, digits, or any of the special characters

      ! # $ % & ' * + - / = ?  ^ _ ` . { | } ~

   period (".") may also appear, but may not be used to start or end the
   local part, nor may two or more consecutive periods appear.  Stated
   differently, any ASCII graphic (printing) character other than the
   at-sign ("@"), backslash, double quote, comma, or square brackets may
   appear without quoting.  If any of that list of excluded characters
   are to appear, they must be quoted.

So it looks like the regex should be made much more complex in order to support all possible local parts.. On my side I am affected by the lack of the underscore as well, and I'm willing to contribute a patch/test. @ekohl what do you think would be the best approach? I feel like a complete rewrite of that lens would be overkill.

ekohl commented 5 months ago

Sounds like you should submit a patch to add at least _ here: https://github.com/voxpupuli/puppet-postfix/blob/2c326befd68bcd1c36a9f6a6ce68d22d6976b483/lib/augeas/lenses/postfix_canonical.aug#L35

But I'll be honest and say that I know little about Augeas lenses.

giacomd commented 5 months ago

OK cool - I'll work on a patch then.

smortex commented 5 months ago

I guess we can also tolerate quoted addresses too, and matching correctness this with a regular expression is probably not possible. So I would not mind if you can pass invalid addresses that are accepted by augeas but break postfix later (I did not checked but I suspect it is already the case), that seems better than rejecting valid addresses that should work.

Maybe matching non-blank for word is enough? Adding "weird" addresses into spec/defines/postfix_canonical_spec.rb should show if it behaves as expected.

voxpupuli / puppet-postfix