Skip to content

Instantly share code, notes, and snippets.

@mattias-p
Last active March 8, 2017 10:06
Show Gist options
  • Save mattias-p/118af12700db6ac2ae58cf57a7fa3780 to your computer and use it in GitHub Desktop.
Save mattias-p/118af12700db6ac2ae58cf57a7fa3780 to your computer and use it in GitHub Desktop.

Approach

Should the CLI and the Backend (and possibly other applications using the Engine) have a common idea of which profile file is represented by a profile name?

  • Yes. It's helpful for different applications to agree on the meaning of different profile names.

    In this case, the resolution of profile names into profile files should be performed in the Engine. The resolution must be performed without any additional configuration details.

    (It is technically possible to let each application provide the Engine with configuration details programmatically, and to make the applications (and possibly the administrator) responsible, contractually, for making sure the provided settings are the same across applications. However, I believe this theoretical possibility to be a bad idea in practice.)

  • No. Different applications have no use for a common concept of profile names.

    In this case, each component should implement its own way to locate profile files. Any configuration settings required to locate profile files should be read from the application configuration file.

Based on the assumption that we agree on the answer to the above question being "yes" and that given the constraints are appropriate, I'm proposing a design.

Proposed design

The engine should have separate methods for resolving a profile name and for loading a profile file. The names lookup() and load() are used to refer to these methods below.

The load() method takes a path as an argument, and overrides previous settings according to the profile file at the given path, or dies if there is a problem reading or interpreting the profile file.

The lookup() method takes a profile name argument, and returns an absolute path to a profile file, or dies if there is no matching profile file.

For a profile name to match a profile file it must first be valid accordnig to the regular expression [0-9A-Za-z][0-9A-Za-z-]{1,198}[0-9A-Za-z].

A basename is constructed by concatenating "profile_" . profile name . ".json" (e.g. the profile name "foobar" is mapped to the basename "profile_foobar.json".) The following tests are made in order.

  1. -f "/usr/local/etc/zonemaster/$basename"
  2. -f "/etc/zonemaster/$basename"
  3. File::ShareDir::dist_file('Zonemaster', $basename)

The lookup() method returns the path of the first hit (as checked by -f or returned from dist_file()), and no further tests are made.

A profile named "default" is loaded automatically (using load()) by the engine configuration module as an initial state for the profile settings. Additional calls to load() (by the application) simply override the previous settings. A profile file with the name "default" is installed together with the module.

An administrator can add a new profile or override the "default" profile by putting an appropriately named profile file in either of the system configuration directories (i.e. "/usr/local/etc/zonemaster" or "/etc/zonemaster".)

This allows:

  • profile names to be independent of file extensions.
  • future extensions of the engine with additional configuration file types without risk of collisions.
  • a default profile to be installed together with the Engine module.
  • additional profiles can be added, and the default profile can be overridden by an administrator using conventional application configuration directories for both GNU/Linux and FreeBSD.
  • ergonomic profile management without the risk of exposing/accessing unexpected files.
  • identical resolution across different users on the same system.
  • the default profile to be used by other tools than the Config module itself (e.g. a migration script needed in a future upgrade)

Additional thoughts

I think the CLI should be able to load profiles using both profile names and absolute paths to profile files using separate --profile-name and --profile-file options.

@sandoche2k
Copy link

The load() method takes a path as an argument, and overrides previous settings according to the profile file at the given path, or dies if there is a problem reading or interpreting the profile file.

The lookup() method takes a profile name argument, and returns an absolute path to a profile file, or dies if there is no matching profile file

Shouldn't it be like this:

  1. The lookup() resolves to a filename and its relative path.
  2. If the lookup() returns a value, then load that file with the relative path
  3. If the lookup() does not return a value then load the default profile

@matsduf
Copy link

matsduf commented Mar 7, 2017

@mattias-p, since profile "default" is special in your proposal, then it should be clarified what will happen if there is profile_default.jsonin more than one of the profile directories.

@sandoche2k wrote:

There should be only one option via the CLI. That is the "--profile" option. The file could be accessible as follows:

zonemaster_cli --profile iana_profile.json afnic.fr
zonemaster_cli --profile /home/sandoche/iana_profile.json afnic.fr

If we have only one switch --profile then we should make a separation of file name and profile name so that if the value matches a profile name it is treated as such, else as a file name.

E.g. profile name (profile "iana", loading file "profile_iana.json"):

zonemaster_cli --profile iana afnic.fr

E.g. file name (profile file "iana_profile.json" in current directory):

zonemaster_cli --profile iana_profile.json afnic.fr

E.g. file name (profile file with full path):

zonemaster_cli --profile /home/sandoche/iana_profile.json afnic.fr

@sandoche2k wrote:

Shouldn't it be like this:

  1. The lookup() resolves to a filename and its relative path.
  2. If the lookup() returns a value, then load that file with the relative path
  3. If the lookup() does not return a value then load the default profile

I do not agree. If the requested profile is not found then it should result in an error. Else we think we are using one profile (maybe misspelled) but we are testing another. According to the proposal, the default is loaded first, an any other profile on-top.

@mattias-p
Copy link
Author

@matsduf I've made clarifications to the default profile. Can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment