voxpupuli-archive / puppet-bacula

Bacula module to manage all components of bacula
https://forge.puppet.com/puppet/bacula
Apache License 2.0
38 stars 52 forks source link

Rewrite of generate_clients.rb #40

Open bastelfreak opened 8 years ago

bastelfreak commented 8 years ago

quote from @hunner:


All of the above code is terrible and should be deleted. This function takes an argument as args but that in handled in the second block below... What is this block doing then? It calls to_hash seemingly out of no where, and that takes the catalog's scope and turns it into a hash, supposedly to look for variables "at top scope" that come from some data source (like the old puppet enterprise console) but it has no safety and will pick up ANY variables in the namespace, including facts. This is a terrible design and seriously bad side-effects. I mean, this function says "okay, I was called with args but before using that I'm just going to see if I can find anything else that could possibly have been passed to me but wasn't" and then tries to execute it. Also, the raises in the begin just get captured by the rescue block that then raises again with the same message. Wat.


based on a discussion in https://github.com/voxpupuli/puppet-bacula/pull/37#discussion_r60458751

dhoppe commented 7 years ago

The function generate_clients does not use the parameter clients from manifests/director.pp. Somehow it uses the fact bacula_client_package from spec/classes/bacula_spec.rb.

I have no idea why the Rspec tests contain these facts, since the module does not provide any of them. Maybe it is time to ask @ccaum how this worked in the first place.

Even if I exclude backup_client_package at lib/puppet/parser/functions/generate_clients.rb and configure a hash of clients, there are several errors regarding db_backend at manifests/director.pp.

I think this module need to be rewritten completely.