Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline definitions of modules #42

Closed
foxcpp opened this issue Apr 9, 2019 · 2 comments · Fixed by #55
Closed

Inline definitions of modules #42

foxcpp opened this issue Apr 9, 2019 · 2 comments · Fixed by #55
Assignees
Labels
config Related to configuration language.

Comments

@foxcpp
Copy link
Owner

foxcpp commented Apr 9, 2019

Allow writing module definitions inside other module definitions, omitting names.

This feature would allow writing less verbose configuration files with fewer entities (so you don't have to look around a lot to understand what is happening). The key point is to require "named entities" only when they are shared between server components (modules).

For example, the following configuration:

hostname example.org

sql {
  driver sqlite3
  dsn /var/lib/maddy/all.db
}

smtp smtp://0.0.0.0:25 {
  pipeline incoming_smtp
}
smtp smtp://0.0.0.0:587 {
  pipeline outgoing_smtp
}

# let's pretend that we have two modules for DKIM signing and verification, dkim_sign and dkim_verify
dkim_sign {
  public_key pubkey_file_path
  private_key privkey_file_path
}

pipeline incoming_smtp {
    filter dkim_verify
    filter milter /var/run/spamassasin.sock
    deliver sql 
}

pipeline outgoing_smtp {
    filter dkim_sign
    deliver incoming_smtp local_only  # let's pretend that pipeline implements DeliveryTarget
    deliver queue remote_only
}

queue {
  max_tries 1
  target remote
}

could be written like that:

hostname example.org

sql {
  driver sqlite3
  dsn /var/lib/maddy/msgs.db
}

smtp smtp://0.0.0.0:25 {
  pipeline {
    filter dkim_verify require # refering to singleton module 'dkim_verify'
    filter milter /var/run/spamassasin.sock # refering to singleton module 'milter'
    deliver sql # refering to 'sql' module instance
  }
}

smtp smtp://0.0.0.0:587 {
  auth pam # refering to singleton module 'pam'
  pipeline {
    filter dkim_sign { # inline-defined instance of module dkim_sign
      public_key pubkey_file_path
      private_key privkey_file_path
    }
    deliver queue { # inline-defined instance of module queue
      max_tries 1
      target remote # refering to singleton module 'remote' here
    }
    deliver smtp
  }
}

Generic syntax change is from

{
  operation module_instance_name module_options
}

to

{
  operation module_name {
    module_config_directives
  }
}

Here "inner" pipeline instance gets some artificial name like parentname_N so it can use it as a key to store persistent data.
!!! Reordering of configuration directives here may change "artificial names". Can we do something about it?

!!! We need a different way to specify per-call options
Perhaps

{
  operation module_name {
    module_config_directives
    opts options_go_here
  }
}

Note: a { dir } b c is parsed as

a {
  dir
}
b c

But I'm not very sure about the whole idea. Any thougnts?

@foxcpp foxcpp added design config Related to configuration language. rfc Request For Comments (ongoing discussion / research needed). labels Apr 9, 2019
@foxcpp foxcpp changed the title Proposal: Inline definitions of modules Inline definitions of modules Apr 11, 2019
@emersion
Copy link
Collaborator

Overall LGTM

Reordering of configuration directives here may change "artificial names". Can we do something about it?

I think we shouldn't allow users to refer to inline names.

We need a different way to specify per-call options

That is annoying. Maybe we can require a newline after } if it's not module options?

Do we really need module options btw?

@foxcpp
Copy link
Owner Author

foxcpp commented Apr 11, 2019

I think we shouldn't allow users to refer to inline names.

Well, this is easily done by not adding inline instances to the global registry.
Another problem is module itself using instance name as a key to store persistent data.
Now I'm thinking about prefixing "inline" names with some prefix (so it will be something like inline:parentname_N so the module can detect if it is defined inline and make initialization fail if it needs to store some persistent data.

That is annoying. Maybe we can require a newline after } if it's not module options?

Yeah, fixing parser would be a better approach.

Do we really need module options btw?

Initial idea was to create the ability to tweak module behavior on per-call basis. For example, instead of creating separate rewrite_map module instance for each used mapping file we can just write filter rewrite_map mapping_file and have one global module instance named just rewrite_map.
However... With this "inline" definitions idea we can perhaps just use them:

filter rewrite_map {
  mapping file
}
deliver proxy {
  target lmtp+unix:///var/path/to/lmtp/sock
}

@foxcpp foxcpp removed the rfc Request For Comments (ongoing discussion / research needed). label Apr 14, 2019
@foxcpp foxcpp self-assigned this Apr 27, 2019
@foxcpp foxcpp added this to the 0.1 milestone May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Related to configuration language.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants