2014-03-25 18:14 GMT+01:00 Stephen Gallagher <
sgallagh@redhat.com>:
> On 03/24/2014 05:02 PM, Miloslav Trmač wrote:
>> 2014-03-19 14:47 GMT+01:00 Stephen Gallagher <
sgallagh@redhat.com
>> Wouldn't it be cleaner to have generic ValidateSettings(settings)
>> DeployRole(settings) as the abstraction used by the "universal"
>> clients (with "settings" possibly having some universal settings,
>> like our favorite firewall_enabled_after_deploy, but mostly
>> role-specific, only using a standard format, perhaps JSON or some
>> object graph with similar capabilities)?
>
> That's an idea. I'm not sure it's doing anything other than moving the
> problem to the settings object instead of the method call, but *shrug*.
Right, it would be even better to provide role-specific APIs (or a generic API with schema enforcement) to change values within the settings object :) in order to make typos in settings names obvious, and that point it is really similar.
It does only one structurally significant thing: encode the "settings format" (JSON/whatever) as a canonical way to describe/dump/persistently store role deployment configuration, giving the API users precisely the same facilities they have during installation.
>> * methods: * GetFirewallPorts: list of port/protocol pairs that the
>> Role needs
>>
>>
>> I'd much rather have FirewallRestriction enum (LocalhostOnly,
>> Unrestricted, Complex) or something like that, or, if we decided
>> to depend on firewalld, an object reference to a firewalld service.
>> Roles will be asking for ICMP, new IP protocols, and whatnot, and
>> we don't need to start with that complexity in the API and the
>> clients.
>
> Sorry, that was a little thin. Could you go into a bit more detail
> here, please?
Stepping back a little, the role
implementation and
external API should be different. The implementation obviously does need to actually contain port numbers and the like somewhere, but I don't think we want them in the role's public API.
The reason is that this introduces too much complexity into the API, at a level that isn't interesting to many important users of the API. The complexity is in all the things a role might need (TCP ports, UDP ports, IP protocols, specific multicast addresses, perhaps even conntrack modules), and the presumed callers of a "Role deploy API" just don't care, they want the role deployed firewalled/not firewalled, and a service dashboard needs to to know whether it has been firewalled. Receiving a list of half-a-dozen different individual firewall concepts to check isn't all that helpful, the public API should be able to give a simple yes/no answer (well, a "yes/no/"it's complicated" answer, which are the badly-explained LocalhostOnly/Unrestricted/Complex values above).