Foodcritic / foodcritic

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

FC019 false positive on `node.force_default` #715

Closed haidangwa closed 5 years ago

haidangwa commented 6 years ago

This looks like a re-occurrence of #181

chefdk: 2.4.17

My cookbook wraps the ssh_known_hosts cookbook. Foodcritic throws this FC019 violation exactly as discussed in #181.

My Gemfile:

gem 'berkshelf', '~> 6.3.1'
gem 'chef', '~> 12'
gem 'chef-zero'
gem 'chefspec', '~> 7.1.0'
gem 'foodcritic', '~> 12.2.2'
gem 'inspec' # constrained by kitchen-inspec
gem 'kitchen-ec2', '~> 1.4.0'
gem 'kitchen-inspec', '~> 0.20'
gem 'kitchen-sync', '~> 2.1'
gem 'rubocop', '~> 0.51.0'
gem 'test-kitchen', '~> 1.19.2'

FC019: Access node attributes in a consistent manner: The line that it complains about simply does

node.force_default['sshd']['config']['AuthorizedKeysFile'] = '/etc/ssh/authorized_keys/%u'

Changing this to use node.default instead will get rid of the violation warning.

TravisWhitehead commented 5 years ago

I am experiencing the same issue on the following versions:

$ chef --version
Chef Development Kit Version: 2.5.3
chef-client version: 13.8.5
delivery version: master (73ebb72a6c42b3d2ff5370c476be800fee7e5427)
berks version: 6.3.1
kitchen version: ERROR
inspec version: 1.51.21
$ foodcritic --version
foodcritic 12.3.0

Could this be resulting from a missing chef_dsl_metadata file for the combination of Chef & foodcritic that I'm using? As far as I can see there is no entry for Chef 13.8

TravisWhitehead commented 5 years ago

I've researched this further. Even if I generate a chef_dsl_metadata/chef_13.8.5.json (beause I am using Chef 13.8.5) by following these instructions, this false-positive persists when I use node.force_default.

I noticed that in the generated file force_default is not present in the dsl_methods list, but if I add it to that list then no false-positive FC019 occurs.

In an old solution to #181, PR #364 added Chef::Node::Attribute.public_instance_methods to chef_node_methods as a solution to the FC019 false-positive discussed in #181.

I've tried adding Chef::Node::Attribute.public_instance_methods to chef_dsl_methods in chef_dsl_metadata/Rakefile and have found that the files generated from this modified Rakefile no longer produce a false-positive FC019.

I'm unsure if this is a suitable solution, so I would like some input from the foodcritic community. I may make a PR soon requesting commentary on this fix.