Open davide125 opened 3 years ago
@davide125 A default approach to perform such a validation is to use verify
property of template
resource however that is tricky given how Apache configurations files are hierarchical. http -t
runs on the entire hierarchy of included files, starting from the root http.conf
, on the filesystem, however verify
only provides a temporary path to a single file(fb_apache.conf
) which doesn't have all the context to be verified in isolation; not even with httpd -t -f <config_file>
.
One can only do a non-trivial, potentially error prone, and brittle implementation which is something that may not be worth doing. We'd be happy to review PRs but not something that we'd implement. Feel free to DM me and I can provide some internal context as well.
@gogsbread - Just for context, Davide is one of your co-workers (he's on the kernel team), and was on the OS Team that wrote these cookbooks (before the OS Team was disbanded in the recent re-org).
And yeah, it's harder with Apache than some others for the reason you describe. This is why we made a task for it rather than doing it (if it was easy we would have made the one-line diff :) ).
However the complete lack of any verification has caused a lot of problems thus far. At Facebook the yum origin servers went down. At SCALE the website went down. So it's painful. We didn't have a great solution off hand, so we just created this as a placeholder.
That said, we added none of that context to this task, which I'm sure I'll get a (justified) earful from @NaomiReeves on, and which caused you to trip on this.
So I think at this point this has enough context to leave open for one of us one day when we feel like digging into a fun problem. But if you prefer to modify the Issue itself, and then delete these comments, that's also fine.
@jaymzh thank you for the context. These comments and discussion and useful and we'll continue to have this documented. I did tag Davide in the diff which has more elaborate documentation on the thought process and some alternatives that were discussed.
In summary, there are two high level approaches I could think of and worth documenting:
httpd -t
in there. If the verification is successful then we go about materializing the changed configurationhttpd -t
to find errors and if wrong restore from the backup
copy and fail the chef run.For 1. @NaomiReeves had some discussions with chef dev
move the templates creation in to a custom resource that would collect/track all of the templates being modified and materialize them all in the same tmp dir grab the existing configurations and de-dup against the updated configs run the verify command on the group of new configs if the verify command passes, move the updated configs to their correct path if the verify command fails, do not move the updated configs and fail the chef run
For 2. I tried to provide a rough implementation, however I couldn't find an easy way currently to access the path where the file was backed-up.
The backup directory is easy to find, it's in the config object, but I definitely would not go that route. What I'd do is something akin to a custom resource in which you...
build_resource
to template them all into your temp directory. build_resource
creates a resource, but doesn't put it in the resource_collection, so when you run it, it won't trigger the custom resource to have "updated". Trigger them all manually with .run_action(:create)
.httpd -t
on that temp directory. Note that on debian-ish distros this is apache2 -t
templates
normally, but if not, throw a Chef::Exceptions::ValidationFailed
exceptionThen you're not reaching into the backup directory or ever fucking with live configs if they don't work. This is similar, but much less complicated, than when I do in fb_network_scripts or whatever it's called these days.
Don't feel obligated to do this, not sure what Naomi has you on, but if you're interested and want to do this, I can definitely review it.
No obligation; I was just toying around with different ideas and learning chef in the process.
@jaymzh I landed https://github.com/facebook/chef-cookbooks/commit/913517dcda6305d25d72fc4201df77d31f304436. that is currently rolled to shard 0. lmk if anything catches your eye that warrants an iteration.
I left some comments for ya. Thanks for the heads up!
We should run
httpd -t
in a validator when rendering config files in templates