Merge pull request #99 from cyberious/TSqlRefacter

(FM-2577) - Change from sqlcmd.exe to win32ole connector

Author: ferventcoder

.fixtures.yml

@@ -2,6 +2,5 @@ fixtures:
   forge_modules:
     acl: "puppetlabs/acl"
     stdlib: "puppetlabs/stdlib"
-    acl: "puppetlabs/acl"
   symlinks:
     sqlserver: "#{source_dir}"

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

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

lib/puppet/type/sqlserver_tsql.rb

@@ -32,9 +32,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
@@ -62,7 +60,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
 

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

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],

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

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

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

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

templates/create/login.sql.erb

@@ -47,4 +47,3 @@ BEGIN
 	<% end -%>
 END
 
-

templates/instance_config.erb

@@ -1 +1 @@
-{ 'instance': '<%= @instance_name %>','admin':'<%= @admin_user %>','pass':'<%= @admin_pass %>' }
+{ "instance": "<%= @instance_name %>","admin":"<%= @admin_user %>","pass":"<%= @admin_pass %>" }