Hiera lookup merge strategy with knockout prefix

[adepretis]

Currently explicit Hiera lookups in e.g. ssh::server overwrite globally configured Hiera merge strategies used when Puppet does automatic class parameter lookups, because the module tries to support and merge both Hiera data and class parameters.

Therefore you can't use e.g. knockout_prefix

Is this a common pattern btw.? - usally either automatic class parameter lookups via Hiera OR explicit class paramters are supported by modules.

Because of supporting both the lookup has to be explicit and it doesn't respect the globally configured merge behaviour:

# manifests/ssh/server.pp
class ssh::server(
  ...
  # integrated with Hiera via automatic class parameter lookup
  Hash    $options              = {},
  ...
) {
  ...
  # merge with additional explicit class parameters
  $hiera_options = lookup("${module_name}::server::options", Optional[Hash], 'deep', {})
  $fin_options = deep_merge($hiera_options, $options)
  ...
}

# Hiera data
ssh::server::options:
  AllowUsers:
    - myuser

```my.pp
# my.pp
class { 'ssh::server':
  options => {
    'AllowUsers' => [ 'myotheruser' ],
  }
}

Results (current behaviour) in:

AllowUsers myuser
AllowUsers myotheruser

IMO common expected behaviour would be (explicit class parameter takes precedence)

AllowUsers myotheruser

The module should either use Hiera data via automatic class parameter lookup with explicit class parameter overrides while respecting global lookup_options like:

class ssh::server(
  ...
  # integrated with Hiera via automatic class parameter lookup
  Hash    $options              = {},
  ...
) {
  # no merging of Hiera data and class parameter here
  ...
}

or provide a way to inject Hiera lookup options:

# manifests/ssh/server.pp
class ssh::server(
  ...
  # integrated with Hiera via automatic class parameter lookup
  Hash    $options                         = {},
  Hash    $hiera_lookup_options = { 'strategy'  => 'deep' },
  ...
) {
  ...
  # merge with additional explicit class parameters
  $hiera_options = lookup("${module_name}::server::options", {
    'value_type' => Optional[Hash],
    'merge'         => $hiera_lookup_options,
  })
  $fin_options = deep_merge($hiera_options, $options)
  ...
}

Which would allow for correct merge behaviour with a knockout_prefix defined:

# defaults.yaml
ssh::server::hiera_lookup_options:
  strategy: deep
  knockout_prefix: '--'
ssh::server::options
  AllowUsers:
     - myuser
     - otheruser

# my_node_or_profile.yaml
ssh::server::options
  AllowUsers:
    - --myuser

resulting in:

AllowUsers otheruser

Of course having just the knockout_prefix as class parameter or hardcoding knockout_prefix with every lookup() would also be possible - I guess the solutions above would be the most flexible.

./discuss ;-)

[saz]

lookup isn't used anymore in master right now and this issue shouldn't be valid anymore.