From 25ac79cc107ebea49d330ebde512b556fde78470 Mon Sep 17 00:00:00 2001 From: Travis Fields Date: Wed, 25 Feb 2015 11:21:58 -0800 Subject: [PATCH 1/2] Fixed a bug with sqlserver_validate_instance_name that was not catching empty strings --- .../parser/functions/sqlserver_validate_instance_name.rb | 3 +++ spec/functions/sqlserver_validate_instance_name_spec.rb | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/lib/puppet/parser/functions/sqlserver_validate_instance_name.rb b/lib/puppet/parser/functions/sqlserver_validate_instance_name.rb index 74991c77..152876e2 100644 --- a/lib/puppet/parser/functions/sqlserver_validate_instance_name.rb +++ b/lib/puppet/parser/functions/sqlserver_validate_instance_name.rb @@ -20,6 +20,9 @@ module Puppet::Parser::Functions end value = args[0] errors = [] + if value.length < 1 || value.empty? + errors << "Instance name must be between 1 to 16 characters" + end if value.length > 16 errors << "Instance name can not be larger than 16 characters, you provided #{value}" end diff --git a/spec/functions/sqlserver_validate_instance_name_spec.rb b/spec/functions/sqlserver_validate_instance_name_spec.rb index 5e180f03..ab394fd3 100644 --- a/spec/functions/sqlserver_validate_instance_name_spec.rb +++ b/spec/functions/sqlserver_validate_instance_name_spec.rb @@ -2,11 +2,17 @@ describe 'sqlserver_validate_instance_name function' do let(:scope) { PuppetlabsSpec::PuppetInternals.scope } + it 'should exist' do expect(Puppet::Parser::Functions.function("sqlserver_validate_instance_name")).to eq("function_sqlserver_validate_instance_name") end + it 'should fail with over 16 characters' do expect { scope.function_sqlserver_validate_instance_name('ABCDEFGHIJKLMNOPQRSTUVWXYZ') }.to raise_error end + it 'should fail empty string' do + expect { scope.function_sqlserver_validate_instance_name('') }.to raise_error + end + end From 071ee8de4a55e373138624d7f5966184ff32d009 Mon Sep 17 00:00:00 2001 From: Travis Fields Date: Wed, 25 Feb 2015 14:14:17 -0800 Subject: [PATCH 2/2] FM-2286 Create/Drop Roles with Authorization --- manifests/role.pp | 30 +++++ spec/defines/role_spec.rb | 135 ++++++++++++++++++++ templates/create/role.sql.erb | 10 ++ templates/delete/role.sql.erb | 5 + templates/query/role_exists.sql.erb | 7 + templates/snippets/role/exists.sql.erb | 3 + templates/snippets/role/owner_check.sql.erb | 4 + 7 files changed, 194 insertions(+) create mode 100644 manifests/role.pp create mode 100644 spec/defines/role_spec.rb create mode 100644 templates/create/role.sql.erb create mode 100644 templates/delete/role.sql.erb create mode 100644 templates/query/role_exists.sql.erb create mode 100644 templates/snippets/role/exists.sql.erb create mode 100644 templates/snippets/role/owner_check.sql.erb diff --git a/manifests/role.pp b/manifests/role.pp new file mode 100644 index 00000000..4bb268dc --- /dev/null +++ b/manifests/role.pp @@ -0,0 +1,30 @@ +define sqlserver::role( + $ensure = present, + $role = $title, + $instance = 'MSSQLSERVER', + $authorization = undef, + $type = 'SERVER', + $database = 'master' +){ + sqlserver_validate_instance_name($instance) + sqlserver_validate_range($role, 1, 128, 'Role names must be between 1 and 128 characters') + + validate_re($type, ['^SERVER$','^DATABASE$'], "Type must be either 'SERVER' or 'DATABASE', provided '${type}'") + + sqlserver_validate_range($database, 1, 128, 'Database name must be between 1 and 128 characters') + if $type == 'SERVER' and $database != 'master' { + fail('Can not specify a database other than master when managing SERVER ROLES') + } + + $create_delete = $ensure ? { + present => 'create', + absent => 'delete', + } + + sqlserver_tsql{ "role-${role}-${instance}": + command => template("sqlserver/${create_delete}/role.sql.erb"), + onlyif => template('sqlserver/query/role_exists.sql.erb'), + instance => $instance, + } + +} diff --git a/spec/defines/role_spec.rb b/spec/defines/role_spec.rb new file mode 100644 index 00000000..511f8f6f --- /dev/null +++ b/spec/defines/role_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' +require File.expand_path(File.join(File.dirname(__FILE__), 'manifest_shared_examples.rb')) + +RSpec.describe 'sqlserver::role', :type => :define do + include_context 'manifests' do + let(:sqlserver_tsql_title) { 'role-myCustomRole-MSSQLSERVER' } + let(:title) { 'myCustomRole' } + end + + context 'type =>' do + describe 'invalid' do + let(:additional_params) { { + :type => 'invalid', + } } + let(:raise_error_check) { "Type must be either 'SERVER' or 'DATABASE', provided 'invalid'" } + it_behaves_like 'validation error' + end + describe 'SERVER' do + let(:should_contain_command) { [ + 'USE [master];', + 'CREATE SERVER ROLE [myCustomRole];', + /IF NOT EXISTS\(\n\s+SELECT name FROM sys\.server_principals WHERE type_desc = 'SERVER_ROLE' AND name = 'myCustomRole'\n\)/, + "THROW 51000, 'The SERVER ROLE [myCustomRole] does not exist', 10" + ] } + let(:should_contain_onlyif) { [ + /IF NOT EXISTS\(\n\s+SELECT name FROM sys\.server_principals WHERE type_desc = 'SERVER_ROLE' AND name = 'myCustomRole'\n\)/, + "THROW 51000, 'The SERVER ROLE [myCustomRole] does not exist', 10" + ] } + it_behaves_like 'sqlserver_tsql command' + it_behaves_like 'sqlserver_tsql onlyif' + end + describe 'DATABASE' do + let(:additional_params) { { + 'type' => 'DATABASE', + } } + let(:should_contain_command) { [ + 'USE [master];', + 'CREATE ROLE [myCustomRole];', + /IF NOT EXISTS\(\n\s+SELECT name FROM sys\.database_principals WHERE type_desc = 'DATABASE_ROLE' AND name = 'myCustomRole'\n\)/, + "THROW 51000, 'The DATABASE ROLE [myCustomRole] does not exist', 10" + ] } + let(:should_contain_onlyif) { [ + /IF NOT EXISTS\(\n\s+SELECT name FROM sys\.database_principals WHERE type_desc = 'DATABASE_ROLE' AND name = 'myCustomRole'\n\)/, + "THROW 51000, 'The DATABASE ROLE [myCustomRole] does not exist', 10", + ] } + + it_behaves_like 'sqlserver_tsql command' + it_behaves_like 'sqlserver_tsql onlyif' + + end + end + + context 'database =>' do + let(:additional_params) { { + 'database' => 'myCrazyDb', + } } + describe 'with server role type' do + let(:raise_error_check) { 'Can not specify a database other than master when managing SERVER ROLES' } + it_behaves_like 'validation error' + end + describe 'with database role type' do + let(:additional_params) { { + 'database' => 'myCrazyDb', + 'type' => 'DATABASE', + } } + let(:should_contain_command) { [ + 'USE [myCrazyDb];', + ] } + it_behaves_like 'sqlserver_tsql command' + end + end + + context 'instance =>' do + describe 'non default instance' do + let(:params) { {:instance => 'MYCUSTOM'} } + it { + should contain_sqlserver_tsql('role-myCustomRole-MYCUSTOM').with_instance('MYCUSTOM') + } + end + describe 'empty instance' do + let(:additional_params) { {'instance' => ''} } + let(:raise_error_check) { 'Instance name must be between 1 to 16 characters' } + it_behaves_like 'validation error' + end + end + + context 'authorization =>' do + describe 'undef' do + let(:should_not_contain_command) { [ + /AUTHORIZATION/i, + 'ALTER AUTHORIZATION ON ', + ] } + it_behaves_like 'sqlserver_tsql without_command' + end + describe 'myUser' do + let(:additional_params) { { + :authorization => 'myUser', + } } + let(:should_contain_command) { [ + 'CREATE SERVER ROLE [myCustomRole] AUTHORIZATION [myUser];', + 'ALTER AUTHORIZATION ON SERVER ROLE::[myCustomRole] TO [myUser];' + ] } + it_behaves_like 'sqlserver_tsql command' + end + describe 'myUser on Database' do + let(:additional_params) { { + :authorization => 'myUser', + :type => 'DATABASE', + } } + let(:should_contain_command) { [ + 'CREATE ROLE [myCustomRole] AUTHORIZATION [myUser];', + 'ALTER AUTHORIZATION ON ROLE::[myCustomRole] TO [myUser];' + ] } + it_behaves_like 'sqlserver_tsql command' + end + end + + context 'ensure =>' do + describe 'absent' do + let(:additional_params) { { + :ensure => 'absent', + } } + let(:should_contain_command) { [ + 'USE [master];', + 'DROP SERVER ROLE [myCustomRole];' + ] } + let(:should_contain_onlyif) { [ + 'IF EXISTS(', + ] } + it_behaves_like 'sqlserver_tsql command' + it_behaves_like 'sqlserver_tsql onlyif' + end + end + +end diff --git a/templates/create/role.sql.erb b/templates/create/role.sql.erb new file mode 100644 index 00000000..57441b2a --- /dev/null +++ b/templates/create/role.sql.erb @@ -0,0 +1,10 @@ +USE [<%= @database %>]; +BEGIN + <%= scope.function_template(['sqlserver/snippets/role/exists.sql.erb']) %> + CREATE <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>]<% if @authorization %> AUTHORIZATION [<%= @authorization %>]<% end %>; + <% if @authorization %> + <%= scope.function_template(['sqlserver/snippets/role/owner_check.sql.erb']) %> + ALTER AUTHORIZATION ON <% if @type =='SERVER' %>SERVER <% end %>ROLE::[<%= @role %>] TO [<%= @authorization %>]; + <% end %> +END +<%= scope.function_template(['sqlserver/query/role_exists.sql.erb']) %> diff --git a/templates/delete/role.sql.erb b/templates/delete/role.sql.erb new file mode 100644 index 00000000..925fe6d3 --- /dev/null +++ b/templates/delete/role.sql.erb @@ -0,0 +1,5 @@ +USE [<%= @database %>]; +BEGIN + DROP <% if @type == 'SERVER' %>SERVER <% end %>ROLE [<%= @role %>]; +END +<%= scope.function_template(['sqlserver/query/role_exists.sql.erb']) %> diff --git a/templates/query/role_exists.sql.erb b/templates/query/role_exists.sql.erb new file mode 100644 index 00000000..61ed61a0 --- /dev/null +++ b/templates/query/role_exists.sql.erb @@ -0,0 +1,7 @@ +USE [<%= @database %>]; +<%= scope.function_template(['sqlserver/snippets/role/exists.sql.erb']) %> + THROW 51000, 'The <%= @type %> ROLE [<%= @role %>] does <% if @ensure == 'present' %>not<% end %> exist', 10 +<% if @ensure == 'present' && @authorization -%> + <%= scope.function_template(['sqlserver/snippets/role/owner_check.sql.erb']) %> + THROW 51000, 'The <%= @type %> ROLE [<%= @role %>] does not have the correct owner of [<%= @authorization %>]', 10 +<% end -%> diff --git a/templates/snippets/role/exists.sql.erb b/templates/snippets/role/exists.sql.erb new file mode 100644 index 00000000..e3c7a18b --- /dev/null +++ b/templates/snippets/role/exists.sql.erb @@ -0,0 +1,3 @@ +IF <% if @ensure == 'present' %>NOT <% end %>EXISTS( + SELECT name FROM sys.<%= @type.downcase %>_principals WHERE type_desc = '<%= @type %>_ROLE' AND name = '<%= @role %>' +) diff --git a/templates/snippets/role/owner_check.sql.erb b/templates/snippets/role/owner_check.sql.erb new file mode 100644 index 00000000..1bc9a194 --- /dev/null +++ b/templates/snippets/role/owner_check.sql.erb @@ -0,0 +1,4 @@ +IF NOT EXISTS( + SELECT p.name,r.name FROM sys.<%= @type.downcase %>_principals r + JOIN sys.<%= @type.downcase %>_principals p ON p.principal_id = r.owning_principal_id + WHERE r.type_desc = '<%= @type.upcase %>_ROLE' AND p.name = '<%= @authorization %>' AND r.name = '<%= @role %>')