Skip to content

Commit

Permalink
Efficiency updates (#18)
Browse files Browse the repository at this point in the history 
* Change the separate 'add' and 'delete' commands to a single 'modify' when authoritative
* Ensure that stderr is captured when running the commands
  • Loading branch information
@trevor-vaughan
trevor-vaughan authored Dec 18, 2019
1 parent cb1b419 commit a5d8c90
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 48 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
* Tue Dec 17 2019 Trevor Vaughan <tvaughan@onyxpoint.com> - 1.1.1-0
- Execute a single modify command instead of a list of 'add' and 'delete'
commands
- Capture stderr on command runs

* Fri Dec 13 2019 Trevor Vaughan <tvaughan@onyxpoint.com> - 1.1.0-0
- Add EL8 support
- Fixed a bug where the system would incorrectly attempt to process users even
Expand Down
30 changes: 14 additions & 16 deletions lib/puppet/provider/group/gpasswd.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
EOM

commands :addmember => 'gpasswd',
:delmember => 'gpasswd'
:modmember => 'gpasswd'

has_feature :manages_members unless %w{HP-UX Solaris}.include? Facter.value(:operatingsystem)
has_feature :libuser if Puppet.features.libuser?
Expand Down Expand Up @@ -110,28 +110,26 @@ def members_insync?(is, should)

def members=(to_set)
cmd = []

if is_new_format?
to_be_added = to_set.split(',')
else
to_be_added = to_set.dup
end

if @resource[:auth_membership]
to_be_removed = @current_members - to_be_added
to_be_added = to_be_added - @current_members

!to_be_removed.empty? && cmd += to_be_removed.map { |x|
[ command(:addmember),'-d',x,@resource[:name] ].shelljoin
}
else
to_be_added = to_be_added | @current_members
end
unless to_be_added.empty?
if @resource[:auth_membership]
cmd << [ command(:modmember),'-M',to_be_added.join(','), @resource[:name] ].shelljoin
else
to_be_added = to_be_added | @current_members

!to_be_added.empty? && cmd += to_be_added.map { |x|
[ command(:addmember),'-a',x,@resource[:name] ].shelljoin
}
!to_be_added.empty? && cmd += to_be_added.map { |x|
[ command(:addmember),'-a',x,@resource[:name] ].shelljoin
}
end

mod_group(cmd)
mod_group(cmd)
end
end

private
Expand All @@ -149,7 +147,7 @@ def members=(to_set)
def mod_group(cmds)
cmds.each do |run_cmd|
begin
output = execute(run_cmd, :custom_environment => @custom_environment, :failonfail => false)
output = execute(run_cmd, :custom_environment => @custom_environment, :failonfail => false, :combine => true)

if output.exitstatus != 0
Puppet.warning("Error modifying #{@resource[:name]} using '#{run_cmd}': #{output}")
Expand Down
2 changes: 1 addition & 1 deletion metadata.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "onyxpoint-gpasswd",
"version": "1.1.0",
"version": "1.1.1",
"author": "Trevor Vaughan <tvaughan@onyxpoint.com>",
"summary": "Adds support for :manages_members to the Linux group native type",
"license": "Apache-2.0",
Expand Down
10 changes: 5 additions & 5 deletions spec/acceptance/suites/default/00_default_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
end

it 'should have populated the group' do
group_members = on(host, "getent group #{group}").output.strip.split(':')[3] || []
group_members = on(host, "getent group #{group}").output.strip.split(':')[3].split(',') || []

expect(group_members - users).to be_empty
end
Expand All @@ -65,7 +65,7 @@
end

it 'should have populated the group' do
group_members = on(host, "getent group #{group}").output.strip.split(':')[3] || []
group_members = on(host, "getent group #{group}").output.strip.split(':')[3].split(',') || []

expect(group_members - users).to be_empty
end
Expand All @@ -84,7 +84,7 @@
end

it 'should have populated the group' do
group_members = on(host, "getent group #{group}").output.strip.split(':')[3] || []
group_members = on(host, "getent group #{group}").output.strip.split(':')[3].split(',') || []

expect(group_members - users).to be_empty
end
Expand All @@ -103,7 +103,7 @@
end

it 'should have populated the group' do
group_members = on(host, "getent group #{group}").output.strip.split(':')[3] || []
group_members = on(host, "getent group #{group}").output.strip.split(':')[3].split(',') || []

expect(group_members - (users + meddling_kids)).to be_empty
end
Expand All @@ -123,7 +123,7 @@
end

it 'should have populated the group' do
group_members = on(host, "getent group #{group}").output.strip.split(':')[3] || []
group_members = on(host, "getent group #{group}").output.strip.split(':')[3].split(',') || []

expect(group_members - ['user1','user2']).to be_empty
end
Expand Down
49 changes: 23 additions & 26 deletions spec/unit/provider/group/gpasswd_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
described_class.stubs(:command).with(:delete).returns '/usr/sbin/groupdel'
described_class.stubs(:command).with(:modify).returns '/usr/sbin/groupmod'
described_class.stubs(:command).with(:addmember).returns '/usr/bin/gpasswd'
described_class.stubs(:command).with(:delmember).returns '/usr/bin/gpasswd'
described_class.stubs(:command).with(:modmember).returns '/usr/bin/gpasswd'

if members
@resource = Puppet::Type.type(:group).new(:name => 'mygroup', :members => members, :provider => provider)
Expand Down Expand Up @@ -38,7 +38,8 @@
'/usr/sbin/groupadd -g 555 -o mygroup',
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)

Expand All @@ -58,7 +59,8 @@
'/usr/sbin/groupadd -r mygroup',
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)
provider.create
Expand All @@ -78,7 +80,8 @@
'/usr/sbin/groupadd mygroup',
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)

Expand All @@ -100,7 +103,8 @@
'/usr/sbin/groupadd mygroup',
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)

Expand All @@ -109,7 +113,8 @@
"/usr/bin/gpasswd -a #{member} mygroup",
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)
end
Expand All @@ -131,7 +136,8 @@
"/usr/bin/gpasswd -a #{member} mygroup",
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)
end
Expand All @@ -156,7 +162,8 @@
"/usr/bin/gpasswd -a #{member} mygroup",
{
:custom_environment => {},
:failonfail => false
:failonfail => false,
:combine => true
}
).returns(success_output)
end
Expand All @@ -179,24 +186,14 @@

@resource[:auth_membership] = :true

(members - old_members).each do |to_add|
provider.expects(:execute).with(
"/usr/bin/gpasswd -a #{to_add} mygroup",
{
:custom_environment => {},
:failonfail => false
}
).returns(success_output)
end
(old_members - members).each do |to_del|
provider.expects(:execute).with(
"/usr/bin/gpasswd -d #{to_del} mygroup",
{
:custom_environment => {},
:failonfail => false
}
).returns(success_output)
end
provider.expects(:execute).with(
"/usr/bin/gpasswd -M #{members.join(',')} mygroup",
{
:custom_environment => {},
:failonfail => false,
:combine => true
}
).returns(success_output)

provider.create

Expand Down

0 comments on commit a5d8c90

Please sign in to comment.