Add hiera support for creating instances of defined types?

[decibelhertz]

Hi folks --

I'm using this module due to the stated project goals meeting my needs much better than other modules at the Forge. Thanks for the work.

I'm wondering if you'd accept a PR for adding some empty hashes to init.ppso that I can declare defined types from this module via hiera. EG...

ssh::server_matches:
  'Address 127.0.0.0/8,[::1]':
    maxsessions: 3
    permitrootlogin: 'prohibit-password'
    order: 98
# etc

For the PR, I would make several default-empty hash variables in the init class and then create_resources() where appropriate...

class ssh (
  Hash $allowgroups,
  Hash $keygens,
  Hash $client_configs,
  Hash $client_hosts,
  Hash $client_matches,
  Hash $server_configs,
  Hash $server_hostkeys,
  Hash $server_matches,
  Hash $server_subsystems,
) {
#...etc
  create_resources('ssh::allowgroup', $allowgroups)
  create_resources('ssh::keygen', $keygens)
}
class ssh::server (
  create_resources('ssh::server::config', $ssh::server_configs)
  create_resources('ssh::server::hostkey', $ssh::server_hostkeys)
  create_resources('ssh::server::match', $ssh::server_matches)
  create_resources('ssh::server::subsystem', $ssh::server_configs)
#...etc
}

I expect the example is clear enough, but will note that I would update the ssh::client class similarly. Happy to update the rspec tests to account for the new behavior, too.

Finally, I will point out that I am presently doing what I want via a profile class....

# Manage the SSH daemon and SSH host keys of node
class profile::ssh(
  Hash $ssh_allowgroups,
  Hash $ssh_keys,
  Hash $sshd_matches,                                                     
  Hash $sshd_subsystems,                                                  
) {                                                                       
                           
  include profile::sshkeys
                                           
  contain ssh::server                                                     
                                                                          
  create_resources('sshkey',$ssh_keys)                                    
  create_resources('ssh::server::match',$sshd_matches)
  create_resources('ssh::server::subsystem',$sshd_subsystems)
                                         
  # There is a chance for duplicates, so we iterate
  $ssh_allowgroups.each |String $key, Hash $value| {
    ensure_resource('ssh::allowgroup',$key,$value)
  }                                                                       
}

However, for maintenance/team discovery purposes, I think it would help to have such variables in the ssh:: variable namespace, hence my inquiry.

Cheers.

[zachfi]

Hi, thank you for raising the issue. I think if we can keep the implementation clean and we account for the behavior in the tests, your proposal sounds reasonable to me. I suspect you didn't mean to include ssh::server::config in the example since its a class and not a define, but the rest of it seems right. I hope the dependencies between the defines and the "parent" class are easy to work out, or perhaps non-existent.

[zachfi]

Please include an update to the docs if you don't mind also. Cheers.

[decibelhertz]

Oops, yes. Looks like I copy/pasted too much. Only defined types would be used.

Will whittle away at the PR as possible and pipe back up. Will update docs as requested.

Cheers.

[decibelhertz]

In digging into this, I am wondering about ssh::allowgroup. One perspective is that it should be renamed ssh::server::allowgroup since it affects sshd and not the ssh client, but I admit to being insensitive to the chroot logic.

Do you want me to modify?

To start, I won't and will simply make the defined type callable from the relative module EG Hash $ssh::allowgroups rather than Hash $ssh::server::allowgroups.

[decibelhertz]

Nevermind that last point.

I will end up putting all hashes in the init class after running through some unit tests.