inkblot / puppet-bind

18 stars 82 forks source link

Allow Puppet to manage zone files centrally #32

Closed Zetten closed 9 years ago

Zetten commented 9 years ago

Add parameter to zone type which allows Puppet to manage the content of zone database files (not just ensure their existence).

inkblot commented 9 years ago

I have also been thinking of adding similar functionality. I think that there is a more profound distinction between zones for which $manage_file is true and those for which it is false. When it is false, the zone is intended, I think, to be updated dynamically via DDNS. When it is true the intent is clearly to have a copy of the zone file which is manually edited and considered authoritative. This is all very related to the existing $allow_updates parameter, which only makes sense for dynamic zones.

The state implied by the defined type as currently implemented equivalent to your $manage_file parameter having a value of false. The more salient feature in this state is that the zone is dynamic. Furthermore, the presence or absence (emptiness: '') of could substitute for an explicit parameter like $manage_file. This would of course require documentation.

What do you think of using the default empty value of $allow_updates vs. a supplied value to indicate the state communicated through $manage_file and dropping the explicit $manage_file parameter?

Zetten commented 9 years ago

When [$manage_file] is false, the zone is intended, I think, to be updated dynamically via DDNS.

There's unfortunately a third possibility: where the file is not managed, and not updated via DDNS, but still guaranteed to exist. Since it's not possible to create a bind::zone without the file being created this is technically possible now, and I was trying to avoid breaking such a configuration.

I think this is quite unlikely, but I wouldn't gamble against someone out there having a workflow which requires the initial provisioning of the zone file but where future updates are performed manually on the node.

But the principle is better than my quick hack, definitely. Maybe the best option is that $allow_updates should be a tri-state variable with possible values of:

Is that what you were thinking of?

inkblot commented 9 years ago

The $allow_updates parameter can't have those values. It is currently used in zone.conf.erb to populate an allow-updates clause in the BIND configuration:

<%- if @allow_updates and @allow_updates != '' -%>
        allow-update {
<%-   Array(@allow_updates).each do |allow_update| -%>
                <%= allow_update %>;
<%-   end -%>
        };
<%- end -%>

I understand what you mean though about there being three states. slave zones are a good example where currently puppet initializes the zone file with one that's /almost/ guaranteed to be replaced by the nameserver, but would do better to just ignore the file when it appears but do nothing to create it.

inkblot commented 9 years ago

Now that I've had a minute to think about it, I think there are at least four different ways Puppet could treat the zone file.

I'm not entirely sure, but I think all of the different modes that apply to master zones also apply in the same way to hint zones, but I don't have as much experience with these.

inkblot commented 9 years ago

I am inspired to take a crack at this four-mode approach myself.

Zetten commented 9 years ago

Yes! Sorry, I got sidetracked on some other things and haven't had a chance to come back to this yet. I think your four approaches cover anything, but I'm not sure about hint zones - I can handle basic bind config but I'm far from an expert.

If you suggest a good approach for the implementation I don't mind having a go myself, but I suspect your puppet skills are more capable than mine.

Personally I would probably go for a more verbose explicit parameter to control which of the four modes a particular zone uses (or even discrete puppet types for different zone management options, e.g. bind::zone::forward, bind::zone::dynamic_master, bind::zone::managed_master, bind::zone::slave), and then validation to ensure correct additional parameters are set. But a dynamic method which infers the action to take based on which parameters are set would perhaps be more 'puppet-y'.

In any case, I did think of at least one extra consideration for the 'managed master' option: performing rndc reload for the zone when the file is updated would be really useful. Perhaps an additional verification of a managed file with named-checkzone would be useful too, but again I'm not sure how 'puppet-y' that is.

Zetten commented 9 years ago

Fulfilled by #36