Foodcritic / foodcritic

Lint tool for Chef cookbooks.
http://foodcritic.io
MIT License
408 stars 155 forks source link

FC033: False alert because of template naming #96

Closed mrusan closed 11 years ago

mrusan commented 11 years ago

Hello,

we ran foodcritic against our cookbooks and it appears to give a false alert on the FC033 rule.

  template "/etc/profile.d/rails_env.sh" do
    mode         '0744'
    source       "rails_env.erb"
    cookbook     'app_passenger'
    variables(
      :environment => node[:app_passenger][:project][:environment]
    )
  end

The issue is that foodcritic is expecting the source to be named as rails_env.sh.erb but Chef doesn't have this kind of limitation, meaning the source name can be chosen freely - for the template resource to work correct the requirement is only for the file with the given name to be in the right directory and actually exist.

Max

kcbraunschweig commented 11 years ago

I just ran into this too. ::sadpanda::

kcbraunschweig commented 11 years ago

This doesn't seem to always happen when you'd think though so maybe there's more too it, but I'm convince that in at least some cases foodcritic is behaving in error.

mrusan commented 11 years ago

You should probably describe the cases when you get the error, as all my cases strictly follow this behaviour.

jaymzh commented 11 years ago

In our case (KC and my), it's a provider that has a template whose source filename doesn't look like the destination filename.

acrmp commented 11 years ago

Hi guys,

I'm going to close this one as I think the issue was resolved with the fix for issue #74, released with foodcritic 1.7.0.

Please re-open if you can still reproduce with later versions.

Thanks,

Andrew.

e0da commented 11 years ago

I am experiencing this now with foodcritic v2.0.0.

% foodcritic --version
foodcritic 2.0.0
% foodcritic site-cookbooks/sr_repo                                      
FC033: Missing template: site-cookbooks/sr_repo/recipes/webhook.rb:33
% head -n44 site-cookbooks/sr_repo/recipes/webhook.rb|tail -n12         
    template '/etc/init/webhook.conf' do
      source 'webhook.upstart.conf.erb'
      mode 0644
      variables(
        :environment => 'production',
        :port        => 9292,
        :root        => CURRENT,
        :stderr_log  => "#{LOGS}/stderr.log",
        :stdout_log  => "#{LOGS}/stdout.log",
        :user        => USER,
      )
    end
% find site-cookbooks/sr_repo/templates/default/webhook.upstart.conf.erb
site-cookbooks/sr_repo/templates/default/webhook.upstart.conf.erb
acrmp commented 11 years ago

Hi Justin,

Thanks for pointing this out. That's weird - I can't reproduce the warning from your example.

Can you provide as much of the following detail as you can so I can attempt to reproduce the issue:

Thanks,

Andrew.

e0da commented 11 years ago

Ruby version and patch level

Ruby 1.9.3-p392

% ruby --version
ruby 1.9.3p392 (2013-02-22 revision 39386) [x86_64-linux]

Operating system flavour and version

Ubuntu 12.10 "Quantal Quetzal" 64-bit w/ Linux 3.5.0

% lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 12.10
Release:    12.10
Codename:   quantal
% uname -a
Linux enzo 3.5.0-26-generic #42-Ubuntu SMP Fri Mar 8 23:18:20 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

Type of filesystem the cookbooks are sitting on

ext4

% df -h site-cookbooks/sr_repo/templates/default/webhook.upstart.conf.erb 
Filesystem      Size  Used Avail Use% Mounted on
/dev/sda1       103G   39G   59G  40% /
% mount | grep sda1
/dev/sda1 on / type ext4 (rw,noatime,errors=remount-ro,barrier=0)

Are other template resources declared in the same recipe?

Yes. Let me confirm there is no sensitive data in here... Looks fine.

site-cookbooks/sr_repo/recipes/webhook.rb

# FIXME move away from fnichol/rvm
include_recipe 'rvm::system'

ROOT    = '/srv/webhook'
CURRENT = "#{ROOT}/current"
SHARED  = "#{ROOT}/shared"
LOGS    = "#{SHARED}/log"
USER    = 'nutricate'
GROUP   = 'nogroup'
REPOS   = 'config/repos.yml'

# post-receive hook is in here
git "/var/lib/nutricate/chef" do
  repository "git@github.com:SmartReceipt/chef.git"
  user    USER
  group   USER
  action :sync
end

deploy '/srv/webhook' do

  repo 'https://github.com/SmartReceipt/webhook'
  symlinks(
    'log' => 'log',
    REPOS => REPOS,
  )

  before_symlink do
    directory "#{SHARED}/config" do
      recursive true
    end
  end

  before_restart do

    bash "bundle install in #{CURRENT}" do
      cwd CURRENT
      code 'bundle install'
    end

    template '/etc/init/webhook.conf' do
      source 'webhook.upstart.conf.erb'
      mode 0644
      variables(
        :environment => 'production',
        :port        => 9292,
        :root        => CURRENT,
        :stderr_log  => "#{LOGS}/stderr.log",
        :stdout_log  => "#{LOGS}/stdout.log",
        :user        => USER,
      )
    end

    template "#{SHARED}/#{REPOS}" do
      source 'webhook.repos.yml.erb'
      mode 0644
    end

    bash "chown #{LOGS} to #{USER}" do
      code "chown -R #{USER} #{LOGS}"
    end

    directory LOGS do
      owner USER
      group GROUP
      recursive true
    end
  end
  notifies :restart, 'service[webhook]'
end

service 'webhook' do
  provider Chef::Provider::Service::Upstart
  supports :restart => true
  action [ :enable, :start ]
end

And to confirm that the templates exist:

% find site-cookbooks/sr_repo/templates 
site-cookbooks/sr_repo/templates
site-cookbooks/sr_repo/templates/default
site-cookbooks/sr_repo/templates/default/webhook.upstart.conf.erb
site-cookbooks/sr_repo/templates/default/webhook.repos.yml.erb

Thanks for your help!

Observation

Look at that! The template resource declarations occur inside a deploy resource declaration. Think that might be a factor?

acrmp commented 11 years ago

Hi Justin,

Thanks for providing this extra info. You're right - the cause for this false positive was because the attributes on nested templates weren't being decoded correctly.

I've pushed a fix to master that should resolve this.

Thanks,

Andrew.

e0da commented 11 years ago

Thanks Andrew! I really appreciate it. :heart: foodcritic!

acrmp commented 11 years ago

Hi Justin,

I've just released this change in 2.0.1. Thanks for helping make foodcritic better.

Cheers,

Andrew.