Skip to content

(MODULES-5693) Replace SQLOLEDB with SQLNCLI11 #243

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 1 commit into from
Oct 23, 2017
Merged

(MODULES-5693) Replace SQLOLEDB with SQLNCLI11 #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 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 @@ -29,10 +29,11 @@ def open(config)

def get_connection_string(config)
params = {
'Provider' => 'SQLOLEDB.1',
'Initial Catalog' => config[:database] || 'master',
'Application Name' => 'Puppet',
'Data Source' => '.'
'Provider' => 'SQLNCLI11',
Copy link
Contributor

Choose a reason for hiding this comment

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

My only question here is... is it always the case that this new OLE DB provider is installed across all of the SQL versions by default (with no additional user provided features specified)? Looks like it requires the "SQL Server Native Client" feature... but I'm not totally sure where that lands in the list of features one might install?

I also see that we removed SNAC_SDK and added a note about Native Client here, but not sure I totally understand the implications:

puppetlabs-sqlserver/lib/puppet/provider/sqlserver_features/mssql.rb

Lines 18 to 22 in 108d109

# resources with the same name. Also due to the SQL Native Client not
# being unique across SQL Server versions (e.g. SQL 2016 installs Native Client
# with a version that matches for SQL 2012) the features need to be collated
# across all versions and then aggregated into a single resource
featurelist = []

It's also not clear to me just based on the name if SNAC_SDK actually contains the new OLEDB provider... or if it means it includes all the C++ headers to consume the providers from native code (which is usually what SDK means in this context).

As long as SQL 2012, 2014 and 2016 contain SQLNCLI11 with a minimal install I'm 👍

Copy link
Contributor

@Iristyle Iristyle Oct 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is why I'm asking btw:

(I presume above links are describing how you might get SQLNCLI11 onto an app server that talks to SQL Server, but doesn't have SQL Server installed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking into this / testing now. I don't know, offhand. Good catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm 99% sure native client is ALWAYS installed with the database engine. But SNAC_SDK is different. IIRC native client is part of the setup pre-reqs.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as the local Vagrant / Beaker test run was against SQL 2012, that should be sufficient to assert that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed that as long as the the SQL Engine is installed, SQL Native Client is too with MSSQL 2012, 2014, and 2016.

'Initial Catalog' => config[:database] || 'master',
'Application Name' => 'Puppet',
'Data Source' => '.',
'DataTypeComptibility' => 80
}

admin_user = config[:admin_user] || ''
Expand Down
12 changes: 6 additions & 6 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 @@ -17,7 +17,7 @@ def stub_connection
context 'command execution' do
before :each do
stub_connection
@connection.stubs(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.;User ID=sa;Password=Pupp3t1@')
@connection.stubs(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;User ID=sa;Password=Pupp3t1@')
end
it 'should not raise an error but populate has_errors with message' do
subject.stubs(:execute).raises(Exception.new("SQL Server\n error has happened"))
Expand All @@ -43,7 +43,7 @@ def stub_connection

context 'Use default authentication' do
it 'should defaul to SQL_LOGIN if admin_login_type is not set' do
@connection.expects(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.;User ID=sa;Password=Pupp3t1@')
@connection.expects(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;User ID=sa;Password=Pupp3t1@')
subject.open_and_run_command('query', {:admin_user => 'sa', :admin_pass => 'Pupp3t1@' })
end
end
Expand All @@ -66,11 +66,11 @@ def stub_connection
end

it 'should not add the default instance of MSSQLSERVER to connection string' do
@connection.expects(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.;User ID=sa;Password=Pupp3t1@')
@connection.expects(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;User ID=sa;Password=Pupp3t1@')
subject.open_and_run_command('query', {:admin_user => 'sa', :admin_pass => 'Pupp3t1@', :instance_name => 'MSSQLSERVER'})
end
it 'should add a non default instance to connection string' do
@connection.expects(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.\\LOGGING;User ID=sa;Password=Pupp3t1@')
@connection.expects(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.\\LOGGING;DataTypeComptibility=80;User ID=sa;Password=Pupp3t1@')
subject.open_and_run_command('query', {:admin_user => 'sa', :admin_pass => 'Pupp3t1@', :instance_name => 'LOGGING'})
end
end
Expand All @@ -93,12 +93,12 @@ def stub_connection
end

it 'should add integrated security to the connection string if admin and password are empty' do
@connection.expects(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.;Integrated Security=SSPI')
@connection.expects(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;Integrated Security=SSPI')
subject.open_and_run_command('query', {:admin_user => '', :admin_pass => '', :admin_login_type => 'WINDOWS_LOGIN'})
end

it 'should add integrated security to the connection string if admin and password are not defined' do
@connection.expects(:Open).with('Provider=SQLOLEDB.1;Initial Catalog=master;Application Name=Puppet;Data Source=.;Integrated Security=SSPI')
@connection.expects(:Open).with('Provider=SQLNCLI11;Initial Catalog=master;Application Name=Puppet;Data Source=.;DataTypeComptibility=80;Integrated Security=SSPI')
subject.open_and_run_command('query', { :admin_login_type => 'WINDOWS_LOGIN' })
end
end
Expand Down