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

add Hash $vhosts_defaults to apache::vhosts class? #2325

Open
janit42 opened this issue Oct 6, 2022 · 8 comments
Open

add Hash $vhosts_defaults to apache::vhosts class? #2325

janit42 opened this issue Oct 6, 2022 · 8 comments

Comments

@janit42
Copy link

janit42 commented Oct 6, 2022

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.

@Ramesh7
Copy link
Contributor

Ramesh7 commented Sep 14, 2023

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?

janit42 pushed a commit to janit42/puppetlabs-apache that referenced this issue Sep 15, 2023
this add the possibility to pass vhosts_defaults to the class for enforcing some site-wide vhost stanards
e.g. via Hiera or to significantly shorten the content of  on nodes hosting many vhosts (puppetlabs#2325)
janit42 pushed a commit to janit42/puppetlabs-apache that referenced this issue Sep 15, 2023
this add the possibility to pass vhosts_defaults to the class for enforcing some site-wide vhost stanards
e.g. via Hiera or to significantly shorten the content of  on nodes hosting many vhosts (puppetlabs#2325)
janit42 pushed a commit to janit42/puppetlabs-apache that referenced this issue Sep 15, 2023
this add the possibility to pass vhosts_defaults to the class for enforcing some site-wide vhost stanards
e.g. via Hiera or to significantly shorten the content of  on nodes hosting many vhosts (puppetlabs#2325)
@janit42
Copy link
Author

janit42 commented Sep 15, 2023

[...] 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 janit42@91ad6e6 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?

@Ramesh7
Copy link
Contributor

Ramesh7 commented Sep 18, 2023

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!!

@ekohl
Copy link
Collaborator

ekohl commented Sep 18, 2023

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)
}

@Ramesh7
Copy link
Contributor

Ramesh7 commented Sep 19, 2023

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.

janit42 pushed a commit to janit42/puppetlabs-apache that referenced this issue Sep 20, 2023
this add the possibility to pass vhosts_defaults to the class for enforcing some site-wide vhost stanards
e.g. via Hiera or to significantly shorten the content of  on nodes hosting many vhosts (puppetlabs#2325)
@wywerne
Copy link

wywerne commented Apr 26, 2024

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)
  }
}

@ekohl
Copy link
Collaborator

ekohl commented Apr 26, 2024

if !empty($configs) {

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

Having said that, #2325 (comment) 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.

@wywerne
Copy link

wywerne commented Apr 26, 2024

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

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

No branches or pull requests

6 participants