Skip to content
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.

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

SqlServerDsc: Add ScriptAnalyzer rule to validate that resource loads SMO #1683

Closed
johlju opened this issue Jan 29, 2021 · 2 comments · Fixed by #1684
Closed

SqlServerDsc: Add ScriptAnalyzer rule to validate that resource loads SMO #1683

johlju opened this issue Jan 29, 2021 · 2 comments · Fixed by #1684

Comments

@johlju
Copy link
Member

johlju commented Jan 29, 2021

In the issue #1680 (comment) there was an issue reported that the resource failed due to the SMO assemblies was not loaded. This was due to that Import-SQLPSModule was not called.

Suggest adding a PSScriptAnalyzer rule that validates that each *-TargetResource function do call Import-SQLPSModule. If a function is not suppose to call Import-SQLPSModule then it would be possible to suppress the analyzer rule for that particular function.

@johlju johlju added help wanted The issue is up for grabs for anyone in the community. tests The issue or pull request is about tests only. labels Jan 29, 2021
@johlju
Copy link
Member Author

johlju commented Jan 29, 2021

This was added in the working branch add-script-analyzer-rule-for-Import-SQLPSModule and in the folder AnalyzerRules. But the problem is that the CustomRulePath in the Script Analyzer settings file analyzersettings.psd1 does not allow an array of paths:

https://github.com/johlju/SqlServerDsc/blob/cfe91305ec1b403023f9c77171c24a676a23ca85/.vscode/analyzersettings.psd1#L3-L6

When adding above configuration all the custom rules stops working.

So only way to support this is to add it do DscResource.Analysis, or make a Pester test that runs Invoke-ScriptAnalyzer with just this particular custom rule. The former is not a good choice since this rule is limited to SqlServerDsc and would need to made more generic like adding the option to configure it through the Script Analyzer settings file (not sure how to do that). So the only viable option right now is to create a QA test that runs this custom rule.

@johlju johlju added in progress The issue is being actively worked on by someone. and removed tests The issue or pull request is about tests only. labels Jan 29, 2021
@johlju
Copy link
Member Author

johlju commented Jan 30, 2021

I have it working with multiple paths in the Script Analyzer settings file. The problem was that the rules need to be exported by a module of in a module script file. So this works:

@{
    CustomRulePath      = @(
        '.\output\RequiredModules\DscResource.AnalyzerRules'
        '.\source\AnalyzerRules\SqlServerDsc.AnalyzerRules.psm1'
    )

    IncludeRules        = @(
        # DSC Resource Kit style guideline rules.
        ...

        # Additional rules
        ...

        'Measure-*'
    )
}

johlju added a commit that referenced this issue Jan 31, 2021
…not load SMO (#1684)

- SqlServerDsc
  - Added a new script analyzer rule to verify that `Import-SQLPSModule` or `Connect-SQL`
    (that implicitly calls `Import-SQLPSModule`) is present in each `Get-`, `Test-`,
    and `Set-TargetResource` function. If neither command is not needed then the
    analyzer rule should be overridden ([issue #1683](#1683)).
  - Added a new pipeline job that runs Script Analyzer on all PowerShell scripts
    in the source folder. The rules are defined by the Script Analyzer settings
    file `.vscode\analyzersettings.psd1` (which also the Visual Studio Code
    PowerShell extension uses).
  - Added unit tests and integration tests for SQL Server 2019
    ([issue #1310](#1310)).
  - Suppressed new custom Script Analyzer rule `SqlServerDsc.AnalyzerRules\Measure-CommandsNeededToLoadSMO`
    for `Get-`, `Test-`, and `Set-TargetResource` functions in the resources.
  - Minor lint errors throughout the repository.
- SqlSetup
  - Minor refactor due to source code lint errors. The loop what evaluates
    the configuration parameters `*FailoverCluster` was change to a `foreach()`.
@johlju johlju removed in progress The issue is being actively worked on by someone. help wanted The issue is up for grabs for anyone in the community. labels Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant