Merge pull request #296 from dylanratcliffe/MODULES-8677-roles-with-same-name

(MODULES-8677) Made resource title unique among many instances

Author: James Pogran

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 
 ## [Unreleased]
 
+### Fixed
+
+- Cannot manage a role with the same name on two instances or two databases ([MODULES-8677](https://tickets.puppetlabs.com/browse/MODULES-8677)) (Thanks [Dylan Ratcliffe](https://github.com/dylanratcliffe))
+
 ## [2.3.0] - 2019-01-22
 
 ### Added

manifests/role.pp

@@ -107,7 +107,7 @@
     }
 
     if size($members) > 0 or $members_purge == true {
-      sqlserver_tsql{ "role-${role}-members":
+      sqlserver_tsql{ "${sqlserver_tsql_title}-members":
         command  => template('sqlserver/create/role/members.sql.erb'),
         onlyif   => template('sqlserver/query/role/member_exists.sql.erb'),
         instance => $instance,

manifests/role/permissions.pp

@@ -53,7 +53,7 @@
     ##
     # Parameters required in template are _state, role, _upermissions, database, type, with_grant_option
     ##
-    sqlserver_tsql{ "role-permissions-${role}-${_state}${_grant_option}-${instance}":
+    sqlserver_tsql{ "role-permissions-${role}-${_state}${_grant_option}-${instance}-${database}":
       instance => $instance,
       command  => template('sqlserver/create/role/permissions.sql.erb'),
       onlyif   => template('sqlserver/query/role/permission_exists.sql.erb'),

spec/acceptance/sqlserver_role_spec.rb

@@ -157,6 +157,47 @@ def ensure_sqlserver_logins_users(db_name)
       run_sql_query(host, { :query => query, :server => hostname, :expected_row_count => 6 })
     end
 
+    it "Create a database-specific role with the same name on two databases", :tier_low => true do
+      pp = <<-MANIFEST
+      sqlserver::config{'MSSQLSERVER':
+        admin_user    => 'sa',
+        admin_pass    => 'Pupp3t1@',
+      }
+      sqlserver::role{'DatabaseRole_1':
+        ensure      => 'present',
+        role        => '#{@role}',
+        database    => '#{db_name}',
+        permissions => {'GRANT' => ['SELECT', 'INSERT', 'UPDATE', 'DELETE', 'CONTROL', 'ALTER']},
+        type        => 'DATABASE',
+      }
+      sqlserver::role{'DatabaseRole_2':
+        ensure      => 'present',
+        role        => '#{@role}',
+        database    => 'master',
+        permissions => {'GRANT' => ['SELECT', 'INSERT', 'UPDATE', 'DELETE', 'CONTROL', 'ALTER']},
+        type        => 'DATABASE',
+      }
+      MANIFEST
+      require 'pry'; binding.pry;
+      execute_manifest(pp, opts = {}) do |r|
+        expect(r.stderr).not_to match(/Error/i)
+      end
+
+      # validate that the database-specific role '#{@role}' is successfully created with specified permissions':
+      # and that it exists in both the MASTER database and the 'db_name' database.
+      query = "USE MASTER;
+      SELECT pr.principal_id, pr.name, pr.type_desc,
+              pr.authentication_type_desc, pe.state_desc, pe.permission_name, dbpr.name
+      FROM sys.database_principals AS pr
+      JOIN sys.database_permissions AS pe
+        ON pe.grantee_principal_id = pr.principal_id
+      JOIN #{db_name}.sys.database_principals as dbpr
+        on pr.name = dbpr.name
+      WHERE pr.name = '#{@role}';"
+
+      run_sql_query(host, { :query => query, :server => hostname, :expected_row_count => 6 })
+    end
+
     it "Create server role #{@role} with optional members and optional members-purge", :tier_low => true do
       pp = <<-MANIFEST
       sqlserver::config{'MSSQLSERVER':

spec/defines/role/permissions_spec.rb

@@ -4,7 +4,7 @@
 RSpec.describe 'sqlserver::role::permissions' do
   include_context 'manifests' do
     let(:title) { 'myTitle' }
-    let(:sqlserver_tsql_title) { 'role-permissions-myCustomRole-GRANT-MSSQLSERVER' }
+    let(:sqlserver_tsql_title) { 'role-permissions-myCustomRole-GRANT-MSSQLSERVER-master' }
     let(:params) { {
         :role => 'myCustomRole',
         :permissions => %w(INSERT UPDATE DELETE SELECT),
@@ -116,6 +116,7 @@
     describe 'customDatabase' do
       let(:additional_params) { {:database => 'customDatabase'} }
       let(:should_contain_command) { ['USE [customDatabase];'] }
+      let(:sqlserver_tsql_title) { 'role-permissions-myCustomRole-GRANT-MSSQLSERVER-customDatabase' }
       it_behaves_like 'sqlserver_tsql command'
       let(:should_contain_onlyif) { ['USE [customDatabase];'] }
       it_behaves_like 'sqlserver_tsql onlyif'
@@ -135,7 +136,7 @@
             :instance => instance
         } }
         it {
-          should contain_sqlserver_tsql("role-permissions-myCustomRole-GRANT-#{instance}").with_instance(instance)
+          should contain_sqlserver_tsql("role-permissions-myCustomRole-GRANT-#{instance}-master").with_instance(instance)
         }
       end
     end

spec/defines/role_spec.rb

@@ -134,7 +134,7 @@
   end
 
   context 'members =>' do
-    let(:sqlserver_tsql_title) { 'role-myCustomRole-members' }
+    let(:sqlserver_tsql_title) { 'role-MSSQLSERVER-master-myCustomRole-members' }
     describe '[test these users]' do
       let(:additional_params) { {
         :members => %w(test these users),
@@ -156,7 +156,7 @@
     end
   end
   context 'members_purge =>' do
-    let(:sqlserver_tsql_title) { 'role-myCustomRole-members' }
+    let(:sqlserver_tsql_title) { 'role-MSSQLSERVER-master-myCustomRole-members' }
     context 'true' do
       describe 'type => SERVER and members => []' do
         let(:additional_params) { {