Opened 9 years ago

Closed 9 years ago

#810 closed enhancement (complete)

TSIG: Create non-module config location support

Reported by: stephen Owned by: vorner
Priority: very high Milestone: Sprint-20110419
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

We need to have a place to put configuration elements that is not associated with a running module. A problem with this is that our configuration system runs code to check the validity of each change. We may perhaps use a simple check for this now, rather than run code - for example a regular expression to confirm that the TSIG key looks valid.

Subtickets

Change History (13)

comment:1 Changed 9 years ago by shane

  • Estimated Difficulty changed from 0.0 to 2
  • Milestone changed from Year 3 Task Backlog to Sprint-20110419
  • Priority changed from major to critical

comment:2 Changed 9 years ago by shane

  • Priority changed from critical to blocker

comment:3 Changed 9 years ago by vorner

  • Owner set to vorner
  • Status changed from new to accepted

comment:4 Changed 9 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from accepted to assigned

Hello

I thought it is easier to just load a bit of python code to check the config than to implement another ad-hoc syntactic check loaded from spec file (which spec file, anyway?). So I created somehow minimalistic plugin loader.

It loads whatever python files are stored in $prefix/share/$package/config_plugins and calls their load() function. It's result should be a double (specObject, checkingFunction). The spec object is the usual spec that arrives by the wire from modules, checking functions is called to validate all the modifications.

There are two things I'm unsure:

  • Where should we store the plugins in our source code? Under the bin/cfgmgr/plugins? And is there a way to get this directory from the build system somehow from python script? Like when the B10_FROM_BUILD is set, we should be able to find them as well.
  • What a module does when it wants to read another module's configuration? It subscribes to the messages of the module and handle its config changes, not responding to it (eg. silently spying it)? If so, I need to send the message (but not expect answer) even when the module is validated by plugin. And what would happen if, for example, we had two auth modules running? If something else, what happens to it?

As for changelog:

[func]     vorner
It is possible to add modules to configuration that doesn't correspond
to running process. It is handled by a python plugin that is loaded
at startup. For more detail, you can have a look at test plugin in
src/bin/cfgmgr/tests/testdata/plugins.

Thanks

comment:5 Changed 9 years ago by vorner

  • Status changed from assigned to reviewing

comment:6 Changed 9 years ago by jelte

BTW, I think we should consider moving all .spec files out of their modules directory, and put them all together in one dir in the source tree (similar to how they end up when installed), cfgmgr could then load them all at once (we also need something like that for 'offline' configuration). This could then also be the place for specs that have no actual module associated with them. Perhaps the specfile itself could then specify whether there is a module they belong to, it has some loadable dynamic checking code, or no checking apart from syntax at all.

Right now, modules can 'spy' on other module's configs with the 'remote config' set of functions; these are 'secretly' updated when configuration changes occur, and modules that listen to the configs of others don't react to them. (Due to the current implementation, they also get no module-level signal when it changes; they should access the config data directly, and not copy it for efficiency; the underlying config values are silently updated)

comment:7 Changed 9 years ago by vorner

As clarified on Jabber, the answer to second question is „yes, the signal is needed to propagate the change to spying modules“. So, I'll update the code soon to include it.

comment:8 Changed 9 years ago by vorner

OK, updated.

comment:9 Changed 9 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:10 follow-up: Changed 9 years ago by stephen

  • Owner changed from stephen to vorner

src/bin/cfgmgr/b10-cfgmsg.py.in

PLUGIN_PATH = [] #FIXME Where should plugins come from? And how to find it?

I suggest $B10_FROM_BUILD/src/bin/cfgmgr/plugins

src/lib/python/isc/config/cfgmgr.py
Comment: rather than use an array of virtual modules and an array of real modules, and branching on which one contains the module being considered, can't we encapsulate a module in a class and use different derived classes for "real" and "virtual" modules? It would aid future extensibility.

Some documentation on how to create a plugin would be helpful - either on the wiki or as a README in the configuration manager directory. The comments in "testplugin.py" are a bit sparse, and it is not the most obvious place for documentation.

comment:11 in reply to: ↑ 10 Changed 9 years ago by vorner

  • Owner changed from vorner to stephen

Hello

Replying to stephen:

src/lib/python/isc/config/cfgmgr.py
Comment: rather than use an array of virtual modules and an array of real modules, and branching on which one contains the module being considered, can't we encapsulate a module in a class and use different derived classes for "real" and "virtual" modules? It would aid future extensibility.

Actually, that would be little harder, than it seems. For one, there's no array (well, they are dicts, but its not important) with real modules. There's one dict where the configuration data (of all modules) live. They are, however, simple python structures (dicts and lists), not classes. So changing these would require quite deep refactoring of the whole config manager, which I don't really like.

Then there's the dict with virtual modules. Yes, I probably could say that every module is equivalent to current virtual and place a function to post the config over the msgq there. But the posting has slightly different „natural“ interface, than the function and would be playing with words mostly (and the difficulty of distinguishing between real and virtual modules would just be placed somewhere else).

And anyway, I don't see the extensibility it would bring. The virtual module contains a callable object, so any behaviour can be placed there if needed in future. And the I think we usually do as little work as possible, until the extensibility is needed. In the case it is needed, it can be changed then? But I simply think it doesn't fit into current design without a rework and the rework would make it only needlessly complicated.

Anyway, I think the config manager has some incorrect behaviour now (in the case one module rejects the configuration, odd things can happen). When these are solved, I could imagine placing something more generic there would be much easier. So, would you agree with a # TODO comment there, explaining the problems and possible solution?

Some documentation on how to create a plugin would be helpful - either on the wiki or as a README in the configuration manager directory. The comments in "testplugin.py" are a bit sparse, and it is not the most obvious place for documentation.

I placed a README into the plugins directory, with small example. Anyway, I hope we'll create a configuration plugin soon, so we would have a real example as well.

comment:12 Changed 9 years ago by stephen

  • Owner changed from stephen to vorner

Actually, that would be little harder, than it seems ...

Fair enough, it was just a thought. There is no need to do needless work.

So, would you agree with a # TODO comment there, explaining the problems and possible solution?

Yes.

I placed a README into the plugins directory...

I suggest adding the file to the EXTRA_DIST line in the Makefile as well.

I don't need to see the code again, please merge when you made the changes outlined above.

comment:13 Changed 9 years ago by vorner

  • Resolution set to complete
  • Status changed from reviewing to closed

Ok, thanks.

Note: See TracTickets for help on using tickets.