puppetlabs-apache icon indicating copy to clipboard operation
puppetlabs-apache copied to clipboard

add Hash $vhosts_defaults to apache::vhosts class?

Open janit42 opened this issue 3 years ago • 8 comments

Use Case

In our environment we have a some per-vhosts-settings which are identical in multiple vhosts. Therefore it would be great to be able to set defaults for multiple vhosts through the apache::vhosts class.

Describe the Solution You Would Like

add a parameter to the apache::vhosts class, that would be passed to the create_resources function:

Hash $vhosts_defaults = {},

Describe Alternatives You've Considered

Alternatively it would be possible to use resource defaults, but we have a Hiera-heavy setup on some nodes, where we just define the apache::vhosts::vhosts in Hiera and include the apache::vhosts class to make sure these get picked up and that would work more smoothly with a $vhosts_defaults parameter.

Additional Context

If there was nothing against implementing such a defaults Hash in principle, I'd try to create a pull request.

janit42 avatar Oct 06 '22 11:10 janit42

Thanks @janit42 for creating issue. I started looking into this but somehow I was not able to get the full context on above feature, can you please given some context around it and also will be helpful if you can put one of example for better understanding?

Ramesh7 avatar Sep 14 '23 03:09 Ramesh7

[...] can you please given some context around it and also will be helpful if you can put one of example for better understanding?

Thanks for your reply @Ramesh7 ! I've created an updated version of the apache::vhosts class in https://github.com/janit42/puppetlabs-apache/commit/91ad6e638f2c228fd9b7b16d442fb2d6fb4dd104 and tried to expand the classes docs with a description and an example for the usage of the proposed parameter.

In principle, I would enable the apache::vhosts class to pass resource defaults for the internally used create_resources function

The new parameter could be used on nodes with a high number of relatively uniform vhosts to pass common vhosts_settings only once through the vhosts_defaults hash instead of writing the same attribute to every single vhost explicitly.

It could be even used, when preset in Hiera via a data/common.yaml file to set site-wide defaults, e.g. if a company policy required masking clients ip-addresses in logfiles, this could be done via setting this in the vhosts_defaults hash in Hiera, so nobody can setup a vhost for whatever service through the apache::vhosts class and forget to activate that setting (this is shown as an example in the commit referenced above).

Does that clarify the idea enough? If so, do you think I could create a pull-request from the referenced commit?

janit42 avatar Sep 15 '23 14:09 janit42

Thanks @janit42 for your quick response. That's good idea, to avoid the repetition of some common params and abstracting those in default params and merge those in apache::vhosts while creating the apache::vhost.

BTW not sure if I am missing here but these vhosts_defaults will get reflect to each vhost which will get created, if I am right then probably your resource creation will look something like :

class apache::vhosts (
  Hash $vhosts          = {},
  Hash $vhosts_defaults = {},
) {
  include apache
  $vhost_params = merge($vhosts, $vhosts_defaults)
  create_resources('apache::vhost', $vhost_params)
}

I would be grateful if you could retrieve the pull request associated with the commit. Thanks again!!

Ramesh7 avatar Sep 18 '23 12:09 Ramesh7

create_resources has a third parameter for defaults. https://www.puppet.com/docs/puppet/7/function.html#create-resources. So it's just:

class apache::vhosts (
  Hash $vhosts          = {},
  Hash $vhosts_defaults = {},
) {
  include apache
  create_resources('apache::vhost', $vhosts, $vhosts_defaults)
}

ekohl avatar Sep 18 '23 14:09 ekohl

create_resources has a third parameter for defaults. https://www.puppet.com/docs/puppet/7/function.html#create-resources. So it's just:

class apache::vhosts (
  Hash $vhosts          = {},
  Hash $vhosts_defaults = {},
) {
  include apache
  create_resources('apache::vhost', $vhosts, $vhosts_defaults)
}

Got it, Thanks @ekohl for correcting me.

Ramesh7 avatar Sep 19 '23 15:09 Ramesh7

You can use :

# @summary
#   deploy with hiera data
# 
# @param configs_hash
#   Default value {}.
# 
class aaa::deploy::vhost (
  Hash $configs_hash = {}
) {
  # NOTE: hiera_hash does not work as expected in a parameterized class
  #   definition; so we call it here.
  #
  # http://docs.puppetlabs.com/hiera/1/puppet.html#limitations
  # https://tickets.puppetlabs.com/browse/HI-118
  #
  $configs = lookup('aaa::deploy::vhost', { 'default_value' => $configs_hash, 'merge' => { 'strategy' => 'deep', }, 'value_type' => Hash })
  if !empty($configs) {
    create_resources('apache::vhost', $configs)
  }
}

wywerne avatar Apr 26 '24 10:04 wywerne

if !empty($configs) {

I think create_resources can deal with empty hashes so this would be redundant.

Having said that, https://github.com/puppetlabs/puppetlabs-apache/issues/2325#issuecomment-1725830203 already has the code. If anyone feels like it, they should submit a patch to https://github.com/puppetlabs/puppetlabs-apache/blob/main/manifests/vhosts.pp.

ekohl avatar Apr 26 '24 11:04 ekohl

But class apache is declared and Apache httpd server is installed without vhosts... is this really necessary?

wywerne avatar Apr 26 '24 11:04 wywerne