-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some configuration improvements #15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure if this helps much for this template. Basically, anything more advanced than this requires proper engineering anyway.
Yeah, I agree. I just submitted it with the idea of giving, in the template, an example of behaviour to follow when dealing with configuration. I've seen many formulas modiying config by default and sometimes, specially when they're not up to date with the OS changes, changing configs in ways that can be dangerous (ie, re-enabling SSLv2 or opening ports by default, etc.) Probably it's better to add these ideas in the docs, right? Or better, also in the docs, as many people that develop formulas, once they get the gist, don't go back much to those docs, and just clone from the template? Another recommendation regarding config would be that, whenever the package allows it, keep the main config as is, and just override the desired parameters in a local config file. But as you say, not sure if this is the place to put those things as examples, or the way to do it is just in the docs? If you consider these ideas ar worthy, I'll be glad to try to add them where you suggest. Otherwise, I'll be OK if this PR is just closed and disregarded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the README update reflecting the actual repo structure. Well done!
|
||
# If don't you have other config files to manage, remove these lines | ||
# The parameter `manage_another_config must default to False` in | ||
# defaults.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could go ahead and simply add those parameters (manage_another_config
, another_config
) to the defaults.yaml
file? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if that is a good idea, this basically assumes that someone will use the template to create a formula with optional configuration.
It is true that unmaintained formulas might modify configuration files in a way that is undesirable, but I don't think it is a good idea to try and blanket-disable all the configuration states in all formulas. Surely most people doing advanced stuff won't even use the template, but those who do are already having trouble the the concept of having map.jinja and defaults.yaml plus the lookup tree in the pillar on top of everything else. It would probably be a good thing if this were indeed put in the docs, maybe some sort of warning to make sure you know what the forumla is going to do before just blindly including everything. In many cases a simple fun of init.sls with basic settings will work just fine, and in those cases often the desired result aligns with what the template does:
If at some point a user would want something else, they would already be at the point where they pick and choose states anyway (i.e. run template.install and maybe template.service but not template.config). To actually fix the issue with degraded formulas and stability, we would have to fix the whole repository deal as a complete thing, for example, getting SPM and a package management system up, maybe in NPM or Composer style where the packages are still mostly referenced from github, but the stuff you get from the package manager is versioned, checked, and perhaps gives warnings if you add a package that is unmaintained, has no tests like kitchen salt+serverspec, or perhaps has produced errors on a test run in general. If you were to want to go ahead and have a more 'managed' template, I'd suggest adding one or more repositories, containing various examples and docs for formulas, and pointing to this one to refer to a basic traditional formula, hosted and named in the style of saltstack-formulas. |
Those things are absolutely valid. The scope of getting it done is just to broad for a single PR. For now I think we could leave |
Closing, as the comments suggest keeping |
We should open a 2nd ticket or repo with the more advanced template techniques as it is definitely valuable. While putting formula stuff in Salt's main docs is a good idea, the issue there is probably that formulas are rather free-form and constraining them from the get-go is maybe not the best way to do things. On the other hand, putting out best practises is probably fine. Do we split this off into a separate repo or do we raise an issue with Salt first? |
Some suggestions as discussed here.