refactor hostkeys and knownhosts classes by moving and exposing their parameters in the relevant parents #412
Conversation
- moved parameters from hostkeys to server - removed creation knownhosts from server class
| Boolean $export_ipaddresses = true, | ||
| Optional[String[1]] $storeconfigs_group = undef, | ||
| Array $extra_aliases = [], | ||
| Array $exclude_interfaces = [], | ||
| Array $exclude_interfaces_re = [], | ||
| Array $exclude_ipaddresses = [], | ||
| Boolean $use_trusted_facts = false, | ||
| Optional[Array[String[1]]] $tags = undef, |
There was a problem hiding this comment.
This is a backwards-incompatible change and I don't really like it. Maybe we can just keep the parameters on ssh::hostkeys and use parameters on ssh::server as default? This way, any value set via e.g. hiera will be working as expected.
There was a problem hiding this comment.
Hi Steffen,
I didn't think about the breaking change side of it as the hostkeys code almost seemed like it was really heading towards being a private class. I'll put it back more like it was as breaking what is working for people isn't great.
There was a problem hiding this comment.
OK. Set it up as you described to keep backward compatibility.
|
|
||
| Class['ssh::server::install'] | ||
| -> Class['ssh::server::config'] | ||
| ~> Class['ssh::server::service'] | ||
| -> Class['ssh::hostkeys'] | ||
| -> Class['ssh::knownhosts'] |
There was a problem hiding this comment.
I think there's a chance, that someone's making use of this by not managing the client, but still making use of the collected known hosts. Except for getting rid of the class, is there any disadvantage which gets solved?
There was a problem hiding this comment.
Hi Steffen,
Thanks for the feedback. Its really about "knownhosts" being about the client side of ssh, its never a server bit of functionality. Unless you go to some trouble, with this in the class, you can't avoid having knownhosts being updated. If you have fleets of systems where you don't want the hosts, IPs and keys being shared between them, this is going to put them there by default. The (very welcome) addition of the filtering of keys is a partial answer to this problem, but in many cases it can be avoided completely by not insisting on writing knownhosts files on every server (who normally won't use them) which also produces a lot of noise in large fleets.
There was a problem hiding this comment.
@stuartrobert that's a good point. I currently use this module to distribute knownhosts to my entire fleet, but I would welcome the ability to limit knownhosts to clients only, in order to reduce the number of changes puppet is making.
There was a problem hiding this comment.
I looked at this again trying to think how to add this functionality with a view to making the server side collection disabled by default, but didn't come up with anything without breaking the principle that this should be primarily a client side item.
This Pull Request (PR) fixes the following issues
Fixes #411