Foodcritic

A lint tool for your Opscode Chef cookbooks.

Foodcritic has two goals:

  • To make it easier to flag problems in your Chef cookbooks that will cause Chef to blow up when you attempt to converge. This is about faster feedback. If you automate checks for common problems you can save a lot of time.

  • To encourage discussion within the Chef community on the more subjective stuff - what does a good cookbook look like? Opscode have avoided being overly prescriptive which by and large I think is a good thing. Having a set of rules to base discussion on helps drive out what we as a community think is good style.

Prerequisites

Foodcritic runs on Ruby (MRI) 1.9.2+ which depending on your workstation setup may be a more recent version of Ruby than you have installed. The Ruby Version Manager (RVM) is a popular choice for running multiple versions of ruby on the same workstation, so you can try foodcritic out without running the risk of damaging your main install.

Installing RVM

RVM can be installed from the current development code on the projects GitHub - however if you are just getting started with RVM I would recommend installing the latest stable version with the stable argument. The full instructions for installing RVM can be found here:

See also this blog post from Michal Papis for more information:

Installing Ruby 1.9.3

With RVM installed successfully you can now use it to install MRI 1.9.3:

$ rvm install ruby-1.9.3

Installing foodcritic

Ok - now we can move on to installing foodcritic itself. Foodcritic is distributed as a Rubygem - run the following commands to install it:

$ rvm use 1.9.3
$ gem install foodcritic

Great - that’s it. You should now find you have a foodcritic command on your PATH. Run foodcritic to see what arguments it supports:

foodcritic [cookbook_path]
    -r, --[no-]repl                  Drop into a REPL for interactive rule editing.
    -t, --tags TAGS                  Only check against rules with the specified tags.
    -f, --epic-fail TAGS             Fail the build if any of the specified tags are matched.
    -C, --[no-]context               Show lines matched against rather than the default summary.
    -I, --include PATH               Additional rule file path(s) to load.
    -S, --search-grammar PATH        Specify grammar to use when validating search syntax.
    -V, --version                    Display version.

Now for the fun part. Try running it against your favourite cookbook.

style attributes

Opscode recommend that you use strings rather than symbols when referencing node attributes. See this explanation from @jtimberman.

Uses symbols to reference attributes

This example would match the FC001 rule because the version attribute has been referenced with a symbol.

Modified version

This modified example would not match the FC001 rule:

# Don't do this
package "mysql-server" do
  version node[:mysql][:version]
  action :install
end
package "mysql-server" do
  version node['mysql']['version']
  action :install
end

style strings

When you declare a resource in your recipes you frequently want to reference dynamic values such as node attributes. This warning will be shown if you are unnecessarily wrapping an attribute reference in a string.

Unnecessary string interpolation

This example would match the FC002 rule because the version attribute has been unnecessarily quoted.

Modified version

This modified example would not match the FC002 rule:

# Don't do this
package "mysql-server" do
  version "#{node['mysql']['version']}"
  action :install
end
package "mysql-server" do
  version node['mysql']['version']
  action :install
end

portability solo

Chef Server extends the feature-set of a Chef deployment and is probably the most popular configuration. It is also possible to run the Chef client in a standalone mode with Chef Solo. Where your cookbooks make use of features that only exist in a Chef Server based setup you should check whether you are running in solo mode.

Does not check for Chef Solo

This example would match the FC003 rule because it does not check if it is running in Chef Solo before using search which is a server-specific feature.

Modified version

This modified example would not match the FC003 rule:

nodes = search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")
if Chef::Config[:solo]
  Chef::Log.warn("This recipe uses search. Chef Solo does not support search.")
else
  nodes = search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")
end

style services

This warning is shown if you are starting or stopping a service using the Chef execute resource rather than the more idiomatic service resource. You can read more about the service resource here:

Uses execute to control a service

This example would match the FC004 rule because it uses execute for service control. There is no reason to use execute because the service resource exposes the start_command attribute to give you full control over the command issued.

Modified version

This modified example would not match the FC004 rule:

# Don't do this
execute "start-tomcat" do
  command "/etc/init.d/tomcat6 start"
  action :run
end
service "tomcat" do
  action :start
end

style

When writing Chef recipes you have the full power of Ruby at your disposal. One of the cases where this is helpful is where you need to declare a large number of resources that only differ in a single attribute - the canonical example is installing a long list of packages.

Unnecessarily repetitive

This example matches the FC005 rule because all the resources of type package differ only in a single attribute - the name of the package to be upgraded. This rule is very simple and looks only for resources that all differ in only a single attribute. For example - if only one of the packages specified the version then this rule would not match.

Modified version

This modified example would not match the FC005 rule. It takes advantage of the fact that Chef processes recipes in two distinct phases:

Don’t worry about changing your recipe if it already does what you want - the amount of Ruby syntactic sugar to apply is very much a matter of personal taste. Note that this rule also isn’t clever enough yet to detect if your resources are wrapped in a control structure and not suitable for ‘rolling up’ into a loop.

# You could do this
package "erlang-base" do
  action :upgrade
end
package "erlang-corba" do
  action :upgrade
end
package "erlang-crypto" do
  action :upgrade
end
package "rabbitmq-server" do
  action :upgrade
end
# It's shorter to do this
%w{erlang-base erlang-corba erlang-crypto rabbitmq-server}.each do |pkg|
  package pkg do
    action :upgrade
  end
end

correctness files

When setting file or directory permissions via the mode attribute you should either quote the octal number or ensure it is specified to five digits. Otherwise the permissions that are set after Ruby coerces the number may not match what you expect.

File mode won't be interpreted correctly
Modified versions

These modified examples would not match the FC006 rule:

# Don't do this
directory "/var/lib/foo" do
  owner "root"
  group "root"
  mode 644
  action :create
end
# This is ok
directory "/var/lib/foo" do
  owner "root"
  group "root"
  mode "644"
  action :create
end

# And so is this
directory "/var/lib/foo" do
  owner "root"
  group "root"
  mode 00644
  action :create
end

correctness metadata

This warning is shown when you include a recipe that is not in the current cookbook and not defined as a dependency in your cookbook metadata. This is potentially a big problem because things will blow up if the necessary dependency cannot be found when Chef tries to converge your node. For more information refer to the Chef wiki metadata page:

The fix is to declare the cookbook of the recipe you are including as a dependency in your metadata.rb file.

You may also see this warning if foodcritic has not been able to infer the name of your cookbook correctly when the cookbook directory does not match the name of the cookbook specified in the include.

Example depency on another cookbook

Assuming you have a recipe that had the following line:

Adding metadata dependency for Chef

Then to remove this warning you would add the apache2 cookbook as a dependency to your own cookbook metadata in the metadata.rb file at the root of your cookbook.

include_recipe "apache2::default"
depends "apache2"

style metadata

This warning is shown if you used knife cookbook create to create a new cookbook and didn’t override the maintainer and maintainer email. You need to set these to real values in metadata.rb or run knife again with the real values.

Maintainer metadata is boilerplate default
Modified version

This modified example would not match the FC008 rule:

# Don't do this
maintainer "YOUR_COMPANY_NAME"
maintainer_email "YOUR_EMAIL"
license "All rights reserved"
description "Installs/Configures example"
long_description IO.read(File.join(File.dirname(__FILE__), 'README.rdoc'))
version "0.0.1"
maintainer "Example Ltd"
maintainer_email "postmaster@example.com"
license "All rights reserved"
description "Installs/Configures example"
long_description IO.read(File.join(File.dirname(__FILE__), 'README.rdoc'))
version "0.0.1"

correctness

This warning is likely to mean that your recipe will fail to run when you attempt to converge. Your recipe may be syntactically valid Ruby, but the attribute you have attempted to set on a built-in Chef resource is not recognised. This is commonly a typo or you need to check the documentation to see what the attribute you are trying to set is called:

Resource with an unrecognised attribute

This example matches the FC009 rule because punter is not a recognised attribute for the file resource.

Modified version

Checking the documentation we can see the correct attribute is owner.

# Don't do this
file "/tmp/something" do
  punter "root"
  group "root"
  mode "0755"
  action :create
end
file "/tmp/something" do
  owner "root"
  group "root"
  mode "0755"
  action :create
end

correctness search

The search syntax used is not recognised as valid Lucene search criteria. This is commonly because you have made a typo or are not escaping special characters in the query syntax.

Note that this rule will not identify syntax errors in searches composed of subexpressions. It checks only for literal search strings.

Unescaped search syntax

This example matches the FC010 rule because search metacharacters - in this case the square brackets - have not been properly escaped.

Modified version

With the characters escaped this will no longer match the rule.

# Don't do this
search(:node, 'run_list:recipe[foo::bar]') do |matching_node|
  puts matching_node.to_s
end
search(:node, 'run_list:recipe\[foo\:\:bar\]') do |matching_node|
  puts matching_node.to_s
end

style readme

The Opscode Community site will now render your cookbook README documentation inline - see this example for the mysql cookbook.

Your README needs to be in Markdown format for this to work. This rule will match any cookbook that does not have a README.md file in the root directory.

style readme

Writing cookbook documentation in RDoc has been deprecated in favour of Markdown format. This rule will match any cookbook that has a README.rdoc file in the root directory.

style files

This warning means that you have hard-coded a file download path in your cookbook to a temporary directory. This can be a problem on boxes built with a small /tmp mount point. Chef has its own configuration option file_cache_path you should use instead:

Downloading to a hard-coded temp directory

This example matches the FC013 rule because it hard-codes the download path to /tmp.

Modified version

To remove this warning use the configured file_cache_path:

# Don't do this
remote_file "/tmp/large-file.tar.gz" do
  source "http://www.example.org/large-file.tar.gz"
end
remote_file "#{Chef::Config[:file_cache_path]}/large-file.tar.gz" do
  source "http://www.example.org/large-file.tar.gz"
end

style libraries

Your cookbook has a fairly long ruby_block resource. Long ruby_block resources are often candidates for extraction to a separate module or class under the libraries directory.

style definitions lwrp

Chef definitions are an older approach to creating a higher-level abstraction for a group of resources. Unlike LWRPs they are not first class resources and cannot receive notifications. You should prefer LWRPs for new development.

correctness lwrp

This warning means that the LWRP does not declare a default action. You should normally define a default action on your resource to avoid confusing users. Most resources have an intuitive default action.

Resource without a default action

This example matches the FC016 rule because it does not declare a default action.

Modified version

With a default action specified this warning will no longer be displayed.

# Don't do this
actions :create
actions :create

# Chef 0.10.10 or greater
default_action :create

# In earlier versions of Chef the LWRP DSL doesn't support specifying
# a default action, so you need to drop into Ruby.
def initialize(*args)
  super
  @action = :create
end

correctness lwrp

This warning means that the LWRP will not currently trigger notifications to other resources. This can be a source of difficult to track down bugs.

Provider that does not send notifications

This example matches the FC017 rule because it does not mark that its state has changed and will therefore not send notifications.

Modified version

The call to updated_by_last_action ensures that notifications will work correctly.

# Don't do this
action :create do
  # create action implementation
end
action :create do
  # create action implementation

  # My state has changed so I'd better notify observers
  new_resource.updated_by_last_action(true)
end

style lwrp deprecated

Provider uses deprecated syntax

This example matches the FC018 rule because it uses the old syntax for indicating it has been updated.

Modified version

This example uses the newer syntax and will not raise the warning.

# Don't do this
action :create do
  # create action implementation

  # My state has changed so I'd better notify observers, but I'm using
  # a deprecated syntax
  new_resource.updated = true
end

# Also don't do this
action :create do
  # create action implementation

  # My state has changed so I'd better notify observers, but I'm using
  # a deprecated syntax
  @updated = true
end
action :create do
  # create action implementation

  # My state has changed so I'd better notify observers
  new_resource.updated_by_last_action(true)
end

style attributes

Node attributes can be accessed in multiple ways in Chef. This warning is shown when a cookbook is not consistent in the approach it uses to access attributes. It is not displayed for variations between cookbooks.

Recipe mixes symbols and strings for accessing node attributes
Modified version
# Don't do this
node[:apache][:dir] = '/etc/apache2'

directory node['apache']['dir'] do
  owner 'apache'
  group 'apache'
  action :create
end
node['apache']['dir'] = '/etc/apache2'

directory node['apache']['dir'] do
  owner 'apache'
  group 'apache'
  action :create
end

correctness

When specifying an only_if or not_if attribute for conditional execution of your resource you have the option of either a command string or Ruby block. This warning is shown when your condition is passed as a string but looks like a Ruby block.

Passes Ruby block condition as string
Modified version
# Don't do this
execute "my_random_command" do
  action :run
  not_if "::File.directory?(node[:foo])"
end
execute "my_random_command" do
  action :run
  not_if { ::File.directory?(node[:foo]) }
end

correctness lwrp

A change introduced in Chef 0.10.6 means that conditions may not work as expected for resources redeclared with the same name. If your LWRP defines a resource and that resource:

  • Has an associated condition which references a resource attribute. AND
  • The resource has a fixed name.

Then you will likely find that only the first resource will be applied. See this ticket for more background:

Resource condition will be evaluated only once

Because the feed_pet resource will have the same name across all instances of your LWRP, the condition will only be checked for the first resource.

Modified version

By making the resource name change for each unique instance of our LWRP instance we avoid this behaviour.

# Don't do this
action :feed do
  execute "feed_pet" do
    command "echo 'Feeding: #{new_resource.name}'; touch '/tmp/#{new_resource.name}'"
    not_if { ::File.exists?("/tmp/#{new_resource.name}")}
  end
end
action :feed do
  execute "feed_pet_#{new_resource.name}" do
    command "echo 'Feeding: #{new_resource.name}'; touch '/tmp/#{new_resource.name}'"
    not_if { ::File.exists?("/tmp/#{new_resource.name}")}
  end
end

correctness

A change introduced in Chef 0.10.6 means that conditions may not work as expected for resources declared within a loop. If your recipe defines a resource and that resource:

Then you will likely find that only the first resource will be applied. See this ticket for more background:

Resource condition will be evaluated only once

Because the feed_pet resource will have the same name for every iteration of the loop, the condition will only be checked for the first resource.

Modified version

By making the resource name change for each iteration of the loop we avoid this behaviour.

# Don't do this
%w{rover fido}.each do |pet_name|
  execute "feed_pet" do
    command "echo 'Feeding: #{pet_name}'; touch '/tmp/#{pet_name}'"
    not_if { ::File.exists?("/tmp/#{pet_name}")}
  end
end
%w{rover fido}.each do |pet_name|
  execute "feed_pet_#{pet_name}" do
    command "echo 'Feeding: #{pet_name}'; touch '/tmp/#{pet_name}'"
    not_if { ::File.exists?("/tmp/#{pet_name}")}
  end
end

style

This warning means you have surrounded a resource with an if or unless rather than defining the condition directly on the resource itself. Note that this warning is only raised for single resources as you could reasonably enclose multiple resources in a condition like this for brevity.

Resource enclosed in a condition

This example matches the FC023 rule because it encloses a rule within a condition, rather than using the built-in Chef not_if or only_if conditional execution attributes.

Modified version

You can avoid the warning above with more idiomatic Chef that specifies the condition above as an attribute on the resource:

# Don't do this
if node['foo'] == 'bar'
  service "apache" do
    action :enable
  end
end
service "apache" do
  action :enable
  only_if { node['foo'] == 'bar'}
end

portability

This warning is shown when:

  • you have a conditional statement in your cookbook based on the platform of the node
  • and at least two platforms are included as equivalent in your conditional
  • and the conditional does not include a platform known to belong to the same family

If you are using Ohai 0.6.12 or greater you should probably use platform_family instead. Otherwise for the greatest portability consider adding the missing platforms to your conditional.

Case statement has a subset of platform flavours

This example matches the FC024 rule because it includes a case statement that matches more than one flavour of a platform family but omits other popular flavours from the same family.

Modified version

This warning is no longer raised when the other common equivalent RHEL-based distributions have been added to the when.

# The RHEL platforms branch below omits popular distributions
# including Amazon Linux.
case node[:platform]
  when "debian", "ubuntu"
    package "foo" do
      action :install
    end
  when "centos", "redhat"
    package "bar" do
      action :install
    end
  end
end
case node[:platform]
  when "debian", "ubuntu"
    package "foo" do
      action :install
    end
  when "centos", "redhat", "amazon", "scientific"
    package "bar" do
      action :install
    end
  end

style deprecated

This warning is shown if:

  • you have a cookbook that installs a Rubygem for use from Chef
  • the cookbook uses the compile-time gem install trick which is deprecated from Chef 0.10.10 and is replaced by the first class chef_gem resource.
Manual compile-time installation

This example matches the FC025 rule because it uses the older approach for installing a gem so that it is available in the current run.

Modified version

Use chef_gem to install the gem to avoid this warning.

r = gem_package "mysql" do
  action :nothing
end

r.run_action(:install)
Gem.clear_paths
chef_gem "mysql"

How can I prevent certain warnings from being shown?

You can include or exclude rules to apply using the --tags argument to foodcritic. The tag expressions you can specify follow the same syntax as that used in Cucumber - see the Cucumber wiki page below for details.

For example if you don’t care about style warnings you could run foodcritic like so:

$ foodcritic --tags ~style cookbooks

Each rule has an implicit tag so you can exclude individual rules by rule code:

$ foodcritic --tags ~FC002 cookbooks

Foodcritic dies with ‘cannot load such file – readline (LoadError)’

This is because your current Ruby has not been installed with Readline. If you are using RVM you can follow the instructions on the RVM site to resolve this:

I’m using foodcritic in my CI build or commit hook and I want it to fail if errors are shown

You can tell foodcritic which warnings you want it to fail the build on with the -f option. See the Failing the build section for more detail.

To manually add a new job to Jenkins to check your cookbooks with foodcritic do the following:

  1. Ensure you have Ruby 1.9.2+ and the foodcritic gem installed on the box running Jenkins.

  2. You’ll probably need to install the Git plugin. In Jenkins select “Manage Jenkins” -> “Manage Plugins”. Select the “Available” tab. Check the checkbox next to the Git Plugin and click the “Install without restart” button.

  3. In Jenkins select “New Job”. Enter a name for the job “my-cookbook”, select “Build a free-style software project” and click “OK”.

  4. On the resulting page select “Git” under “Source Code Management” and enter the URL for your repo.

  5. Check the checkbox “Poll SCM” under “Build Triggers”.

  6. Click “Add Build Step” -> “Execute shell” under “Build”. This is where we will call foodcritic.

  7. Assuming you are using rvm enter the following as the command:

    #!/usr/bin/env rvm-shell 1.9.3
    foodcritic .
  8. Click “Save”.

  9. Cool, we’ve created your new job. Now lets see if it works. Click “Build Now” on the left-hand side.

  10. You can click the build progress bar to be taken directly to the console output.

  11. After a moment you should see that the build has been successful and foodcritic warnings (if any) are shown in your console output.

Yes, for maximum goodness you should be automating all this with Chef. :-)

For more information refer to the instructions for building a “free-style software project” here:

See also this blog post about rvm-shell which ensures you have the right version of Ruby loaded when trying to build with foodcritic:

Failing the build

The above is a start, but we’d also like to fail the build if there are any warnings that might stop the cookbook from working.

CI is only useful if people will act on it. Lets start by only failing the build when there is a correctness problem that would likely break our Chef run. We’ll continue to have the other warnings available for reference in the console log but only correctness issues will fail the build.

  1. Select the “my-cookbook” job in Jenkins and click “Configure”.

  2. Scroll down to our “Execute shell” command and change it to look like the following:

    #!/usr/bin/env rvm-shell 1.9.3
    foodcritic -f correctness .
  3. Click “Save” and then “Build Now”.

More complex expressions

Foodcritic supports more complex expressions with the standard Cucumber tag syntax. For example:

#!/usr/bin/env rvm-shell 1.9.3
foodcritic -f any -f ~FC014 .

Here we use any to fail the build on any warning, but then use the tilde ~ to exclude FC014. The build will fail on any warning raised, except FC014.

You can find more detail on Cucumber tag expressions at the Cucumber wiki:

Tracking warnings over time

The Jenkins Warnings plugin can be configured to understand foodcritic output and track your cookbook warnings over time.

  1. You’ll need to install the Warnings plugin. In Jenkins select “Manage Jenkins” -> “Manage Plugins”. Select the “Available” tab. Check the checkbox next to the Warnings Plugin and click the “Install without restart” button.

  2. From “Manage Jenkins” select “Configure System”. Scroll down to the “Compiler Warnings” section and click the “Add” button next to “Parsers”.

  3. Enter “Foodcritic” in the Name field.

  4. Enter the following regex in the “Regular Expression” field:

    ^(FC[0-9]+): (.*): ([^:]+):([0-9]+)$
  5. Enter the following Groovy script into the “Mapping Script” field:

    import hudson.plugins.warnings.parser.Warning
    
    String fileName = matcher.group(3)
    String lineNumber = matcher.group(4)
    String category = matcher.group(1)
    String message = matcher.group(2)
    
    return new Warning(fileName, Integer.parseInt(lineNumber), "Chef Lint Warning", category, message);
  6. To test the match, enter the following example message in the “Example Log Message” field:

    FC001: Use strings in preference to symbols to access node attributes: ./recipes/innostore.rb:30
  7. Click in the “Mapping Script” field and you should see the following appear below the Example Log Message:

    One warning found
    file name: ./recipes/innostore.rb
    line number: 30
    priority: Normal Priority
    category:  FC001
    type: Chef Lint Warning
    message: Use strings in prefe[...]ols to access node attributes
  8. Cool, it’s parsed our example message successfully. Click “Save” to save the parser.

  9. Select the “my-cookbook” job in Jenkins and click “Configure”.

  10. Check the checkbox next to “Scan for compiler warnings” underneath “Post-build Actions”.

  11. Click the “Add” button next to “Scan console log” and select our “Foodcritic” parser from the drop-down list.

  12. Click the “Advanced…” button and check the “Run always” checkbox.

  13. Click “Save” and then “Build Now”.

  14. Add the bottom of the console log you should see something similar to this:

    [WARNINGS] Parsing warnings in console log with parsers [Foodcritic]
    [WARNINGS] Foodcritic : Found 48 warnings.
  15. Click “Back to Project”. Once you have built the project a couple of times the warnings trend will appear here.

There are two approaches you can take to implementing a new rule.

  • You can create a local rule specific to your organisation. This is often the easiest way to get started.

  • You can extend Foodcritic with a new rule. Do this if you are intending to contribute your rule back so people outside your organisation can use it.

Local rules

Your local rules directory should have a Gemfile defined to express the versions of foodcritic that your rule is known to work with. This is to avoid API changes breaking your rules. Foodcritic follows the RubyGems RationalVersioningPolicy.

$ mkdir local-rules && cd local-rules
$ echo "gem 'foodcritic', '~> 1.0.0'" > Gemfile
$ gem install bundler
$ bundle install --binstubs
$ cat > rules.rb <<EOF
rule "MYORG001", "My custom rule" do
  tags %w{style attributes}
  recipe do |ast|
    attribute_access(ast, :type => :string).map{|ar| match(ar)}
  end
end
EOF
$ cd ..
$ local-rules/bin/foodcritic -t MYORG001 -I local-rules cookbooks

Contributing a rule

In this contrived example we are going to add a new rule to raise a warning if a recipe or provider declares a log resource that contains the word ‘password’.

First, a word about how Foodcritic works. The rules are defined in a simple DSL here:

Each rule has a block that defines the matching logic for the rule. The block accepts a Nokogiri document that contains the Ripper parsed representation of your recipe. Ripper is a very useful module that ships with recent versions of Ruby allowing you to walk the tree of symbols representing your program without actually having to eval it.

Grab the source code

It’s easiest to fork the repository first and then clone from the forked version. Browse to the foodcritic repository on GitHub:

Click the ‘Fork’ button in the top right-hand corner. Cool, now you have your own copy of the repo.

$ git clone https://you@github.com/you/foodcritic.git
$ rvm use 1.9.3
$ cd foodcritic
$ git checkout -b log-contains-password
Switched to a new branch 'log-contains-password'
$ bundle install

Run the foodcritic build

Before we make any code changes lets check that the existing code builds successfully on our machine.

$ bundle exec rake

Add a new feature

Lets add a Cucumber feature to describe our new rule with a couple of scenarios:

$ cat > features/123_check_for_log_with_password.feature <<EOF
Feature: Check for log statements that contain passwords

In order to prevent disclosure of credentials
As a developer
I want to identify when a log statement includes the word password

Scenario: Log includes password
  Given a recipe that declares a log resource including the word password
  When I check the cookbook
  Then the log resource includes password warning 123 should be displayed

Scenario: Log does not include password
  Given a recipe that declares a log resource that does not include the word password
  When I check the cookbook
  Then the log resource includes password warning 123 should not be displayed
EOF

Run Cucumber

Each of the lines in the scenario needs a matching step definition . If we run cucumber against our new feature:

$ bundle exec cucumber features/123_check_for_log_with_password.feature
...
You can implement step definitions for undefined steps with these snippets:

Given /^a recipe that declares a log resource including the word password$/ do
  pending # express the regexp above with the code you wish you had
end

Given /^a recipe that declares a log resource that does not include the word password$/ do
  pending # express the regexp above with the code you wish you had
end

Cucumber is telling us that we need to implement the Given steps. The When and Then steps have already been implemented for us.

Implement a step definition

Lets implement the first Given, the one that creates a recipe that declares a log resource with the word password.

$ cat >> features/step_definitions/cookbook_steps.rb <<EOF

Given 'a recipe that declares a log resource including the word password' do
  write_recipe %q{
    log "The secret password is: password1"
  }
end
EOF

Now lets run Cucumber. We expect it to fail at this stage.

$ bundle exec cucumber features/123_check_for_log_with_password.feature
...
Failing Scenarios:
cucumber features/123_check_for_log_with_password.feature:7 # Scenario: Log includes password

Great, we have a failing test. Now we can set about implementing the rule.

Ensure cucumber will recognise the new warning

In order for Cucumber to correctly recognise the new warning it needs to be added to the WARNINGS hash in support/command_helpers.rb.

$ cat <<EOF | git apply
diff --git a/features/support/command_helpers.rb b/features/support/command_helpers.rb
--- a/features/support/command_helpers.rb
+++ b/features/support/command_helpers.rb
@@ -31,3 +31,4 @@ module FoodCritic
       'FC022' => 'Resource condition within loop may not behave as expected',
-      'FC023' => 'Prefer conditional attributes'
+      'FC023' => 'Prefer conditional attributes',
+      'FC123' => 'Log contains password'
     }
EOF

Implementing the rule

Lets use the very awesome Pry REPL to interactively explore writing a new rule, much as we might use Shef for writing Chef recipes.

Run the build with FC_REPL=true and you will be dropped into the Pry REPL:

$ FC_REPL=true bundle exec cucumber features/123_check_for_log_with_password.feature:7

Add a new rule

First define a new placeholder rule:

pry> rule "FC123", "Log contains password" do
  recipe do |ast|
    binding.pry
  end
end

Check that it has been added to the list of rules:

pry> puts rules

And exit, to be taken to the binding you declared above:

pry> exit

Listing the available DSL methods

A number of pre-canned DSL methods exist to help in writing rules. You can list these by typing:

pry> Api.instance_methods.sort

To see the documentation for one method in particular (here the included_recipes method) type:

pry> show-doc included_recipes

From: foodcritic/lib/foodcritic/api.rb @ line 133:
Number of lines: 4
Owner: FoodCritic::Api
Visibility: public
Signature: included_recipes(ast)

Retrieve the recipes that are included within the given recipe AST.

param [Nokogiri::XML::Node] ast The recipe AST
return [Hash] include_recipe nodes keyed by included recipe name

Write an expression that matches your criteria

To view the XML representation of the AST, type:

pry> puts ast

You can use Nokogiri’s great support for XPath or CSS selectors to match against statements within the tree. Lets try and write a XPath expression that will match only the nodes you are interested in.

pry> puts ast.xpath("//command[ident/@value='log']/descendant::tstring_content[contains(@value, 'password')]")
<tstring_content value="The secret password is: password1">
  <pos line="1" column="5"/>
</tstring_content>

Flagging matches

For foodcritic to report your warning you currently need to specify them as matches. You need to map a node in the AST that has a child pos node which foodcritic can use to display the line number.

pry> ast.xpath("//command[ident/@value='log']/descendant::tstring_content[contains(@value, 'password')]").map{|n| match(n)}
=> [{:matched=>"tstring_content", :line=>"1", :column=>"5"}]

Update the rule definition

Now we have what we think is the matching expression lets update the rule definition:

pry> cd ..
pry> reset_rules
pry> rule "FC123", "Log contains password" do
  tags %w{security}
  recipe do |ast|
    ast.xpath("//command[ident/@value='log']/descendant::tstring_content[contains(@value, 'password')]").map{|n| match(n)}
  end
end
pry> exit

And finally re-run the check against the newly redefined rule:

pry> recheck
pry> review
=> FC011: Missing README in markdown format: cookbooks/example/README.md:1
FC123: Log contains password: cookbooks/example/recipes/default.rb:1

pry> exit
pry> exit
    When I check the cookbook                                               # features/step_definitions/cookbook_steps.rb:410
    Then the log resource includes password warning 123 should be displayed # features/step_definitions/cookbook_steps.rb:434

1 scenario (1 passed)
3 steps (3 passed)

Next steps

Once the step definitions have been completed you need to:

  • Think of other scenarios that you need to test. For this example you might consider that Chef allows you to log via direct calls to the Chef logger as well and modify your scenarios and rule definition to deal with this.
  • Run the feature to confirm it passes on both Ruby 1.9.2 and 1.9.3 as there are AST differences between them.
  • Run the complete build again.
  • Push your changes to GitHub and open a pull request.

That’s it - have fun. Thanks!

attribute_access

Find attributes accesses by type.

You might use this method to enforce local style rules on how attributes are accessed.

# All references to attributes using strings
# For example: node['foo']
attribute_access(ast, :type => :string)

# All references to attributes using symbols
# For example: node[:foo]
attribute_access(ast, :type => :symbol)

# All references to attributes using dots (vivified methods)
# For example: node.foo
attribute_access(ast, :type => :symbol)
categorytypenamedescription
param Nokogiri::XML::Node ast

The AST of the cookbook recipe to check

param Hash options

The options hash (see allowed values below)

option Symbol :type

The approach used to access the attributes. One of :string, :symbol or :vivified.

option Boolean :ignore_calls

Exclude attribute accesses that mix strings/symbols with dot notation. Defaults to false.

return Array

The matching nodes if any

checks_for_chef_solo?

Does the specified recipe check for Chef Solo?

You can use this to check for the portability of the recipe between Chef Server and Chef Solo.

# Returns true if the recipe checks for Chef Solo before using
# server-specific functionality.
checks_for_chef_solo?(ast)
categorytypenamedescription
param Nokogiri::XML::Node ast

The AST of the cookbook recipe to check

return Boolean

True if there is a test for Chef::Config[:solo] in the recipe

chef_dsl_methods

The set of methods in the Chef DSL.

You can use this to see if a method reference is part of the Chef DSL or defined in a cookbook.

# Is search a method in the Chef DSL?
chef_dsl_methods.include?(:search)
=> true
categorytypenamedescription
return Array

Array of method symbols

chef_solo_search_supported?

Is the chef-solo-search library available?

Will return true if any cookbook in the cookbook tree relative to the specified recipe includes the chef-solo-search library. You can use this to see if search is available in Chef Solo mode.

# True if chef_solo_search is supported
chef_solo_search_supported?('foo/recipes/default.rb')
categorytypenamedescription
param String recipe_path

The path to the current recipe

return Boolean

True if the chef-solo-search library is available.

cookbook_name

The name of the cookbook containing the specified file.

You can use this method in rules that need to work out if recipe code refers to the current cookbook: for example when looking at included_recipe statements or LWRP usage.

cookbook_name('foo/recipes/default.rb')
=> "foo"
categorytypenamedescription
param String file

The file in the cookbook

return String

The name of the containing cookbook

declared_dependencies

The dependencies declared in cookbook metadata.

You can use this to check if all dependencies have been declared correctly or to find all cookbooks that share a common dependency.

ast = read_ast('postgresql/metadata.rb')
declared_dependencies(ast)
=> ["openssl"]
categorytypenamedescription
param Nokogiri::XML::Node ast

The metadata rb AST

return Array

List of cookbooks depended on

file_match

Create a match for a specified file. Use this if the presence of the file triggers the warning rather than content.

This is an alternative to match where you don’t have a specific AST element to associate the warning with. The call to file_match will typically be the last line in your rule.

file_match('foo/recipes/default.rb')
=> {:filename=>"foo/recipes/default.rb",
 :matched=>"foo/recipes/default.rb",
 :line=>1,
 :column=>1}
categorytypenamedescription
param String file

The filename to create a match for

return Hash

Hash with the match details

find_resources

Find Chef resources of the specified type.

Note that currently this method does not return blockless resources.

# Returns all resources in the AST
find_resources(ast)

# Returns just the packages
find_resources(ast, :type => :package)
categorytypenamedescription
param Nokogiri::XML::Node ast

The AST of the cookbook recipe to check

param Hash options

The options hash (see allowed values below)

option Symbol :type

The type of resource to look for (or :any for all resources)

return Array

AST nodes of Chef resources.

included_recipes

Retrieve the recipes that are included within the given recipe AST.

You can use this method to determine (and validate) recipe runtime dependencies.

# Find all include_recipe statements, discarding the AST nodes to just
# show the recipe names.
included_recipes(ast).keys
=> ["postgresql::client"]
categorytypenamedescription
param Nokogiri::XML::Node ast

The recipe AST

return Hash

Hash keyed by included recipe name where the value is the AST node
of the include_recipe statement.

literal_searches

Searches performed by the specified recipe that are literal strings. Searches with a query formed from a subexpression will be ignored.

ast = read_ast('zenoss/recipes/server.rb')
literal_searches(ast).size
=> 3
categorytypenamedescription
param Nokogiri::XML::Node ast

The AST of the cookbook recipe to check

return Array

The matching nodes

match

Create a match from the specified node.

Return matches when a rule has matched against a recipe. A call to match is typically the last line of your rule.

Ensure that the AST node you are passing to this method has a descendant pos node so that the match can be associated with a line in the file.

# You will frequently use map to apply the match function to an array of
# nodes that you consider matches for your rule.
attribute_access(ast, :type => :string).map{|s| match(s)}
categorytypenamedescription
param Nokogiri::XML::Node node

The node to create a match for

return Hash

Hash with the matched node name and position with the recipe

os_command?

Does the provided string look like an Operating System command? This is a rough heuristic to be taken with a pinch of salt.

categorytypenamedescription
param String str

The string to check

return Boolean

True if this string might be an OS command

read_ast

Read the AST for the given Ruby source file.

Many of the other functions documented here take an ast as an argument. You can also use Nokogiri’s support querying the AST with XPath or CSS to implement your own rules.

# Normally the recipe AST will be passed in to your rule without you
# needing to use read_ast.

# However if you need to you can read in the AST for a cookbook recipe
# directly.
ast = read_ast('apache2/recipes/default.rb')
categorytypenamedescription
param String file

The file to read

return Nokogiri::XML::Node

The recipe AST

resource_attribute

Retrieve a single-valued attribute from the specified resource.

# Given resource is a package
resource_attribute(resource, 'action')
=> :install
categorytypenamedescription
param Nokogiri::XML::Node resource

The resource AST to lookup the attribute under

param String name

The attribute name

return String

The attribute value for the specified attribute

resource_attribute?

Is the specified attribute valid for the type of resource? Note that this method will return true if the resource_type is not recognised.

resource_attribute?(:file, :mode) 
=> true

resource_attribute?(:file, :size) 
=> false

# If the resource is not a Chef built-in then the attribute is always
# valid
resource_attribute?(:my_custom_resource, :whatever) 
=> true
categorytypenamedescription
param Symbol resource_type

The type of Chef resource

param Symbol attribute_name

The attribute name

return Boolean

True if the attribute is valid for this type of resource

resource_attributes

Retrieve all attributes from the specified resource.

Use this method for convenient access to the resource attributes without needing to query the AST.

resource_attributes(resource)
=> {:name=>"zenoss", "arch"=>"kernel", "action"=>:install}
categorytypenamedescription
param Nokogiri::XML::Node resource

The resource AST

return Hash

The resource attributes

resource_attributes_by_type

Retrieve the attributes as a hash for all resources of a given type.

Use this if you want to compare the attributes and values used by resources of the same type.

# The values of the Hash (ignored here) are arrays of resource ASTs.
resource_attributes_by_type(ast).keys.sort
=> ["apt_package",
 "apt_repository",
 "execute",
 "package",
 "ruby_block"]
categorytypenamedescription
param Nokogiri::XML::Node ast

The recipe AST

return Hash

Resources keyed by type, with an array for each

resource_name

Retrieve the name attribute associated with the specified resource.

resource_name(resource)
=> "zenoss"
categorytypenamedescription
param Nokogiri::XML::Node resource

The resource AST to lookup the name attribute under

return String

The name attribute value

resource_type

Return the type, e.g. ‘package’ of a given resource.

You could use this if you wanted to take different action in your rule based on the resource type.

resource_type(resource)
=> "yum_package"
categorytypenamedescription
param Nokogiri::XML::Node resource

The resource AST

return String

The type of resource

resources_by_type

Retrieve all resources of a given type.

The key of the hash is the type of resource (as a string). The value is an array of Hashes.

resources_by_type(ast).keys
=> ["yum_key",
 "yum_repository",
 "package",
 "service",
 "yum_package",
 "apt_repository",
 "apt_package"]
categorytypenamedescription
param Nokogiri::XML::Node ast

The recipe AST

return Hash

The matching resources

ruby_code?

Does the provided string look like ruby code? This does not evaluate the expression, instead only checking that it appears syntactically valid.

You can use this method to check that ruby_block resources and recipes themselves look like Ruby code.

# Lots of strings are well-formed Ruby statements, including some strings
# you might not expect:
ruby_code?('System.out.println("hello world");')
=> true

# This operating system command doesn't look like valid Ruby but others
# might.
ruby_code?('find -type f -print')
=> false
categorytypenamedescription
param String str

The string to check for rubiness

return Boolean

True if this string could be syntactically valid Ruby

searches

Searches performed by the specified recipe. In contrast to literal_searches this method returns all searches.

You could use this method to identify all searches that search for a particular type of object, or to identify searches that are valid for a particular Chef version.

categorytypenamedescription
param Nokogiri::XML::Node ast

The AST of the cookbook recipe to check.

return Array

The AST nodes in the recipe where searches are performed

standard_cookbook_subdirs

The list of standard cookbook sub-directories.

You can use this method when you need to traverse cookbooks manually yourself in order to determine directories to descend into.

standard_cookbook_subdirs
=> ["attributes",
 "definitions",
 "files",
 "libraries",
 "providers",
 "recipes",
 "resources",
 "templates"]
categorytypenamedescription
return Array

The standard list of directories.

valid_query?

Is this a valid Lucene query?

Use this method when acting on searches in a recipe in order to check that they are valid before continuing with the rest of your rule.

valid_query?('run_list:recipe[foo::bar]')
=> false

valid_query?('run_list:recipe\[foo\:\:bar\]')
=> true
categorytypenamedescription
param String query

The query to check for syntax errors

return Boolean

True if the query is well-formed