Merge branch 'master' into stable

* master:
  (MODULES-3752) Fix modifying server_roles for an existing WINDOWS_LOGIN
  (BKR-914) Add workaround for beaker bug BKR-914
  (MODULES-3083) Fix modifying server roles for an existing login
  (MODULES-3256) Fix creating a login resource with additional attributes
  (MODULES-3256) Refactor sqlserver_login_spec acceptance test
  (MODULES-2554) Fix remove database
  (MODULES-2323) Fix deleting an SQL login
  (MODULES-3202) Fix install dependencies with custom source
  (MODULES-3256) Fix create/delete of Windows based logins
  (MODULES-3427) Allow multiple SYSADMIN role accounts
  (PA-285) Branding name change

Author: glennsarti

README.md

@@ -204,6 +204,10 @@ Installs and configures features such as SSMS and Master Data Service.
 
 * `pid`: *Optional.* Supplies a product key to configure which edition of SQL Server to use. Valid options: a string containing a valid product key. Default: undef (if not specified, SQL Server runs in Evaluation mode).
 
+* `source`: *Required.* Locates the SQL Server installer. Valid options: a string containing the path to an executable. Puppet must have permission to execute the installer.
+
+* `windows_feature_source`: *Optional.* Specifies the location of the Windows Feature source files.  This may be needed to install the .Net Framework. See https://support.microsoft.com/en-us/kb/2734782 for more information.
+
 Please note that if an option is set in both its own specific parameter and `install_switches`, the specifically named parameter takes precedence. For example, if you set the product key in both `pid` and in `install_switches`, SQL Server honors the `pid` parameter.
 
 For more information about installer switches and configuring SQL Server, see the links below:
@@ -259,6 +263,8 @@ Installs and configures a SQL Server instance.
 
 * `sql_sysadmin_accounts`: *Required.* Specifies one or more SQL or system accounts to receive sysadmin status. Valid options: an array containing one or more valid usernames.
 
+* `windows_feature_source`: *Optional.* Specifies the location of the Windows Feature source files.  This may be needed to install the .Net Framework. See https://support.microsoft.com/en-us/kb/2734782 for more information.
+
 Please note that if an option is set in both its own specific parameter and `install_switches`, the specifically named parameter takes precedence. For example, if you set the product key in both `pid` and in `install_switches`, SQL Server honors the `pid` parameter.
 
 For more information about installer switches and configuring SQL Server, see the links below:

lib/puppet/provider/sqlserver.rb

@@ -39,22 +39,32 @@ def not_nil_and_not_empty?(obj)
     !obj.nil? and !obj.empty?
   end
 
-  def self.run_install_dot_net
-    install_dot_net = <<-DOTNET
-Install-WindowsFeature  NET-Framework-Core
+  def self.run_install_dot_net(source_location = nil)
+    if (!source_location.nil?)
+      warn("The specified windows_source_location directory for sqlserver of \"#{source_location}\" does not exist") unless Puppet::FileSystem.directory?(source_location)
+    end
 
-Write-Host "Installing .Net Framework 3.5, do not close this prompt..."
-DISM /Online /Enable-Feature /FeatureName:NetFx3 /All /LimitAccess /Source:$LocalSource | Out-Null
+    install_dot_net = <<-DOTNET
 $Result = Dism /online /Get-featureinfo /featurename:NetFx3
 If($Result -contains "State : Enabled")
 {
-    Write-Host "Install .Net Framework 3.5 successfully."
+  Write-Host ".Net Framework 3.5 is already installed."
 }
 Else
 {
-    Write-Host "Failed to install Install .Net Framework 3.5,please make sure the local source is correct."
+  Write-Host "Installing .Net Framework 3.5, do not close this prompt..."
+  DISM /Online /Enable-Feature /FeatureName:NetFx3 /All /NoRestart /Quiet /LimitAccess #{ "/Source:\"#{source_location}\"" unless source_location.nil? } | Out-Null
+  $Result = Dism /online /Get-featureinfo /featurename:NetFx3
+  If($Result -contains "State : Enabled")
+  {
+      Write-Host "Install .Net Framework 3.5 successfully."
+  }
+  Else
+  {
+      Write-Host "Failed to install Install .Net Framework 3.5#{ ", please make sure the windows_feature_source is correct" unless source_location.nil?}."
+  }
 }
-    DOTNET
+DOTNET
     powershell([install_dot_net])
   end
 end

lib/puppet/provider/sqlserver_features/mssql.rb

@@ -99,16 +99,16 @@ def create_temp_for_install_switch
     return nil
   end
 
-  def installNet35
-    result = Puppet::Provider::Sqlserver.run_install_dot_net
+  def installNet35(source_location = nil)
+    result = Puppet::Provider::Sqlserver.run_install_dot_net(source_location)
   end
 
   def create
     if @resource[:features].empty?
       warn "Uninstalling all sql server features not tied into an instance because an empty array was passed, please use ensure absent instead."
       destroy
     else
-      installNet35
+      installNet35(@resource[:windows_feature_source])
       debug "Installing features #{@resource[:features]}"
       add_features(@resource[:features])
       @property_hash[:features] = @resource[:features]

lib/puppet/provider/sqlserver_instance/mssql.rb

@@ -57,16 +57,16 @@ def modify_features(features, action)
     end
   end
 
-  def installNet35
-    result = Puppet::Provider::Sqlserver.run_install_dot_net
+  def installNet35(source_location = nil)
+    result = Puppet::Provider::Sqlserver.run_install_dot_net(source_location)
   end
 
   def create
     if @resource[:features].empty?
       warn "Uninstalling all features for instance #{@resource[:name]} because an empty array was passed, please use ensure absent instead."
       destroy
     else
-      installNet35
+      installNet35(@resource[:windows_feature_source])
       add_features(@resource[:features])
       # cmd_args = build_cmd_args(@resource[:features])
       # begin
@@ -125,16 +125,32 @@ def build_cmd_args(features, action="install")
         end
       end
 
-      format_cmd_args_array('/SQLSYSADMINACCOUNTS', @resource[:sql_sysadmin_accounts], cmd_args)
+      format_cmd_args_array('/SQLSYSADMINACCOUNTS', @resource[:sql_sysadmin_accounts], cmd_args, true)
       format_cmd_args_array('/ASSYSADMINACCOUNTS', @resource[:as_sysadmin_accounts], cmd_args)
     end
     cmd_args
   end
 
-  def format_cmd_args_array(switch, arr, cmd_args)
+  def format_cmd_args_array(switch, arr, cmd_args, use_discrete = false)
     if not_nil_and_not_empty? arr
       arr = [arr] if !arr.kind_of?(Array)
-      cmd_args << "#{switch}=#{arr.collect { |item| "\"#{item}\"" }.join(' ')}"
+
+      # The default action is to join the array elements with a space ' ' so the cmd_args ends up like;
+      # ["/SWITCH=\"Element1\" \"Element2\""]
+      # Whereas if use_discrete is set, the args are appended as discrete elements in the cmd_args array e.g.;
+      # ["/SWITCH=\"Element1\"","\"Element2\""]
+
+      if use_discrete
+        arr.map.with_index { |var,i|
+          if i == 0
+            cmd_args << "#{switch}=\"#{var}\""
+          else
+            cmd_args << "\"#{var}\""
+          end
+        }
+      else
+        cmd_args << "#{switch}=#{arr.collect { |item| "\"#{item}\"" }.join(' ')}"
+      end
     end
   end
 

lib/puppet/type/sqlserver_features.rb

@@ -14,6 +14,11 @@
 
   newparam(:source)
 
+  newparam(:windows_feature_source) do
+    desc 'Specify the location of the Windows Feature source files.  This may be needed to install the .Net Framework.
+          See https://support.microsoft.com/en-us/kb/2734782 for more information.'
+  end
+
   newparam(:pid) do
     desc 'Specify the SQL Server product key to configure which edition you would like to use.'
 

lib/puppet/type/sqlserver_instance.rb

@@ -15,6 +15,11 @@
 
   end
 
+  newparam(:windows_feature_source) do
+    desc 'Specify the location of the Windows Feature source files.  This may be needed to install the .Net Framework.
+          See https://support.microsoft.com/en-us/kb/2734782 for more information.'
+  end
+
   newparam(:pid) do
     desc 'Specify the SQL Server product key to configure which edition you would like to use.'
 

spec/acceptance/sqlserver_database_spec.rb

@@ -29,9 +29,7 @@ def run_sql_query_opts (query, expected_row_count)
     end
 
     after(:each) do
-
       # delete created database:
-
       pp = <<-MANIFEST
         sqlserver::config{'MSSQLSERVER':
           admin_user    => 'sa',
@@ -41,8 +39,7 @@ def run_sql_query_opts (query, expected_row_count)
           ensure  => 'absent',
         }
       MANIFEST
-      #comment out the below line because of ticket MODULES-2554.
-      #ensure_sqlserver_database(host, pp)
+      ensure_sqlserver_database(host, pp)
     end
 
     it "Test Case C89019: Create a database" do
@@ -67,6 +64,37 @@ def run_sql_query_opts (query, expected_row_count)
       run_sql_query(host, run_sql_query_opts(query, 1))
     end
 
+    it "Delete a database" do
+      pp = <<-MANIFEST
+        sqlserver::config{'MSSQLSERVER':
+          admin_user    => 'sa',
+          admin_pass    => 'Pupp3t1@',
+        }
+        sqlserver::database{'#{@db_name}':
+        }
+      MANIFEST
+      ensure_sqlserver_database(host, pp)
+
+      puts "Validate the Database '#{@db_name}' exists"
+      query = "SELECT database_id from sys.databases WHERE name = '#{@db_name}';"
+      run_sql_query(host, run_sql_query_opts(query, 1))
+
+      pp = <<-MANIFEST
+        sqlserver::config{'MSSQLSERVER':
+          admin_user    => 'sa',
+          admin_pass    => 'Pupp3t1@',
+        }
+        sqlserver::database{'#{@db_name}':
+          ensure => absent,
+        }
+      MANIFEST
+      ensure_sqlserver_database(host, pp)
+
+      puts "Validate the Database '#{@db_name}' does not exist"
+      query = "SELECT database_id from sys.databases WHERE name = '#{@db_name}';"
+      run_sql_query(host, run_sql_query_opts(query, 0))
+    end
+
     it "Test Case C89076: Create database with optional collation_name" do
       pp = <<-MANIFEST
         sqlserver::config{'MSSQLSERVER':

spec/acceptance/sqlserver_instance_spec.rb

@@ -11,15 +11,16 @@ def new_random_instance_name
 describe "sqlserver_instance", :node => host do
   version = host['sql_version'].to_s
 
-
-  def ensure_sqlserver_instance(host, features, inst_name, ensure_val = 'present')
+  def ensure_sqlserver_instance(host, features, inst_name, ensure_val = 'present', sysadmin_accounts = "['Administrator']")
     manifest = <<-MANIFEST
     sqlserver_instance{'#{inst_name}':
       name                  => '#{inst_name}',
       ensure                => <%= ensure_val %>,
       source                => 'H:',
+      security_mode         => 'SQL',
+      sa_pwd                => 'Pupp3t1@',
       features              => [ <%= mssql_features %> ],
-      sql_sysadmin_accounts => ['Administrator'],
+      sql_sysadmin_accounts => #{sysadmin_accounts},
       agt_svc_account       => 'Administrator',
       agt_svc_password      => 'Qu@lity!',
     }
@@ -34,18 +35,69 @@ def ensure_sqlserver_instance(host, features, inst_name, ensure_val = 'present')
     end
   end
 
+  #Return options for run_sql_query
+  def run_sql_query_opts (inst_name, query, expected_row_count)
+    run_sql_query_opt = {
+        :query => query,
+        :instance => inst_name,
+        :server => '.',
+        :sql_admin_user => 'sa',
+        :sql_admin_pass => 'Pupp3t1@',
+        :expected_row_count => expected_row_count,
+    }
+  end
+
+  def sql_query_is_user_sysadmin(username)
+    <<-QUERY
+    Select [Name]
+      FROM SYS.Server_Principals
+      WHERE (type = 'S' or type = 'U')
+      AND [Name] like '%\\#{username}'
+      AND is_disabled = 0 AND IS_SRVROLEMEMBER('sysadmin', name) = 1;
+    QUERY
+  end
+
   context "Create an instance", {:testrail => ['88978', '89028', '89031', '89043', '89061']} do
+
+    before(:context) do
+      @ExtraAdminUser = 'ExtraSQLAdmin'
+      pp = <<-MANIFEST
+      user { '#{@ExtraAdminUser}':
+        ensure => present,
+        password => 'Puppet01!',
+      }
+      MANIFEST
+      apply_manifest_on(host,pp)
+    end
+
+    after(:context) do
+      pp = <<-MANIFEST
+      user { '#{@ExtraAdminUser}':
+        ensure => absent,
+      }
+      MANIFEST
+      apply_manifest_on(host,pp)
+    end
+
     inst_name = new_random_instance_name
     features = ['SQL', 'SQLEngine', 'Replication', 'FullText', 'DQ']
 
     it "create #{inst_name} instance" do
-      ensure_sqlserver_instance(host, features, inst_name)
+      ensure_sqlserver_instance(host, features, inst_name,'present',"['Administrator','ExtraSQLAdmin']")
 
       validate_sql_install(host, {:version => version}) do |r|
         expect(r.stdout).to match(/#{Regexp.new(inst_name)}/)
       end
     end
 
+    it "#{inst_name} instance has Administrator as a sysadmin" do
+      run_sql_query(host, run_sql_query_opts(inst_name, sql_query_is_user_sysadmin('Administrator'), expected_row_count = 1))
+    end
+
+    it "#{inst_name} instance has ExtraSQLAdmin as a sysadmin" do
+      run_sql_query(host, run_sql_query_opts(inst_name, sql_query_is_user_sysadmin('ExtraSQLAdmin'), expected_row_count = 1))
+    end
+
     it "remove #{inst_name} instance" do
       ensure_sqlserver_instance(host, features, inst_name, 'absent')