Make parsing defaults based on other attributes consistent.
Currently in our codebase, defaults for attributes are specified in the attributes.rb
, but attributes that we want to default based on other attributes have diverged into being set in two different places.
- In our 'parse' libraries run during the converge of the config recipe. (Which is always run first)
- Here is a simple example for gitlab-shell: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/2003bc5d/files/gitlab-cookbooks/gitlab/libraries/gitlab_shell.rb#L31
- As needed in other recipes during their converge, after all the use settings have already been applied.
- Here is a recent example with postgres: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/68cdea57/files/gitlab-cookbooks/postgresql/recipes/directory_locations.rb#L2
We should try and make the code more consistent. As having the code do this in two separate places can cause issues when the first method is trying to use a setting that isn't set until the second method (which comes later). See #3920 (closed) for an example of this issue.
In the first method, where to look for settings is currently fairly well known. But note that even in our simple shell example it takes two lines to establish the new default, the first being a necessary duplication of what the config parse actually does. Whereas our second method is able to set the new default in a single line. (It just happens to be setting three new defaults in that file)
And our first method can get par more complex, if you take a look in other recipes.
My personal opinion is that we should look at moving more things to the second method, but establish a very consistent naming schema and location for them, so people can easily find where the settings are being populated. Another reason for the second, is that it has actually been necessary in the past. For example when we wanted to change default nginx settings based on the result of a resource, like let's encrypt's authorisation.