Skip to content

(FM-2577) Minor SQL server connection building refactorings #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 13, 2015
Merged

(FM-2577) Minor SQL server connection building refactorings #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b09b670
(FM-2577) Remove run_authenticated_sqlcmd
Iristyle May 13, 2015
0e63dde
(maint) Remove unnecessary sql_connection code
Iristyle May 13, 2015
01e6cb6
(maint) Refactor connection string building
Iristyle May 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions lib/puppet/provider/sqlserver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,53 +25,6 @@ def try_execute(command, msg = nil)
end
end

##
# Used by tsql provider
##
def self.run_authenticated_sqlcmd(query, opts)
b = binding
@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
raise Puppet::ParseError, "Config file does not exist"
end
temp = Tempfile.new(['puppet', '.sql'])
begin
temp.write(query)
temp.flush
temp.close
#input file is used in the authenticated_query.ps1.erb template
input_file = native_path(temp.path)
@instance = opts[:instance_name]
erb_template = File.join(template_path, 'authenticated_query.ps1.erb')
ps1 = ERB.new(File.new(erb_template).read, nil, '-')
temp_ps1 = Tempfile.new(['puppet', '.ps1'])
begin
temp_ps1.write(ps1.result(b))
temp_ps1.flush
temp_ps1.close
#We want to not fail the exec but fail the overall process once we get the clean result back, otherwise we report the temp file which is meaningless
result = Puppet::Util::Execution.execute(['powershell.exe', '-noprofile', '-executionpolicy', 'unrestricted', temp_ps1.path], {:failonfail => false}) #We expect some things to fail in order to run as an only if
debug("Return result #{result}")
if opts[:failonfail] && result.match(/THROW CAUGHT/)
fail(result.gsub('THROW CAUGHT:', ''))
end
if result.match(/Msg \d+, Level 16/)
fail(result)
end
return result
ensure
temp_ps1.close
temp_ps1.unlink
end
ensure
temp.close
temp.unlink
end
return result
end

private
def self.native_path(path)
path.gsub(File::SEPARATOR, File::ALT_SEPARATOR)
Expand Down
48 changes: 0 additions & 48 deletions lib/puppet/templates/authenticated_query.ps1.erb
View file
Open in desktop

This file was deleted.

34 changes: 11 additions & 23 deletions lib/puppet_x/sqlserver/sql_connection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ module Sqlserver
class SqlConnection
attr_reader :exception_caught


def initialize
@connection = nil
@data = nil
end

def open_and_run_command(query, config)
begin
open(config)
Expand All @@ -34,19 +28,19 @@ def open(config)
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;"
params = {
'Provider' => 'SQLOLEDB.1',
'User ID' => config['admin'],
'Password' => config['pass'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Persist Security Info and tests failing as a result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha - will fixup test. Note that I removed Persist Security Info since it has been the default for a long time.

'Initial Catalog' => config['database'] || 'master',
'Application Name' => 'Puppet',
'Data Source' => 'localhost'
}
if config['instance'] !~ /^MSSQLSERVER$/
connection_string << "Data Source=localhost\\#{config['instance']};"
else
connection_string << "Data Source=localhost;"
params['Data Source'] = "localhost\\#{config['instance']}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line was lost, never to be found again. @Iristyle @cyberious was that intentional?

end

params.map { |k, v| "#{k}=#{v}" }.join(';')
end

def command(sql)
Expand Down Expand Up @@ -80,8 +74,6 @@ def close
end

def reset_instance
@data = nil
@fields = nil
@exception_caught = nil
end

Expand All @@ -106,10 +98,6 @@ def parse_column_names(result)
def win32_exception
::WIN32OLERuntimeError
end

def connection=(conn)
@connection = conn
end
end

class ResultOutput
Expand Down
6 changes: 3 additions & 3 deletions spec/unit/puppet_x/sql_connection_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def stub_no_errors
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;')
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;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)
Expand All @@ -48,11 +48,11 @@ def 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;')
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;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;')
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;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
Expand Down