Name: foodcritic-rules
Owner: Etsy, Inc.
Description: Etsy's foodcritic rules
Created: 2012-05-15 11:07:07.0
Updated: 2017-09-26 23:47:10.0
Pushed: 2017-04-18 04:43:29.0
Homepage: null
Size: 176
Language: Ruby
GitHub Committers
| User | Most Recent Commit | # Commits |
|---|
Other Committers
| User | Most Recent Commit | # Commits |
|---|
================
These are the foodcritic rules we've written here at Etsy to enforce rules on our cookbooks. Some of these rules are best practices we've identified for our particular use case, and some were put in place as remediation items after issues we experienced.
Once you've cloned the repo from github, you can run foodcritic using the following options to test your cookbooks against these rules:
critic -t etsy,FC023 -I <rules.rb> cookbooks
You should make sure that
Note that we're also checking against the built in foodcritic rule FC023 (Prefer conditional attributes). This is to complement the functionality in rule ETSY004 (see description below)
All of the below rules have the “style” tag as they are best practices and recommendations from our experience rather than rules to detect things which might break your cookbooks.
We've had several instances of new packages being put into our yum repository and rolled out to servers before we intended due to the use of the :upgrade action in “package” and “yum_package” resources. This rule will detect when :upgrade has been used instead of :install. An optional whitelist is included at the top of the file to allow you to relax this rule manually for specific packages.
The whitelisted package list is defined at the top of rules.rb, and defaults to empty:
upgrade_whitelist = []
For example, this block would trip this rule:
age "sendmail" do
tion :upgrade
This rule detects when Git commands are run as bash commands inside of an “execute” block, rather than using the “git” resource. We alert on this behaviour as running these commands inside “execute” blocks gives you much less visibility of the commands' output and status.
For example, this block would trip this rule:
ute "checkout" do
mmand "git clone git://github.com/foo/bar.git"
d "/home/me"
This rule detects when wget or curl commands are run as bash commands inside of an “execute” block, rather than using the “remote_file” resource. We alert on this behaviour as running these commands inside “execute” blocks gives you much less visibility of the commands' output and status.
For example, this block would trip this rule:
ute "fetch_file" do
command "wget -O /tmp/foo.tar.gz http://me.mycorp.com/files/foo.tar.gz"
not_if "test -f /tmp/foo.tar.gz"
This rule detects when an “execute” block is defined without specifying a conditional (ie not_if or only_if) or action :nothing. This is to avoid the presence of resources which run on every chef run, whether they need to or not. Please note, this rule does not identify “execute” blocks which are themselves wrapped in a conditional, ie an if statement. For this reason, we recommend that you also use the built in foodcritic rule FC023, which detects this case. Rule FC023 recommends that you always use conditionals inside your resources instead of wrapping them, and we'd concur with this.
For example, this block would trip this rule:
ute "fetch_file" do
command "wget -O /tmp/foo.tar.gz http://me.mycorp.com/files/foo.tar.gz"
As would this block, even though there is technically a conditional wrapping the resource.
File.exists?("/tmp/foo.tar.gz")
execute "fetch_file" do
command "wget -O /tmp/foo.tar.gz http://me.mycorp.com/files/foo.tar.gz"
end
However, the above block would subsequently be flagged by rule FC023.
This rule identifies when a recipe might send “action :restart” to a service we've identified as a “core” service. We've had instances in the past of chef changes causing hard restarts of services such as memcached or httpd, which can have unexpected side effects. For this reason, we alert if any restart notifications are sent to a defined list of “core” services. Restart notifications sent to any services not in this list will not cause this rule to trip.
The core services list is defined at the top of rules.rb:
eservices = ["httpd", "mysql", "memcached", "postgresql-server"]
For example, this block would trip this rule:
book_file "/etc/httpd/conf.d/myvhost.conf" do
urce "myvhost.conf"
de 0644
ner "root"
oup "root"
tifies :restart, resources(:service => "httpd")
Wheras this block would not (as we're using reload instead of restart):
book_file "/etc/httpd/conf.d/myvhost.conf" do
urce "myvhost.conf"
de 0644
ner "root"
oup "root"
tifies :reload, resources(:service => "httpd")
And neither would this one (as sendmail is not a core service):
late "/etc/mail/sendmail.mc" do
urce "sendmail.mc.erb"
ner "root"
oup "root"
de 00644
tifies :restart, resources(:service => "sendmail")
This rule detects when an “execute” block is using a shell command when Chef already provides a built in resource with equivalent functionality.
The list of commands to check for is defined at the top of rules.rb:
ecommands = ["yum -y", "yum install", "yum reinstall", "yum remove", "mkdir", "useradd", "usermod", "touch"]
For example, this block would trip this rule:
ute "reinstall_pear" do
mmand "yum -y reinstall php-pear"
This rule identifies when a recipe is installing a package for something we've identified as a “core” service without specifying a specific version number to install. We've had instances in the past of divergence occurring when action :install is used by itself, and a new package has been added to the repository resulting in new servers installing the new version while old servers are still using the previous version.
The list of package names to check for is defined at the top of rules.rb:
eservicepackages = ["httpd", "Percona-Server-server-51", "memcached", "postgresql-server"]
For example, this block would trip this rule:
age "httpd" do
tion :install