create_resources
77 views
Skip to first unread message
Nerbolff
Feb 6, 2021, 8:34:53 AM2/6/21
to Puppet Users
hello there,
I would like to clean up my puppet recipes.
My init.pp. I have ~three_hundred entries as 'new resources'(see below).
here is how is setup today. I am wondering if we could do better. I am working with puppet v5.
ideas or suggestions? thank you very much.
mymodule/manifests/init.pp
class mymodule {
if ($::collect == 'external') {
create_resources('nginx::resource::server', lookup('mymodule::external_http_domains'), lookup('mymodule::http_defaults'))
...
...
...
} else {
create_resources('nginx::resource::server', lookup('mymodule::internal_http_domains'), lookup('mymodule::http_defaults'))
...
...
...
}
mymodule/data/external.yaml
mymodule::http_defaults:
ensure: present
ssl: true
listen_port: 443
...
...
...
mymodule::external_http_domains:
use_default_location: false
ssl_cert: /etc/nginx/ssl/twe/order1.internet.com.pem
ssl_key: /etc/nginx/ssl/twe/order1.internet.com.key
...
...
...
Martin Alfke
Feb 7, 2021, 6:39:46 AM2/7/21
to Puppet Users
Hi,
there are some minor not-best-practices in your code:
1. create_resource
I prefer using lambda over create_resources as these allows explizit resource declaration which are then bound to the class.
This gives you explizit ordering, which you must add if using create_resource.
2. explizit lookups
you are doing explizit lookups. I prefer parameters using automatic databinding
3. accessing facts
Using $::factname is not the most modern way. Migrate to using facts hash (facts is a protected variable).
e.g. $facts['collect']
Your new code could be the following:
class mymodule (
Hash $external_http_domains, # not providing a default means that you must deliver data.
Optional[Hash] $internal_http_domains = undef, # if you want to allow undef values, you can set the data type to optional.
Hash $http_defaults,
){
if $facts['collect'] == 'external' {
$external_http_domain.each |$key, $value| {
nginx::resource::server { $key:
* => $value + $http_defaults,
}
}
} else {
$internal_http_domains.each |$key, $value| {
nginx::resource::server { $key:
* => $value + $http_defaults,
}
}
}
}
hth,
Martin
> --
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/1124d0d6-06b4-4ad5-9f06-a8a30fb9bbbfn%40googlegroups.com.
there are some minor not-best-practices in your code:
1. create_resource
I prefer using lambda over create_resources as these allows explizit resource declaration which are then bound to the class.
This gives you explizit ordering, which you must add if using create_resource.
2. explizit lookups
you are doing explizit lookups. I prefer parameters using automatic databinding
3. accessing facts
Using $::factname is not the most modern way. Migrate to using facts hash (facts is a protected variable).
e.g. $facts['collect']
Your new code could be the following:
class mymodule (
Hash $external_http_domains, # not providing a default means that you must deliver data.
Optional[Hash] $internal_http_domains = undef, # if you want to allow undef values, you can set the data type to optional.
Hash $http_defaults,
){
if $facts['collect'] == 'external' {
$external_http_domain.each |$key, $value| {
nginx::resource::server { $key:
* => $value + $http_defaults,
}
}
} else {
$internal_http_domains.each |$key, $value| {
nginx::resource::server { $key:
* => $value + $http_defaults,
}
}
}
}
hth,
Martin
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/1124d0d6-06b4-4ad5-9f06-a8a30fb9bbbfn%40googlegroups.com.
Reply all
Reply to author
Forward
0 new messages