Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

New Rule Proposal: uid/gid should be integer #53

Closed
jaymzh opened this issue Jul 13, 2012 · 12 comments
Closed

New Rule Proposal: uid/gid should be integer #53

jaymzh opened this issue Jul 13, 2012 · 12 comments

Comments

@jaymzh
Copy link
Collaborator

jaymzh commented Jul 13, 2012

Specifying a user like:

user "foo" do
  uid "3456"
  ...
end

Will cause chef to choke. FC should catch that this string is really an integer.

@jtimberman
Copy link
Contributor

The user resource uid attribute can be a string or an integer.

      def uid(arg=nil)
        set_or_return(
          :uid,
          arg,
          :kind_of => [ String, Integer ]
        )
      end

Two users, string vs integer uid:

user "foo" do
 uid "34567"
end

user "bar" do
  uid 34568
end
INFO: Processing user[foo] action create ((irb#1) line 1)
DEBUG: user[foo] user does not exist
DEBUG: user[foo] setting uid to 34567
DEBUG: Executing useradd -u '34567' foo
DEBUG: ---- Begin output of useradd -u '34567' foo ----
DEBUG: STDOUT: 
DEBUG: STDERR: 
DEBUG: ---- End output of useradd -u '34567' foo ----
DEBUG: Ran useradd -u '34567' foo returned 0
INFO: user[foo] created
INFO: Processing user[bar] action create ((irb#1) line 4)
DEBUG: user[bar] user does not exist
DEBUG: user[bar] setting uid to 34568
DEBUG: Executing useradd -u '34568' bar
DEBUG: ---- Begin output of useradd -u '34568' bar ----
DEBUG: STDOUT: 
DEBUG: STDERR: 
DEBUG: ---- End output of useradd -u '34568' bar ----
DEBUG: Ran useradd -u '34568' bar returned 0
INFO: user[bar] created

Chef version 10.12.0.

@acrmp
Copy link
Member

acrmp commented Aug 27, 2012

Hi Phil,

Do you have any more detail on the situation where you saw this issue or can I close it? Joshua's example seems to show strings and integers are both valid.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Aug 27, 2012

Just yesterday I had a block like:

group 'foo' do
  ...
  gid "12345"
end

try to resolve "12345" as a group name in 10.12. I'll try to get a reproduction posted today.

@kcbraunschweig
Copy link

This is indeed problematic for the group resource, which wasn't the original example:

chef > recipe
chef:recipe > group 'foo' do
chef:recipe > gid "30090"
chef:recipe ?> end
Chef::Exceptions::ValidationFailed: Option gid must be a kind of [Integer]! You passed "30090".

@jtimberman
Copy link
Contributor

Indeed!

      def gid(arg=nil)
        set_or_return(
          :gid,
          arg,
          :kind_of => [ Integer ]
        )
      end

@jtimberman
Copy link
Contributor

In light of the inconsistency between the uid on user resource and gid on group resource, I think the preference should be to update Chef to allow strings for the gid.

I created CHEF-3395 for this. A rule in foodcritic may be okay too, but I think this is the long term solution.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Aug 28, 2012

So what's odd is that KC gets a crash, but I've had Chef tell me it can't resolve "xxxx" (some gid as a string) to a GID... which is different.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Aug 28, 2012

Hmm. I cannot reproduce this in shef. Weird. I can reproduce KC's output. Your bug seems reasonable, @jtimberman.

@jaymzh
Copy link
Collaborator Author

jaymzh commented Aug 30, 2012

OK, here's the repro case:

group "foo" do
  gid 30099
end
user "foo" do
  uid "30099"
  shell "/sbin/nologin"
  home "/home/foo"
  gid "30099"
end

The error is:

Chef::Exceptions::User: user[foo] ((irb#1) line 4) had an error: Chef::Exceptions::User: Couldn't lookup integer GID for group name 30099

It doesn't happen every time though.... @jtimberman I told you I wasn't insane. :)

@kevinburke
Copy link

Maybe you could add a warning for a string uid/gid that looks like an integer? Just ran into this as well...

@sethvargo
Copy link

Awesome community-ness going on here. While I agree with @jtimberman - I think we should fix the upstream problem in Chef - Foodcritic is a linting tool aimed at provided consistency (among other things), so I'm in favor of having a rule to force consistency.

@lamont-granquist
Copy link
Contributor

It looks like @jtimberman fixed this in core chef back in 10.14.2

chef/chef@cc50e4b

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants