Merge branch 'main' into instancecheckscontinued

SQLDBAWithABeard · SQLDBAWithABeard · commit 16638f9f8784 · 2023-08-03T16:04:39.000Z
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -24,7 +24,8 @@
                 "TylerLeonhardt.vscode-inline-values-powershell",
                 "cschleiden.vscode-github-actions",
                 "ms-mssql.mssql",
-                "Gruntfuggly.todo-tree"
+                "Gruntfuggly.todo-tree",
+                "streetsidesoftware.code-spell-checker"
             ],
             "settings": {
                 "editor.renderWhitespace": "all",
diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml
@@ -1,7 +1,7 @@
 version: "3"
 services:
     dbachecks1:
-      image: dbachecks/sqlinstance1:v2.36.0
+      image: dbachecks/sqlinstance1:v2.38.0
       volumes:
         - mydata:/var/opt/backups
         - shared:/shared
@@ -13,7 +13,7 @@ services:
       hostname: dbachecks1
 
     dbachecks2:
-      image: dbachecks/sqlinstance2:v2.36.0
+      image: dbachecks/sqlinstance2:v2.38.0
       volumes:
         - mydata:/var/opt/backups
         - shared:/shared
@@ -27,7 +27,7 @@ services:
 
       # This is our SQL2022 container
     dbachecks3:
-      image: dbachecks/sqlinstance3:v2.36.0
+      image: dbachecks/sqlinstance3:v2.38.0
       volumes:
         - mydata:/var/opt/backups
         - shared:/shared
diff --git a/.github/workflows/PR.yml b/.github/workflows/PR.yml
@@ -59,7 +59,7 @@ jobs:
         path: ${{ env.buildFolderName }}
     - name: Run Tests
       shell: pwsh
-      run: ./build.ps1 -tasks noop; ipmo dbatools ; ./build.ps1 -tasks test # to get around dbatools failing to load XE.core.dll if
+      run: ./build.ps1 -tasks noop ; ./build.ps1 -tasks test # to get around dbatools failing to load XE.core.dll if
     - name: Publish Test Artifact
       uses: actions/upload-artifact@v3
       with:
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -47,5 +47,8 @@
     "[markdown]": {
         "editor.trimAutoWhitespace": false,
         "files.trimTrailingWhitespace": false
-    }
-}
+    },
+    "cSpell.enableFiletypes": [
+        "powershell"
+    ]
+}
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,7 +1,171 @@
 # Contributing
 
-## TODO
+## Welcome
+
+Before we go any further, thanks for being here. Thanks for using dbachecks and especially thanks 
+for being here and looking into how you can help!
+
+## Important resources
+
+- docs
+- bugs
+- communicate with the team
+    - slack
+    - github discussions?
+- presentations\blogs?
 
 ## Running the Tests
 
 If want to know how to run this module's tests you can look at the [Testing Guidelines](https://dsccommunity.org/guidelines/testing-guidelines/#running-tests)
+
+## Environment details
+
+We strongly believe that 'every repo should have a devcontainer' and therefore we've built one
+for this project that includes 3 SQL Servers and everything you need to develop and build the 
+dbachecks module.
+
+It's magic!
+
+### Prerequisites:
+
+In order to use the devcontainer there are a few things you need to get started.
+
+- [Docker](https://www.docker.com/get-started)
+- [git](https://git-scm.com/downloads)
+- [VSCode](https://code.visualstudio.com/download)
+- [Remote Development Extension for VSCode](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.vscode-remote-extensionpack)
+
+### Setup
+
+Once the prerequisites are in place follow these steps to download the repo and start up the
+devcontainer. The first time you build the devcontainer it will need to pull down the images
+so that could take a hot second depending on your internet speeds.
+
+1. Download the repo from GitHub
+    ```PowerShell
+    # change directory to where you'd like the repo to go
+    cd C:\GitHub\
+
+    # clone the repo from GitHub
+    git clone https://github.com/dataplat/dbachecks
+
+    # move into the folder
+    cd .\dbachecks\
+
+    # open VSCode
+    code .
+    ```
+
+754662. Once code opens, there should be a toast in the bottom right that suggests you 'ReOpen in Container'.
+1. The first time you do this it may take a little, and you'll need an internet connection, as it'll download the container images used in our demos
+
+### Develop & Build
+
+We are using the [Sampler](https://github.com/gaelcolas/Sampler) Powershell Module to structure our module.
+This makes it easier to develop and test the module locally.
+
+The workflow for using this and developing the code - for example to add a new Database level check you could follow
+this guide.
+
+1. Download the repo locally and create a new branch to develop on
+    ```PowerShell
+    git checkout -b newStuff # give it a proper name!
+    ```
+
+1. Develop in the source repository, to add a check you need to add the following code:
+  - add check code to `source/checks/DatabaseV5.Tests.ps1`
+  - add required configurations to `source/internal/configurations/configuration.ps1`
+    - `skip.database.checkName`
+    - `policy.database.checkNameExcludeDb`
+  - add required properties to object info to `source/internal/functions/Get-AllDatabaseInfo.ps1`
+
+1. Build the module
+    ```PowerShell
+    ./build.ps1 -Tasks build
+    ```
+
+1. Sampler automatically adds the new version to your path you can prove that with the following code:
+    ```PowerShell
+    get-module dbachecks -ListAvailable | Select-Object Name, ModuleBase
+    ```
+
+1. Import new version of the module
+    ```PowerShell
+    Import-Module dbachecks -force
+    ```
+
+1. Test out the new code
+    ```PowerShell
+    # save the password to make for easy connections
+    $password = ConvertTo-SecureString "dbatools.IO" -AsPlainText -Force
+    $cred = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList "sqladmin", $password
+
+    $show = 'All'
+    $checks = 'RecoveryModel' # <-- change this to your new check name 
+
+    $sqlinstances = 'localhost,7401', 'localhost,7402', 'localhost,7403'
+    #$sqlinstances = 'dbachecks1', 'dbachecks2', 'dbachecks3' # need client aliases for this to work New-DbaClientAlias
+
+    # Run v5 checks
+    $v5code = Invoke-DbcCheck -SqlInstance $Sqlinstances -SqlCredential $cred -Check $Checks -legacy $false -Show $show -PassThru -Verbose
+    ```
+
+1. If you are working on the v4 --> v5 upgrade you can also confirm your v5 test results match v4 with the following
+    ```PowerShell
+    # save the password to make for easy connections
+    $password = ConvertTo-SecureString "dbatools.IO" -AsPlainText -Force
+    $cred = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList "sqladmin", $password
+
+    $show = 'All'
+    $checks = 'RecoveryModel' # <-- change this to your new check name 
+
+    $sqlinstances = 'localhost,7401', 'localhost,7402', 'localhost,7403'
+    #$sqlinstances = 'dbachecks1', 'dbachecks2', 'dbachecks3' # need client aliases for this to work New-DbaClientAlias
+
+    # Check results of the tests - are we testing the same things with the same results for v4 & v5
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks
+    # Include the specific details for the perf testing
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks -PerfDetail
+    # Include the test results - this helps troubleshooting if your tests aren't the same
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks -showTestResults
+    ```
+
+1. Once you are happy with your code, push your branch to GitHub and create a PR against the dbachecks repo.
+
+1. Thanks!
+
+### Rebuild your devcontainer
+
+The only way to properly rebuild to ensure that all volumes etc are removed is to open up a console
+or PowerShell window outside of the devcontainer and run the following:
+
+```PowerShell
+    cd \path-of-dbachecks-folder\.devcontainer 
+
+    docker-compose -f "docker-compose.yml" -p "bitsdbatools_devcontainer" down
+```
+
+## How to submit changes: 
+TODO:
+Pull Request protocol etc. You might also include what response they'll get back from the team on submission, or any caveats about the speed of response.
+
+## How to report a bug: 
+TODO:
+Bugs are problems in code, in the functionality of an application or in its UI design; you can submit them through "bug trackers" and most projects invite you to do so, so that they may "debug" with more efficiency and the input of a contributor. Take a look at Atom's example for how to teach people to report bugs to your project.
+
+## Templates:
+TODO: 
+in this section of your file, you might also want to link to a bug report "template" like this one here which contributors can copy and add context to; this will keep your bugs tidy and relevant.
+
+## Style Guide
+TODO:
+include extensions and vscode settings we use to keep things neat
+
+## Code of Conduct
+TODO: maybe beef this out - stolen from data sat repo for now.
+
+We expect and demand that you follow some basic rules. Nothing dramatic here. There will be a proper code of conduct for the websites added soon, but in this repository
+
+BE EXCELLENT TO EACH OTHER
+
+Do I need to say more? If your behaviour or communication does not fit into this statement, we do not wish for you to help us.
diff --git a/containers/JessAndBeard.psm1 b/containers/JessAndBeard.psm1
@@ -2351,13 +2351,19 @@ The Tags are the same"
           FailedChange  = 0 # + or - the number of tests failed for v5
           SkippedChange = 0 # + or - the number of tests skipped for v5
       },
-            @{
+      @{
           Name          = 'SupportedBuild'
           RunChange     = -3 # + or - the number of tests run for v5
           PassedChange  = -3 # + or - the number of tests passed for v5
           FailedChange  = 0 # + or - the number of tests failed for v5
           SkippedChange = 0 # + or - the number of tests skipped for v5
-
+      },
+      @{
+          Name          = 'GuestUserConnect'
+          RunChange     = 0 # + or - the number of tests run for v5
+          PassedChange  = +2 # + or - the number of tests passed for v5
+          FailedChange  = -2 # + or - the number of tests failed for v5
+          SkippedChange = 0 # + or - the number of tests skipped for v5
       }
   )
   $runchange = 0
diff --git a/containers/base/dockerfile b/containers/base/dockerfile
@@ -3,7 +3,7 @@ FROM dbachecks/sqlinstance1
 USER root
 
 # remove old dbatools directory so 2.0 will import
-RUN rm -rf /usr/local/share/powershell/Modules/dbatools
+RUN rm -rf /usr/local/share/powershell/Modules/dbatools/1.1.145
 
 # Copy Profile
 
diff --git a/developing/PSConfEU demo.md b/developing/PSConfEU demo.md
@@ -0,0 +1,48 @@
+# PSConfEU demo
+
+1. Develop in the source repository
+  - copy existing check & rewrite - add check to `source/checks/Databasev5.Tests.ps1`
+  - add configuration to `source/internal/configurations/configuration.ps1`
+    - `skip.database.pseudosimple`
+    - `policy.database.pseudosimpleexcludedb`
+  - add object info to `source/internal/functions/Get-AllDatabaseInfo.ps1`
+
+
+2. Build the module
+    ```PowerShell
+    ./build.ps1 -Tasks build
+    ```
+
+3. Sampler automatically adds the new version to your path
+    ```PowerShell
+    get-module dbachecks -ListAvailable | select name, modulebase
+    ```
+
+4. Import new version of the module (if you get a bogus error the first time retry it)
+    ```PowerShell
+    Import-Module dbachecks -force
+    ```
+
+5. Test out the new code
+
+    ```PowerShell
+    # save the password to make for easy connections
+    $password = ConvertTo-SecureString "dbatools.IO" -AsPlainText -Force
+    $cred = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList "sqladmin", $password
+
+    $show = 'All'
+    $checks = 'RecoveryModel'
+
+    #$sqlinstances = 'localhost,7401', 'localhost,7402', 'localhost,7403'
+    $sqlinstances = 'dbachecks1', 'dbachecks2', 'dbachecks3' # need client aliases for this to work New-DbaClientAlias
+
+    # Run v4 checks
+    $v4code = Invoke-DbcCheck -SqlInstance $Sqlinstances -SqlCredential $cred -Check $Checks -legacy $true -Show $show -PassThru
+    # Run v5 checks
+    $v5code = Invoke-DbcCheck -SqlInstance $Sqlinstances -SqlCredential $cred -Check $Checks -legacy $false -Show $show -PassThru -Verbose
+
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks -PerfDetail
+    Invoke-PerfAndValidateCheck -SQLInstances $sqlinstances -Checks $Checks -showTestResults
+    ```
+
diff --git a/source/checks/Databasev5.Tests.ps1 b/source/checks/Databasev5.Tests.ps1
@@ -58,17 +58,17 @@ Describe "Suspect Page" -Tag SuspectPage, High , Database -ForEach $InstancesToT
 }
 
 Describe "Database Collation" -Tag DatabaseCollation, High, Database -ForEach $InstancesToTest {
+    #TODO: Should we have a skip option for each IT block?
     $skip = ($__dbcconfig | Where-Object { $_.Name -eq 'skip.database.databasecollation' }).Value
     Context "Testing database collation on <_.Name>" {
         It "Database <_.Name> collation <_.Collation> should match server collation <_.ServerCollation> on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database } else { $psitem.ConfigValues.wrongcollation -notcontains $PsItem.Name } } {
             $psitem.ServerCollation | Should -Be $psitem.Collation -Because "You will get collation conflict errors in tempdb"
         }
 
         # wrong collation set
-        It "Database <_.Name> collation <_.Collation> should not match server collation <_.ServerCollation> on <_.SqlInstance>" -ForEach $psitem.Databases.Where{ $_.Name -in $psitem.ConfigValues.wrongcollation } {
+        It "Database <_.Name> collation <_.Collation> should not match server collation <_.ServerCollation> on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ $_.Name -in $psitem.ConfigValues.wrongcollation } {
             $psitem.ServerCollation | Should -Not -Be $psitem.Collation -Because "You have defined the database to have another collation then the server. You will get collation conflict errors in tempdb"
         }
-
     }
 }
 
@@ -222,3 +222,55 @@ Describe "Compatibility Level" -Tag CompatibilityLevel, High, Database -ForEach
         }
     }
 }
+
+Describe "Guest User" -Tag GuestUserConnect, Security, CIS, Medium, Database -ForEach $InstancesToTest {
+    $Skip = ($__dbcconfig | Where-Object Name -EQ 'skip.security.guestuserconnect').Value
+
+    Context "Testing Guest user has CONNECT permission" {
+        It "Database Guest user should return no CONNECT permissions in <_.Name> on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database } else { $psitem.ConfigValues.guestuserexclude -notcontains $psitem.Name } } {
+            $psitem.GuestUserConnect | Should -BeFalse -Because "we don't want the guest user to have connect access to our database."
+        }
+    }
+}
+
+Describe "Recovery Model" -Tag RecoveryModel, DISA, Medium, Database -ForEach $InstancesToTest {
+    $Skip = ($__dbcconfig | Where-Object Name -EQ 'skip.database.recoverymodel').Value
+
+    Context "Testing Recovery Model" {
+        It "Database <_.Name> should be set to <_.ConfigValues.recoverymodeltype> on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database } else { $psitem.ConfigValues.recoverymodelexclude -notcontains $psitem.Name } } {
+            $psitem.RecoveryModel | Should -Be $psitem.ConfigValues.recoverymodeltype -Because "You expect this recovery model."
+        }
+    }
+}
+
+Describe "PseudoSimple Recovery Model" -Tag PseudoSimple, Medium, Database -ForEach $InstancesToTest {
+    $Skip = ($__dbcconfig | Where-Object Name -EQ 'skip.database.pseudosimple').Value
+
+    Context "Testing database is not in PseudoSimple recovery model" {
+        It "Database <_.Name> has PseudoSimple recovery model equal false on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database -and $_.RecoveryModel -eq 'Full' } else { $psitem.ConfigValues.pseudosimpleexclude -notcontains $psitem.Name -and $_.RecoveryModel -eq 'Full' } } {
+            $psitem.PseudoSimple | Should -BeFalse -Because "PseudoSimple means that a FULL backup has not been taken and the database is still effectively in SIMPLE mode"
+        }
+    }
+}
+
+Describe "Contained Database Auto Close" -Tag ContainedDBAutoClose, CIS, Database -ForEach $InstancesToTest {
+    $Skip = ($__dbcconfig | Where-Object Name -EQ 'skip.security.containedbautoclose').Value
+
+    Context "Testing contained database auto close option" {
+        It "Database <_.Name> should have auto close set to false on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database -and $_.ContainmentType -ne "NONE" } else { $psitem.ConfigValues.contdbautocloseexclude -notcontains $psitem.Name -and $_.ContainmentType -ne "NONE" } } {
+            $psitem.ContainedDbAutoClose | Should -BeFalse -Because "Contained Databases should have auto close set to false for CIS compliance."
+        }
+    }
+}
+
+Describe "Contained Database SQL Authenticated Users" -Tag ContainedDBSQLAuth, CIS, Database -ForEach $InstancesToTest {
+    $Skip = ($__dbcconfig | Where-Object Name -EQ 'skip.security.ContainedDBSQLAuth').Value
+
+    #if ($version -lt 13 ) { $skip = $true }
+
+    Context "Testing contained database to see if sql authenticated users exist" {
+        It "Database <_.Name> should have no sql authenticated users on <_.SqlInstance>" -Skip:$skip -ForEach $psitem.Databases.Where{ if ($Database) { $_.Name -in $Database -and $_.ContainmentType -ne "NONE" } else { $psitem.ConfigValues.contdbsqlauthexclude -notcontains $psitem.Name -and $_.ContainmentType -ne "NONE" } } {
+            $psitem.ContainedDbSqlAuthUsers | Should -Be 0 -Because "We expect there to be no sql authenticated users in contained database."
+        }
+    }
+}
diff --git a/source/internal/configurations/configuration.ps1 b/source/internal/configurations/configuration.ps1
diff --git a/source/internal/functions/Get-AllDatabaseInfo.ps1 b/source/internal/functions/Get-AllDatabaseInfo.ps1
