Skip to content

Commit 020f8a0

Browse files
committed
Clarify parameter defaults guidance in bgtm and style guide
Both pages left module authors unsure where parameter defaults belong, which caused confusion in module reviews: static values duplicated in both init.pp and module Hiera data, and parameters declared Optional that always receive a value from Hiera. - bgtm.md: add a "Parameter defaults" subsection under Parameters covering static-vs-OS-specific placement, the one-home rule, and Optional[T] = undef usage, with a cross-link to the style guide. - style_guide.markdown: split "Parameter defaults" into static (inline in init.pp) and OS-specific approaches. The OS-specific example keeps the common value inline and overrides only the deviating OS through automatic parameter lookup, so authors add Hiera data for the exceptions rather than every supported OS. Cites puppet-chrony as a real-world reference, clarifies that common.yaml is not the home for parameter defaults, and adds guidance on keeping each default in one place and on Optional parameters. - style_guide.markdown: reconcile the pre-existing myservice class-layout example, which previously placed a static service_ensure default in common.yaml, by moving that default inline to match the new guidance. Closes #216 Signed-off-by: Michael Harp <mike@mikeharp.com>
1 parent 34b71c1 commit 020f8a0

2 files changed

Lines changed: 76 additions & 25 deletions

File tree

docs/_openvox_8x/bgtm.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,23 @@ Hardcoded parameters in templates inhibits flexibility over time. It is far bett
193193

194194
For an example of a module that capitalizes on offering many parameters, please see [puppetlabs-apache](https://forge.puppet.com/puppetlabs/apache).
195195

196+
#### Parameter defaults
197+
198+
Where you put a parameter's default value depends on whether that value is the same on every supported operating system:
199+
200+
* **Static defaults that are identical across every supported OS** belong inline in the parameter declaration in `init.pp`.
201+
* **OS-specific defaults** belong in module Hiera data with a per-OS hierarchy, resolved through automatic parameter lookup.
202+
203+
Keeping static defaults inline puts the value right where the parameter is declared, which is what module reviewers expect and what keeps the default easy to find.
204+
205+
Inline defaults also render in generated reference documentation with any toolchain. Defaults placed only in `data/common.yaml` are invisible to upstream [puppet-strings](https://github.com/puppetlabs/puppet-strings/issues/250), though OpenVox's [openvox-strings](https://github.com/voxpupuli/openvox-strings/pull/27) can now read them.
206+
207+
Give each default a single home. Duplicating a value between `init.pp` and module Hiera data means two sources of truth that can drift apart.
208+
209+
Reserve `Optional[T] = undef` for parameters where `undef` is a genuine runtime value, such as a parameter that toggles an optional feature off. Avoid declaring a parameter `Optional` simply to defer its default to Hiera when the parameter will always receive a value, since that misleads users into thinking `undef` is a supported state.
210+
211+
For more on the mechanics of parameter defaults and Hiera data in modules, see the [Puppet language style guide](./style_guide.html#parameter-defaults).
212+
196213
### Ordering
197214

198215
Best practice is to base all order-related dependencies (such as `require` and `before`) on classes rather than resources. Class-based ordering allows you to shield the implementation details of each class from the other classes.

docs/_openvox_8x/style_guide.markdown

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -733,9 +733,9 @@ In `init.pp`:
733733
# filtered out if present
734734
#
735735
class myservice (
736-
Enum['running', 'stopped'] $service_ensure,
736+
Enum['running', 'stopped'] $service_ensure = 'running',
737737
String $tempfile_contents,
738-
Optional[Array[String[1]]] $package_list = undef,
738+
Optional[Array[String[1]]] $package_list = undef,
739739
) {
740740
# Example of additional assertion with a better error message than just saying that
741741
# there was a type mismatch for $package_list.
@@ -777,21 +777,12 @@ version: 5
777777
defaults:
778778
data_hash: yaml_data
779779

780-
# The default values can be merged if you want to extend with additional packages
781-
# If not, use 'default_hierarchy' instead of 'hierarchy'
782-
#
783780
hierarchy:
784781
- name: 'Per Operating System'
785782
path: 'os/%{os.name}.yaml'
786-
- name: 'Common'
787-
path: 'common.yaml'
788783
```
789784
790-
In module `data/common.yaml`:
791-
792-
```yaml
793-
myservice::service_ensure: running
794-
```
785+
The static `service_ensure` default lives inline in `init.pp`, so only the OS-specific `package_list` needs data files.
795786

796787
In module `data/os/centos.yaml`:
797788

@@ -895,41 +886,84 @@ class ntp (
895886

896887
### Parameter defaults
897888

898-
Adding default values to the parameters in classes and defined types makes your module easier to use. Use Hiera data in the module and rely on automatic parameter lookup for class parameters. See the documentation about [automatic parameter lookup](./hiera_automatic.html#puppet-lookup) for detailed information.
889+
Adding default values to the parameters in classes and defined types makes your module easier to use. Take care to declare the data type of parameters, as this provides automatic type assertions.
890+
891+
Where a default value lives depends on whether it varies by operating system.
892+
893+
#### Static defaults
894+
895+
When a parameter's default is the same on every supported operating system, declare it inline in the parameter list.
896+
897+
Keeping static defaults inline puts the value right where the parameter is declared, which keeps it easy to find and is the convention module reviewers expect.
898+
899+
Inline defaults also render in generated reference documentation with any toolchain. Defaults placed only in `data/common.yaml` are invisible to upstream [puppet-strings](https://github.com/puppetlabs/puppet-strings/issues/250), though OpenVox's [openvox-strings](https://github.com/voxpupuli/openvox-strings/pull/27) can now read them.
900+
901+
**Good:**
902+
903+
```puppet
904+
class my_module (
905+
String[1] $ensure = 'present',
906+
Integer[0] $port = 8080,
907+
Boolean $manage = true,
908+
) {
909+
# body of class
910+
}
911+
```
912+
913+
#### OS-specific defaults
899914

900-
Take care to declare the data type of parameters, as this provides automatic type assertions.
915+
Typically one or two operating systems need a different value while the rest share a common one. Keep the common value as the inline default and use module Hiera data only to override it for the operating systems that differ.
916+
917+
Automatic parameter lookup takes precedence over the inline default when a matching key exists, so you supply data only for the exceptions rather than for every supported OS. See the documentation about [automatic parameter lookup](./hiera_automatic.html#puppet-lookup) for detailed information.
901918

902919
**Good:**
903920

921+
The common `config` default stays inline; only nodes in the Red Hat family override it through Hiera:
922+
904923
```puppet
905-
# parameter defaults provided via APL > puppet 4.9.0
906924
class my_module (
907-
String $source,
908-
String $config,
925+
String[1] $source = 'default source value',
926+
String[1] $config = '/etc/mymodule/mymodule.conf',
909927
) {
910928
# body of class
911929
}
912930
```
913931

914-
with a `hiera.yaml` in the root of the module:
932+
with a `hiera.yaml` in the root of the module that adds a per-OS hierarchy level:
915933

916934
```yaml
917935
---
918936
version: 5
919-
default_hierarchy:
920-
- name: 'defaults'
921-
path: 'defaults.yaml'
937+
defaults:
922938
data_hash: yaml_data
939+
hierarchy:
940+
- name: 'OS family'
941+
paths:
942+
- '%{facts.os.name}.yaml'
943+
- '%{facts.os.family}.yaml'
923944
```
924945

925-
and with the file `data/defaults.yaml`:
946+
and an override for just the deviating OS in `data/RedHat.yaml`:
926947

927948
```yaml
928-
mymodule::source: 'default source value'
929-
mymodule::config: 'default config value'
949+
mymodule::config: '/etc/sysconfig/mymodule'
930950
```
931951

932-
This places the values in the defaults hierarchy, which means that the defaults are not merged into overriding values. If you want to merge the defaults into those values, change the `default_hierarchy` to `hierarchy`.
952+
Nodes in the Red Hat family pick up the Hiera value; every other operating system falls through to the inline default. No `common.yaml` entry is needed, because the inline default already covers the common case.
953+
954+
The [puppet-chrony](https://github.com/voxpupuli/puppet-chrony) module is a good real-world reference for this pattern: it keeps common defaults inline in `init.pp` and overrides only the values that differ in per-OS Hiera data files, with no `common.yaml`.
955+
956+
#### Keep each default in one place
957+
958+
Give each default a single home. Declaring the same value both inline in `init.pp` and in module Hiera data creates two sources of truth that can drift apart. Choose the location based on whether the default varies by OS, and put it there only.
959+
960+
Under this guidance, parameter defaults rarely belong in `common.yaml`: a static default goes inline, and an OS-specific one goes in a per-OS data file. `common.yaml` stays available for module data that isn't a plain class-parameter default, such as values you look up explicitly with `lookup`.
961+
962+
#### Optional parameters
963+
964+
Reserve `Optional[T] = undef` for parameters where `undef` is a genuine runtime value, such as a parameter that switches an optional feature off. In that case, `undef` is a meaningful state the user can select.
965+
966+
Avoid declaring a parameter `Optional` just to defer its default to Hiera when the parameter will always receive a value. Doing so misleads users into thinking `undef` is a supported state when it isn't, and it weakens the type assertion. Declare the real type and give the parameter its actual default instead.
933967

934968
### Exported resources
935969

0 commit comments

Comments
 (0)