Numeric value for mode should be 5 digits (CHEF-174) #9

Merged
merged 1 commit into from Feb 21, 2012

Conversation

Projects
None yet
2 participants
@aia
Contributor

aia commented Feb 21, 2012

In FC006, numeric value for mode should be 5 digits to ensure that ruby (jruby) parses the mode correctly. I actually run into this one myself with jruby.

As seen in
http://wiki.opscode.com/display/chef/Resources#Resources-CookbookFile
"A note about mode: When specifying the mode, the value can be a quoted string, eg "644". For a numeric value (i.e., without quotes), it should be 5 digits, e.g., 00644 to ensure that Ruby can parse it correctly. For more detail, see Ticket CHEF-174."

Given how working with numbers in Ruby can be tricky, "FC002: Avoid string interpolation where not required" does not sound like a good advice. My rule of thumb - quote unless you are really really sure.

@acrmp acrmp merged commit 5606616 into Foodcritic:master Feb 21, 2012

@acrmp

This comment has been minimized.

Show comment Hide comment
@acrmp

acrmp Feb 21, 2012

Owner

Hi Artem,

Thanks for raising this - you're absolutely right the current rule implementation is broken.

I added the rule initially in reference to CHEF-174 but made a conscious decision to implement it to allow a 4-digit literal based on reading another discussion.

Whether or not Ruby treats it as an octal number is controlled by whether it has a leading zero.

The current rule works for the common case because if you don't set the setuid, setguid or sticky bits then to pass the rule you will need to specify a leading zero. However if you have a file mode that sets one of these bits then you could get away currently without specifying the zero which is just plain nasty.

I've merged your commit but it will probably need more work to allow 4-digit literals provided they have a leading zero - there are lots of examples of this (legal) usage in the Opscode cookbook tree. Is this something you could take a look at?

Can you expand on FC002 in a separate issue.

Thanks,

Andrew.

Owner

acrmp commented Feb 21, 2012

Hi Artem,

Thanks for raising this - you're absolutely right the current rule implementation is broken.

I added the rule initially in reference to CHEF-174 but made a conscious decision to implement it to allow a 4-digit literal based on reading another discussion.

Whether or not Ruby treats it as an octal number is controlled by whether it has a leading zero.

The current rule works for the common case because if you don't set the setuid, setguid or sticky bits then to pass the rule you will need to specify a leading zero. However if you have a file mode that sets one of these bits then you could get away currently without specifying the zero which is just plain nasty.

I've merged your commit but it will probably need more work to allow 4-digit literals provided they have a leading zero - there are lots of examples of this (legal) usage in the Opscode cookbook tree. Is this something you could take a look at?

Can you expand on FC002 in a separate issue.

Thanks,

Andrew.

acrmp added a commit that referenced this pull request Feb 21, 2012

@aia

This comment has been minimized.

Show comment Hide comment
@aia

aia Feb 22, 2012

Contributor

Hi Andrew,

I will be glad to see how this can be expanded to cover the 4-digit literal case with leading 0. In my humble opinion though, Opscode should change their cookbooks to use the less ambiguous 5-digits or quotes rule for mode.

Here's a case that actually happened to us. We pushed the 2755 mode in production and learned the effect of that the hard way. After the incident, we decided to enforce the 5-digit rule very strictly. The result of making a mode mistake is too costly. Thus, the rule needs to very simple and unambiguous. "5-digits or quotes" is easy enough for everyone to remember.

I think your linter idea is absolutely great and I like the implementation.

Contributor

aia commented Feb 22, 2012

Hi Andrew,

I will be glad to see how this can be expanded to cover the 4-digit literal case with leading 0. In my humble opinion though, Opscode should change their cookbooks to use the less ambiguous 5-digits or quotes rule for mode.

Here's a case that actually happened to us. We pushed the 2755 mode in production and learned the effect of that the hard way. After the incident, we decided to enforce the 5-digit rule very strictly. The result of making a mode mistake is too costly. Thus, the rule needs to very simple and unambiguous. "5-digits or quotes" is easy enough for everyone to remember.

I think your linter idea is absolutely great and I like the implementation.

@acrmp

This comment has been minimized.

Show comment Hide comment
@acrmp

acrmp Feb 22, 2012

Owner

Thanks, that's great.

Yes, I think the "5-digits" rule is the message Opscode are trying to send in the docs and I agree it would be great if the Opscode cookbook tree were updated to reflect this for consistency.

If you are able to complete the contributor agreement then I expect they'd probably appreciate a pull request:
http://wiki.opscode.com/display/chef/How+to+Contribute

Owner

acrmp commented Feb 22, 2012

Thanks, that's great.

Yes, I think the "5-digits" rule is the message Opscode are trying to send in the docs and I agree it would be great if the Opscode cookbook tree were updated to reflect this for consistency.

If you are able to complete the contributor agreement then I expect they'd probably appreciate a pull request:
http://wiki.opscode.com/display/chef/How+to+Contribute

@acrmp

This comment has been minimized.

Show comment Hide comment
@acrmp

acrmp Feb 22, 2012

Owner

See pull request #12 which has the change to allow 4-digit literals with leading zeros.

Owner

acrmp commented Feb 22, 2012

See pull request #12 which has the change to allow 4-digit literals with leading zeros.

@aia

This comment has been minimized.

Show comment Hide comment
@aia

aia Feb 22, 2012

Contributor

Opened a ticket to convert Opscode cookbooks to 5-digit notation
http://tickets.opscode.com/browse/COOK-1054

Contributor

aia commented Feb 22, 2012

Opened a ticket to convert Opscode cookbooks to 5-digit notation
http://tickets.opscode.com/browse/COOK-1054

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment