Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use regular hiera parameter lookup instead of manual calling lookup or add flag #327

Closed
c33s opened this issue Mar 28, 2022 · 4 comments · Fixed by #328
Closed

Use regular hiera parameter lookup instead of manual calling lookup or add flag #327

c33s opened this issue Mar 28, 2022 · 4 comments · Fixed by #328

Comments

@c33s
Copy link

c33s commented Mar 28, 2022

currently this module uses a mix out of automatic hiera class parameter lookup and a manual call to lookup

class ssh::server (
...
  Hash    $options              = {},
...
) inherits ssh::params {
  $hiera_options = lookup("${module_name}::server::options", Optional[Hash], 'deep', {})
...
  $fin_options = deep_merge($hiera_options, $options)

this can lead to an unexpected behavior if you define ssh::server_options and expect it to obey the defined lookup_options

if i define the following in common.yaml:

ssh::server_options:
  Include: '/etc/ssh/sshd_config.d/*.conf'
  LogLevel: VERBOSE

i cannot simple define the same without Include for a specific host which does not support the include funciton. why not only lookup the options via hiera? with hiera i can define the merge behavior myself and are not stuck with an enforced deep merge.

ssh::server_options: {merge: {strategy: deep, merge_hash_arrays: true, knockout_prefix: "--", sort_merged_arrays: false}}

or what about adding a flag for this?

  Boolean $do_extra_hiera_lookup        = true
  String $extra_hiera_lookup_merge_behavior = 'deep'
  
) inherits ssh::params {
  # Merge hashes from multiple layer of hierarchy in hiera
  if ($do_extra_hiera_lookup) {
    $hiera_options = lookup("${module_name}::server::options", Optional[Hash], $extra_hiera_lookup_merge_behavior, {})
    $hiera_match_block = lookup("${module_name}::server::match_block", Optional[Hash], $extra_hiera_lookup_merge_behavior, {})
  } else {
    $hiera_options = {}
    $hiera_match_block = {}
  }
...

this would really help

@SimonHoenscheid
Copy link
Contributor

@c33s that would be a good addition to PR #325 . I thought about this as well.

@c33s
Copy link
Author

c33s commented Apr 2, 2022

@SimonHoenscheid that would be nice :)

@saz
Copy link
Owner

saz commented Apr 13, 2022

This is some old behavior from an earlier hiera time :-)
I don't see a reason to keep it like that.

@SimonHoenscheid maybe you can add it to your PR #325?

@SimonHoenscheid
Copy link
Contributor

@saz I can add an other PR that can be merged directly afterwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants