Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user resource should support filtertable #990

Merged
merged 6 commits into from
Sep 9, 2016
Merged

Conversation

ksubrama
Copy link

No description provided.

@chris-rock
Copy link
Contributor

chris-rock commented Sep 6, 2016

This PR introduces a new syntax for the user resource but still allows users to fallback to the old behavior.

New:

describe users.where(uid: 0).entries do
  it { should eq ['root'] }
  its('uids') { should eq [1234] }
  its('gids') { should eq [1234] }
end

Still works:

  describe user('root') do
    it { should_not exist }
    its ('uid') { should eq 0}
    its ('username') { should eq 'root'}
    its ('gid') { should eq 0}
    its ('home') { should eq '/root'}
    its ('shell') { should eq '/bin/bash'}
  end

The new syntax allows queries across multiple users. The single user resource is a very common special case that we will continue to support.

Integration test pass now for linux and OS X
screen shot 2016-09-06 at 3 26 06 pm

TODOs:

  • Ubuntu, Redhat, Suse
  • OSX
  • Windows
  • Solaris
  • AIX
  • FreeBSD
  • HPUX
  • Performance optimizations for single user resource
  • update unit tests

@chris-rock chris-rock changed the title WIP: Filtertables for users to support sid. WIP: user resource should support filtertable Sep 6, 2016
@chris-rock chris-rock changed the title WIP: user resource should support filtertable user resource should support filtertable Sep 8, 2016
@chris-rock
Copy link
Contributor

I'll talk with HPE for HPUX support later

@stevendanna
Copy link
Contributor

stevendanna commented Sep 8, 2016

Overall this looks good to me.

One thing that caught my eye is that on Linux, I believe it is the case that the users resource will parse /etc/passwd while user resource will shell out to commands. This could potentially be an issue for some users who have configured user databases in nsswitch other than the passwd file as the two resources will show different results. I think the behavior here is reasonable, just wanted to note the difference.

@chris-rock
Copy link
Contributor

@stevendanna This is a good point. Currently the users resource only supports local users. That is something that should be documented properly. Same applies for Windows et al.

@chris-rock
Copy link
Contributor

@stevendanna I added further comments to address that and make the behavior of the users resource more clear

@stevendanna
Copy link
Contributor

👍

@chris-rock chris-rock merged commit 2341075 into master Sep 9, 2016
@chris-rock chris-rock deleted the ksubrama/user_sid branch September 9, 2016 08:24
@chris-rock chris-rock modified the milestone: 0.34.0 Sep 9, 2016
@chris-rock
Copy link
Contributor

fixes #948

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

Successfully merging this pull request may close these issues.

None yet

4 participants