Skip to content

Commit 3bf21e0

Browse files
committed
(MODULES-2543) Purge members from SQL Server Role
Previously the purge_members parameter of the sqlserver::role resource was not taking effect. This commit re-instates the acceptance test for this behaviour and changes the sql query used to detect members which are no longer required. It appears the table variable usage was returning zero results for the row count. Instead the detection is changed to just using the SELECT query, instead of an INSERT INTO, which does not require any row count calculation. This commit also modifies the role deletion as SQL Server requires any members to be removed prior to a role being deleted.
1 parent a52e962 commit 3bf21e0

File tree

4 files changed

+30
-28
lines changed

4 files changed

+30
-28
lines changed

spec/acceptance/sqlserver_role_spec.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ def ensure_sqlserver_logins_users(host, db_name)
157157
run_sql_query(host, { :query => query, :server => hostname, :expected_row_count => 6 })
158158
end
159159

160-
# temporarily skip this test because of ticket MODULES-2543
161-
xit "Create server role #{@role} with optional members and optional members-purge" do
160+
it "Create server role #{@role} with optional members and optional members-purge" do
162161
pp = <<-MANIFEST
163162
sqlserver::config{'MSSQLSERVER':
164163
admin_user => 'sa',

spec/defines/role_spec.rb

+6-20
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,10 @@
171171
END"
172172
] }
173173
let(:should_contain_onlyif) { [
174-
"DECLARE @purge_members TABLE (
175-
ID int IDENTITY(1,1),
176-
member varchar(128)
177-
)",
178-
"INSERT INTO @purge_members (member) (
179-
SELECT m.name FROM sys.server_role_members rm
174+
"SELECT m.name FROM sys.server_role_members rm
180175
JOIN sys.server_principals r ON rm.role_principal_id = r.principal_id
181-
JOIN sys.server_principals m ON rm.member_principal_id = m.principal_id
182-
WHERE r.name = 'myCustomRole'",
183-
"IF 0 != (SELECT COUNT(*) FROM @purge_members)
184-
THROW 51000, 'Unlisted Members in Role, will be purged', 10",
176+
JOIN sys.server_principals m ON rm.member_principal_id = m.principal_id
177+
WHERE r.name = 'myCustomRole'"
185178
] }
186179
it_behaves_like 'sqlserver_tsql command'
187180
it_behaves_like 'sqlserver_tsql onlyif'
@@ -201,17 +194,10 @@
201194
END"
202195
] }
203196
let(:should_contain_onlyif) { [
204-
"DECLARE @purge_members TABLE (
205-
ID int IDENTITY(1,1),
206-
member varchar(128)
207-
)",
208-
"INSERT INTO @purge_members (member) (
209-
SELECT m.name FROM sys.database_role_members rm
197+
"SELECT m.name FROM sys.database_role_members rm
210198
JOIN sys.database_principals r ON rm.role_principal_id = r.principal_id
211-
JOIN sys.database_principals m ON rm.member_principal_id = m.principal_id
212-
WHERE r.name = 'myCustomRole'",
213-
"IF 0 != (SELECT COUNT(*) FROM @purge_members)
214-
THROW 51000, 'Unlisted Members in Role, will be purged', 10",
199+
JOIN sys.database_principals m ON rm.member_principal_id = m.principal_id
200+
WHERE r.name = 'myCustomRole'",
215201
] }
216202
it_behaves_like 'sqlserver_tsql command'
217203
it_behaves_like 'sqlserver_tsql onlyif'

templates/delete/role.sql.erb

+13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
USE [<%= @database %>];
22
BEGIN
3+
DECLARE @cmd AS NVARCHAR(MAX) = N'';
4+
5+
SELECT @cmd = @cmd + '
6+
ALTER <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>] DROP MEMBER ' + QUOTENAME(members.[name]) + ';'
7+
FROM sys.<%= @type.downcase %>_role_members AS rolemembers
8+
JOIN sys.<%= @type.downcase %>_principals AS roles
9+
ON roles.[principal_id] = rolemembers.[role_principal_id]
10+
JOIN sys.<%= @type.downcase %>_principals AS members
11+
ON members.[principal_id] = rolemembers.[member_principal_id]
12+
WHERE roles.name = '<%= @role %>'
13+
14+
EXEC(@cmd);
15+
316
DROP <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>];
417
END
518
<%= scope.function_template(['sqlserver/query/role_exists.sql.erb']) %>

templates/query/role/member_exists.sql.erb

+10-6
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@ DECLARE
88
SET @member = '<%= member %>';
99
SET @error_msg = 'The member [<%= member %>] is <% if @ensure == 'present'%>not <% end %>a member of the role [<%=@role %>]';
1010
<%= scope.function_template(['sqlserver/snippets/role/member_exists.sql.erb']) -%>
11-
THROW 51000, @error_msg, 10
12-
<% end %>
11+
THROW 51000, @error_msg, 10;
12+
<% end -%>
1313

1414
<% if @members_purge %>
15-
<%= scope.function_template(['sqlserver/snippets/role/populate_purge_members.sql.erb']) %>
16-
IF 0 != (SELECT COUNT(*) FROM @purge_members)
17-
THROW 51000, 'Unlisted Members in Role, will be purged', 10
18-
<% end %>
15+
IF EXISTS(
16+
SELECT m.name FROM sys.<%= @type.downcase %>_role_members rm
17+
JOIN sys.<%= @type.downcase %>_principals r ON rm.role_principal_id = r.principal_id
18+
JOIN sys.<%= @type.downcase %>_principals m ON rm.member_principal_id = m.principal_id
19+
WHERE r.name = '<%= @role %>'
20+
<% if [email protected]? %>AND m.name NOT IN (<%= @members.collect{|m| "'#{m}'"}.join(',') %>)<% end %>
21+
) THROW 51000, 'Unlisted Members in Role, will be purged', 10;
22+
<% end -%>

0 commit comments

Comments
 (0)