From 7e5fc9d35ef2b5b0703901f7d175f39b4b4da3d8 Mon Sep 17 00:00:00 2001 From: Travis Fields Date: Mon, 11 May 2015 17:43:41 -0700 Subject: [PATCH 1/3] (fix) - Remove duplicate acl dependency in .fixtures.yml --- .fixtures.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.fixtures.yml b/.fixtures.yml index a5f7e87f..70348761 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -2,6 +2,5 @@ fixtures: forge_modules: acl: "puppetlabs/acl" stdlib: "puppetlabs/stdlib" - acl: "puppetlabs/acl" symlinks: sqlserver: "#{source_dir}" From d1a29d9023c12b452effe7dea2c9b559481ae895 Mon Sep 17 00:00:00 2001 From: Travis Fields Date: Mon, 11 May 2015 20:10:24 -0700 Subject: [PATCH 2/3] (maint) Move config file to vardir instead of C:\Program Files\ --- lib/puppet/provider/sqlserver.rb | 2 +- manifests/config.pp | 10 +++------- spec/defines/config_spec.rb | 12 ++++++------ 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/puppet/provider/sqlserver.rb b/lib/puppet/provider/sqlserver.rb index eabb84be..dc6cc212 100644 --- a/lib/puppet/provider/sqlserver.rb +++ b/lib/puppet/provider/sqlserver.rb @@ -30,7 +30,7 @@ def try_execute(command, msg = nil) ## def self.run_authenticated_sqlcmd(query, opts) b = binding - @sql_instance_config = "C:/Program Files/Microsoft SQL Server/.puppet/.#{opts[:instance_name]}.cfg" + @sql_instance_config = File.join(Puppet[:vardir], "cache/sqlserver/.#{resource[:instance]}.cfg") if File.exists?(@sql_instance_config) @sql_instance_config = native_path(@sql_instance_config) else diff --git a/manifests/config.pp b/manifests/config.pp index 640782fa..28f26949 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -24,14 +24,10 @@ $instance_name = $title, ) { #possible future parameter if we do end up supporting different install directories - $install_dir ='C:/Program Files/Microsoft SQL Server' - $config_dir = "${install_dir}/.puppet" + $config_dir = "${::puppet_vardir}/cache/sqlserver" $config_file = "${config_dir}/.${instance_name}.cfg" - if !defined(File[$config_dir]){ - file{ $config_dir: - ensure => directory - } - } + ensure_resource('file', ["${::puppet_vardir}/cache",$config_dir], { 'ensure' => 'directory','recurse' => 'true' }) + file{ $config_file: content => template('sqlserver/instance_config.erb'), require => File[$config_dir], diff --git a/spec/defines/config_spec.rb b/spec/defines/config_spec.rb index 3c2c7017..def67c9c 100644 --- a/spec/defines/config_spec.rb +++ b/spec/defines/config_spec.rb @@ -4,15 +4,15 @@ RSpec.describe 'sqlserver::config', :type => :define do let(:title) { 'MSSQLSERVER' } let(:params) { { - :instance_name => 'MSSQLSERVER', - :admin_user => 'sa', - :admin_pass => 'Pupp3t1@', + :instance_name => 'MSSQLSERVER', + :admin_user => 'sa', + :admin_pass => 'Pupp3t1@', } } - let(:facts) { {:osfamily => 'windows', :platform => :windows} } + let(:facts) { {:osfamily => 'windows', :platform => :windows, :puppet_vardir => 'C:/ProgramData/PuppetLabs/puppet/var'} } describe 'compile' do it { - should contain_file('C:/Program Files/Microsoft SQL Server/.puppet/.MSSQLSERVER.cfg') - should contain_file('C:/Program Files/Microsoft SQL Server/.puppet') + should contain_file('C:/ProgramData/PuppetLabs/puppet/var/cache/sqlserver/.MSSQLSERVER.cfg') + should contain_file('C:/ProgramData/PuppetLabs/puppet/var/cache/sqlserver') } end end From 9f24c0cd078a21f07d071aa2c225432da773f695 Mon Sep 17 00:00:00 2001 From: Travis Fields Date: Mon, 27 Apr 2015 14:34:03 -0700 Subject: [PATCH 3/3] (FM-2577) - Change from sqlcmd.exe to win32ole connector - remove dependency to sqlcmd.exe and temp files, gives us more control and ability to get error messages back - Login contains GO in sql query which breaks with new method - Removes GO and adds the TRY and CATCH used throughout - Change config to double quotes, Ruby does not parse JSON with single quotes --- lib/puppet/provider/sqlserver_tsql/mssql.rb | 32 +++- lib/puppet/type/sqlserver_tsql.rb | 9 +- lib/puppet_x/sqlserver/sql_connection.rb | 137 ++++++++++++++++++ spec/functions/sqlserver_upcase_spec.rb | 1 - ...qlserver_validate_hash_uniq_values_spec.rb | 1 - spec/unit/puppet_x/sql_connection_spec.rb | 85 +++++++++++ templates/create/login.sql.erb | 1 - templates/instance_config.erb | 2 +- 8 files changed, 252 insertions(+), 16 deletions(-) create mode 100644 lib/puppet_x/sqlserver/sql_connection.rb create mode 100644 spec/unit/puppet_x/sql_connection_spec.rb diff --git a/lib/puppet/provider/sqlserver_tsql/mssql.rb b/lib/puppet/provider/sqlserver_tsql/mssql.rb index 373f4cba..c49dd110 100644 --- a/lib/puppet/provider/sqlserver_tsql/mssql.rb +++ b/lib/puppet/provider/sqlserver_tsql/mssql.rb @@ -1,17 +1,33 @@ require File.expand_path(File.join(File.dirname(__FILE__), '..', 'sqlserver')) +require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'puppet_x/sqlserver/sql_connection')) Puppet::Type::type(:sqlserver_tsql).provide(:mssql, :parent => Puppet::Provider::Sqlserver) do - def run(query, opts) - debug("Running resource #{query} against #{resource[:instance]} with failonfail set to #{opts[:failonfail]}") - opts[:instance_name] = resource[:instance] - result = Puppet::Provider::Sqlserver.run_authenticated_sqlcmd(query, opts) - return result + + def run(query) + debug("Running resource #{query} against #{resource[:instance]}") + config = get_instance_config + sqlconn = PuppetX::Sqlserver::SqlConnection.new + + sqlconn.open_and_run_command(query, config) end - def run_update - result = self.run(resource[:command], {:failonfail => true}) - return result + def get_instance_config + config_file = File.join(Puppet[:vardir], "cache/sqlserver/.#{resource[:instance]}.cfg") + if !File.exists? (config_file) + fail('Required config file missing, add the appropriate sqlserver::config and rerun') + end + if !File.readable?(config_file) + fail('Unable to read config file, ensure proper permissions and please try again') + end + JSON.parse(File.read(config_file)) end + def run_check + return self.run(resource[:onlyif]) + end + + def run_update + return self.run(resource[:command]) + end end diff --git a/lib/puppet/type/sqlserver_tsql.rb b/lib/puppet/type/sqlserver_tsql.rb index c6794a80..241fcd2e 100644 --- a/lib/puppet/type/sqlserver_tsql.rb +++ b/lib/puppet/type/sqlserver_tsql.rb @@ -31,9 +31,7 @@ def self.checks newcheck(:onlyif, :parent => Puppet::Property::SqlserverTsql) do #Runs in the event that our TSQL exits with anything other than 0 def check(value) - begin - output = provider.run(value, :failonfail => false) - end + output = provider.run(value) debug("OnlyIf returned exitstatus of #{output.exitstatus}") output.exitstatus != 0 end @@ -61,7 +59,10 @@ def output def refresh if self.check_all_attributes(true) - provider.run_update + result = provider.run_update + if result.has_errors + fail("Unable to apply changes, failed with error message #{result.error_message}") + end end end diff --git a/lib/puppet_x/sqlserver/sql_connection.rb b/lib/puppet_x/sqlserver/sql_connection.rb new file mode 100644 index 00000000..61921769 --- /dev/null +++ b/lib/puppet_x/sqlserver/sql_connection.rb @@ -0,0 +1,137 @@ +module PuppetX + module Sqlserver + + CONNECTION_CLOSED = 0 + + class SqlConnection + attr_reader :exception_caught + + + def initialize + @connection = nil + @data = nil + end + + def open_and_run_command(query, config) + begin + open(config) + command(query) + ensure + close + end + + result + end + + private + def connection + @connection ||= create_connection + end + + def open(config) + connection_string = get_connection_string(config) + connection.Open(connection_string) unless !connection_closed? + end + + def get_connection_string(config) + config = {'database' => 'master'}.merge(config) + # Open ADO connection to the SQL Server database + connection_string = "Provider=SQLOLEDB.1;" + connection_string << "Persist Security Info=False;" + connection_string << "User ID=#{config['admin']};" + connection_string << "password=#{config['pass']};" + connection_string << "Initial Catalog=#{config['database']};" + connection_string << "Application Name=Puppet;" + if config['instance'] !~ /^MSSQLSERVER$/ + connection_string << "Data Source=localhost\\#{config['instance']};" + else + connection_string << "Data Source=localhost;" + end + end + + def command(sql) + reset_instance + begin + r = execute(sql) + yield(r) if block_given? + rescue win32_exception => e + @exception_caught = e + end + nil + end + + def result + ResultOutput.new(has_errors, error_message) + end + + def has_errors + @exception_caught != nil + end + + def error_message + @exception_caught.message unless @exception_caught == nil + end + + def close + begin + connection.Close unless connection_closed? + rescue win32_exception => e + end + end + + def reset_instance + @data = nil + @fields = nil + @exception_caught = nil + end + + def connection_closed? + connection.State == CONNECTION_CLOSED + end + + def create_connection + require 'win32ole' + WIN32OLE.new('ADODB.Connection') + end + + def execute (sql) + connection.Execute(sql, nil, nil) + end + + def parse_column_names(result) + result.Fields.extend(Enumerable).map { |column| column.Name } + end + + # having as a method instead of hard coded allows us to stub and test outside of Windows + def win32_exception + ::WIN32OLERuntimeError + end + + def connection=(conn) + @connection = conn + end + end + + class ResultOutput + attr_reader :exitstatus, :error_message, :raw_error_message + + def initialize(has_errors, error_message) + @exitstatus = has_errors ? 1 : 0 + if error_message + @raw_error_message = error_message + @error_message = parse_for_error(error_message) + end + end + + def has_errors + @exitstatus != 0 + end + + private + def parse_for_error(result) + match = result.match(/SQL Server\n\s+(.*)/i) + match[1] unless match == nil + end + end + end +end diff --git a/spec/functions/sqlserver_upcase_spec.rb b/spec/functions/sqlserver_upcase_spec.rb index 47312687..70b26b37 100644 --- a/spec/functions/sqlserver_upcase_spec.rb +++ b/spec/functions/sqlserver_upcase_spec.rb @@ -1,4 +1,3 @@ -#! /usr/bin/env ruby -S rspec require 'spec_helper' describe "the sqlserver_upcase function" do diff --git a/spec/functions/sqlserver_validate_hash_uniq_values_spec.rb b/spec/functions/sqlserver_validate_hash_uniq_values_spec.rb index 68485600..4c339410 100644 --- a/spec/functions/sqlserver_validate_hash_uniq_values_spec.rb +++ b/spec/functions/sqlserver_validate_hash_uniq_values_spec.rb @@ -1,4 +1,3 @@ -#! /usr/bin/env ruby -S rspec require 'spec_helper' describe "the sqlserver_validate_hash_uniq_values" do diff --git a/spec/unit/puppet_x/sql_connection_spec.rb b/spec/unit/puppet_x/sql_connection_spec.rb new file mode 100644 index 00000000..2e4006f6 --- /dev/null +++ b/spec/unit/puppet_x/sql_connection_spec.rb @@ -0,0 +1,85 @@ +require 'rspec' +require 'spec_helper' +require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'lib/puppet_x/sqlserver/sql_connection')) + +RSpec.describe PuppetX::Sqlserver::SqlConnection do + let(:subject) { PuppetX::Sqlserver::SqlConnection.new } + let(:config) { {'admin' => 'sa', 'pass' => 'Pupp3t1@', 'instance' => 'MSSQLSERVER'} } + + def stub_connection + @connection = mock() + subject.stubs(:create_connection).returns(@connection) + subject.stubs(:win32_exception).returns(Exception) + end + + def stub_no_errors + subject.stubs(:has_errors).returns(false) + subject.stubs(:error_message).returns(nil) + end + + describe 'open_and_run_command' do + context 'command' do + before :each do + stub_connection + @connection.stubs(:State).returns(0) + @connection.stubs(:Open).with('Provider=SQLOLEDB.1;Persist Security Info=False;User ID=sa;password=Pupp3t1@;Initial Catalog=master;Application Name=Puppet;Data Source=localhost;') + end + it 'should not raise an error but populate has_errors with message' do + subject.stubs(:win32_exception).returns(Exception) + subject.stubs(:execute).raises(Exception.new("SQL Server\n error has happened")) + expect { + result = subject.open_and_run_command('whacka whacka whacka', config) + expect(result.exitstatus).to eq(1) + expect(result.error_message).to eq('error has happened') + }.to_not raise_error(Exception) + + end + it 'should yield when passed a block' do + subject.stubs(:execute).returns('results') + subject.open_and_run_command('myquery', config) do |r| + expect(r).to eq('results') + end + end + end + context 'closed connection' do + before :each do + stub_connection + stub_no_errors + @connection.stubs(:State).returns(0) + end + it 'should not add MSSQLSERVER to connection string' do + @connection.stubs(:Open).with('Provider=SQLOLEDB.1;Persist Security Info=False;User ID=sa;password=Pupp3t1@;Initial Catalog=master;Application Name=Puppet;Data Source=localhost;') + subject.open_and_run_command('query', {'admin' => 'sa', 'pass' => 'Pupp3t1@', 'instance' => 'MSSQLSERVER'}) + end + it 'should add a non default instance to connection string' do + @connection.stubs(:Open).with('Provider=SQLOLEDB.1;Persist Security Info=False;User ID=superuser;password=puppetTested;Initial Catalog=master;Application Name=Puppet;Data Source=localhost\LOGGING;') + subject.open_and_run_command('query', {'admin' => 'superuser', 'pass' => 'puppetTested', 'instance' => 'LOGGING'}) + end + end + context 'open connection' do + it 'should not reopen an existing connection' do + stub_connection + @connection.expects(:open).never + @connection.stubs(:State).returns(1) + @connection.expects(:Execute).with('query', nil, nil) + subject.open_and_run_command('query', {'admin' => 'sa', 'pass' => 'Pupp3t1@', 'instance' => 'MSSQLSERVER'}) + end + end + context 'return result with errors' do + it { + subject.expects(:open).with({'admin' => 'sa', 'pass' => 'Pupp3t1@', 'instance' => 'MSSQLSERVER'}) + subject.expects(:command).with('SELECT * FROM sys.databases') + subject.expects(:close).once + subject.stubs(:has_errors).returns(:true) + subject.stubs(:error_message).returns( + 'SQL Server + invalid syntax provider') + result = + subject.open_and_run_command('SELECT * FROM sys.databases', + {'instance' => 'MSSQLSERVER', 'admin' => 'sa', 'pass' => 'Pupp3t1@'}) + expect(result.exitstatus).to eq(1) + expect(result.error_message).to eq('invalid syntax provider') + } + end + end +end diff --git a/templates/create/login.sql.erb b/templates/create/login.sql.erb index 290c49c6..1670eb67 100644 --- a/templates/create/login.sql.erb +++ b/templates/create/login.sql.erb @@ -47,4 +47,3 @@ BEGIN <% end -%> END - diff --git a/templates/instance_config.erb b/templates/instance_config.erb index a0defafd..de5d3888 100644 --- a/templates/instance_config.erb +++ b/templates/instance_config.erb @@ -1 +1 @@ -{ 'instance': '<%= @instance_name %>','admin':'<%= @admin_user %>','pass':'<%= @admin_pass %>' } +{ "instance": "<%= @instance_name %>","admin":"<%= @admin_user %>","pass":"<%= @admin_pass %>" }