From 3bf21e0767c1fbd93cf7bb825e454418287be6dd Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Mon, 19 Jun 2017 15:03:31 -0700 Subject: [PATCH] (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. --- spec/acceptance/sqlserver_role_spec.rb | 3 +-- spec/defines/role_spec.rb | 26 +++++----------------- templates/delete/role.sql.erb | 13 +++++++++++ templates/query/role/member_exists.sql.erb | 16 ++++++++----- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/spec/acceptance/sqlserver_role_spec.rb b/spec/acceptance/sqlserver_role_spec.rb index 69275a40..1c940214 100644 --- a/spec/acceptance/sqlserver_role_spec.rb +++ b/spec/acceptance/sqlserver_role_spec.rb @@ -157,8 +157,7 @@ def ensure_sqlserver_logins_users(host, db_name) run_sql_query(host, { :query => query, :server => hostname, :expected_row_count => 6 }) end - # temporarily skip this test because of ticket MODULES-2543 - xit "Create server role #{@role} with optional members and optional members-purge" do + it "Create server role #{@role} with optional members and optional members-purge" do pp = <<-MANIFEST sqlserver::config{'MSSQLSERVER': admin_user => 'sa', diff --git a/spec/defines/role_spec.rb b/spec/defines/role_spec.rb index f37a7be8..21fd39a4 100644 --- a/spec/defines/role_spec.rb +++ b/spec/defines/role_spec.rb @@ -171,17 +171,10 @@ END" ] } let(:should_contain_onlyif) { [ - "DECLARE @purge_members TABLE ( -ID int IDENTITY(1,1), -member varchar(128) -)", - "INSERT INTO @purge_members (member) ( -SELECT m.name FROM sys.server_role_members rm +"SELECT m.name FROM sys.server_role_members rm JOIN sys.server_principals r ON rm.role_principal_id = r.principal_id - JOIN sys.server_principals m ON rm.member_principal_id = m.principal_id - WHERE r.name = 'myCustomRole'", - "IF 0 != (SELECT COUNT(*) FROM @purge_members) - THROW 51000, 'Unlisted Members in Role, will be purged', 10", + JOIN sys.server_principals m ON rm.member_principal_id = m.principal_id + WHERE r.name = 'myCustomRole'" ] } it_behaves_like 'sqlserver_tsql command' it_behaves_like 'sqlserver_tsql onlyif' @@ -201,17 +194,10 @@ END" ] } let(:should_contain_onlyif) { [ - "DECLARE @purge_members TABLE ( -ID int IDENTITY(1,1), -member varchar(128) -)", - "INSERT INTO @purge_members (member) ( -SELECT m.name FROM sys.database_role_members rm +"SELECT m.name FROM sys.database_role_members rm JOIN sys.database_principals r ON rm.role_principal_id = r.principal_id - JOIN sys.database_principals m ON rm.member_principal_id = m.principal_id - WHERE r.name = 'myCustomRole'", - "IF 0 != (SELECT COUNT(*) FROM @purge_members) - THROW 51000, 'Unlisted Members in Role, will be purged', 10", + JOIN sys.database_principals m ON rm.member_principal_id = m.principal_id + WHERE r.name = 'myCustomRole'", ] } it_behaves_like 'sqlserver_tsql command' it_behaves_like 'sqlserver_tsql onlyif' diff --git a/templates/delete/role.sql.erb b/templates/delete/role.sql.erb index 925fe6d3..1352aa7e 100644 --- a/templates/delete/role.sql.erb +++ b/templates/delete/role.sql.erb @@ -1,5 +1,18 @@ USE [<%= @database %>]; BEGIN + DECLARE @cmd AS NVARCHAR(MAX) = N''; + + SELECT @cmd = @cmd + ' + ALTER <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>] DROP MEMBER ' + QUOTENAME(members.[name]) + ';' + FROM sys.<%= @type.downcase %>_role_members AS rolemembers + JOIN sys.<%= @type.downcase %>_principals AS roles + ON roles.[principal_id] = rolemembers.[role_principal_id] + JOIN sys.<%= @type.downcase %>_principals AS members + ON members.[principal_id] = rolemembers.[member_principal_id] + WHERE roles.name = '<%= @role %>' + + EXEC(@cmd); + DROP <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>]; END <%= scope.function_template(['sqlserver/query/role_exists.sql.erb']) %> diff --git a/templates/query/role/member_exists.sql.erb b/templates/query/role/member_exists.sql.erb index 6072a0da..d532083b 100644 --- a/templates/query/role/member_exists.sql.erb +++ b/templates/query/role/member_exists.sql.erb @@ -8,11 +8,15 @@ DECLARE SET @member = '<%= member %>'; SET @error_msg = 'The member [<%= member %>] is <% if @ensure == 'present'%>not <% end %>a member of the role [<%=@role %>]'; <%= scope.function_template(['sqlserver/snippets/role/member_exists.sql.erb']) -%> - THROW 51000, @error_msg, 10 -<% end %> + THROW 51000, @error_msg, 10; +<% end -%> <% if @members_purge %> -<%= scope.function_template(['sqlserver/snippets/role/populate_purge_members.sql.erb']) %> -IF 0 != (SELECT COUNT(*) FROM @purge_members) - THROW 51000, 'Unlisted Members in Role, will be purged', 10 -<% end %> +IF EXISTS( +SELECT m.name FROM sys.<%= @type.downcase %>_role_members rm + JOIN sys.<%= @type.downcase %>_principals r ON rm.role_principal_id = r.principal_id + JOIN sys.<%= @type.downcase %>_principals m ON rm.member_principal_id = m.principal_id + WHERE r.name = '<%= @role %>' + <% if !@members.empty? %>AND m.name NOT IN (<%= @members.collect{|m| "'#{m}'"}.join(',') %>)<% end %> +) THROW 51000, 'Unlisted Members in Role, will be purged', 10; +<% end -%>