Add class apache::vhosts to create apache::vhost resources

[gerhardsam]

This way it is possible to create apache::vhost resources directly via the main class without declaring them in a separate role or profile.

[jonnytdevops]

@gerhardsam Thank you for your efforts, they are very much appreciated!

It would be appreciated if you could give a little context to why this feature would be beneficial to all users. What are the issues related to using the apache::vhost type to obtain the desired functionality?

Thanks

[smoeding]

With this change you can use the apache::vhosts key in hiera to specify the vhosts to create. So in many cases it would be sufficient to declare the apache class and have all parameters in hiera. Currently an additional class is needed to define your vhosts from hiera (since a defined type does not use hiera for data binding: https://ask.puppet.com/question/1549/how-to-create-an-apache-vhost-in-hiera-when-using-puppetlabsapache/).

[jonnytdevops]

@smoeding What we normally do if you wish for heira compatibility, is to create a new .pp file and call create_resources().

I personally do not like the duplicated behaviour which could confuse users.

@hunner @bmjen any thoughts on this?

Thanks!

[bmjen]

I agree with @jonnytpuppet here. I'd prefer to keep the current behavior and not overcomplicate the apache class. This is traditionally the model we use in other supported modules, and as mentioned by JT, has a working model for hiera.

[gerhardsam]

@jonnytpuppet Thanks for the feedback and sorry for not answering earlier.
The context is the following: We use Satellite 6 (which acts ENC) which allows only to add classes but not defined types. Therefore it is like @smoeding depicted that I had to write an extra wrapper class to create the defined resource apache::vhost. Because I wanted to get rid of this necessity I wanted to mimic the pattern which was is used in the puppet-nginx module.

@bmjen I do not think that is an overcomplication of the apache class but as an option I could add a class apache::vhosts which would create the apache:vhost resources instead.
Would that be a viable option?

[jonnytdevops]

@gerhardsam I honestly believe the best way forward is to create a wrapper class in a seperate .pp file that makes a call out to create_resources, then uses the results to call apache::vhost

An argument could be made that this should actually fall outside of the module altogether, and should be part of your own environment manifests, however we have president for having a wrapper class inside a module in a separate .pp file, which I'd be willing to merge :)

Thanks!

[gerhardsam]

@jonnytpuppet I extracted the creation of the apache::vhost resources in the new class apache::vhosts. Does that conform to your expectation?

I am not sure yet, whether to create a separate acceptance test or whether to keep it in spec/acceptance/vhost_spec.rb?

[jonnytdevops]

@gerhardsam You still have references to vhosts in the apache class. Shouldn't these be removed now?

Thanks

[gerhardsam]

@jonnytpuppet I removed the references in the apache class, added docs and squashed the commits.
Would you mind reviewing the change? Thanks.

[gerhardsam]

@jonnytpuppet Thanks for merging!