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

SqlRSSetup: EditionUpgrade parameter bug. #1398

Closed
sguilbault-sherweb opened this issue Jul 15, 2019 · 3 comments · Fixed by #1665
Closed

SqlRSSetup: EditionUpgrade parameter bug. #1398

sguilbault-sherweb opened this issue Jul 15, 2019 · 3 comments · Fixed by #1665
Labels
bug The issue is a bug. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub

Comments

@sguilbault-sherweb
Copy link

Details of the scenario you tried and the problem that is occurring

The parameter is a bool. Normally it should support $True to use it and $False to skip it. The way the ressource is built, whatever the value passed, since it's defined, it will act as if the value was set to $True. In documentation, it say the default value is $False but looking at the code it's $NULL. When the parameter isn't specified, it work as expected. I assumed the only tests that were done are with the value at $True and at $NULL.

Verbose logs showing the problem

N/A

Suggested solution to the issue

Add a test on the value to make sure is $True instead of just defined. (In the Set function at least)

The DSC configuration that is used to reproduce the issue (as detailed as possible)

N/A

SQL Server edition and version the target node is running

2017 Standard

SQL Server PowerShell modules present on the target node

N/A

The operating system the target node is running

Windows Server 2016

Version and build of PowerShell the target node is running

5.1.14393.2969
10.0.14393.2969

Version of the DSC module that was used ('dev' if using current dev branch)

13.0.0.0

@johlju
Copy link
Member

johlju commented Jul 16, 2019

Yes, it looks like this line is wrong

https://github.com/PowerShell/SqlServerDsc/blob/2d37a930d957d2bbe148d118a5065a2f6f33d29e/DSCResources/MSFT_SqlRSSetup/MSFT_SqlRSSetup.psm1#L405-L410

An evaluation should be added here to make sure EditionUpgrade -eq $true and only then add EditionUpgrade to the argument list.

@johlju johlju added bug The issue is a bug. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub help wanted The issue is up for grabs for anyone in the community. labels Jul 16, 2019
@sguilbault-sherweb
Copy link
Author

looking at issue list this one might also be linked #1311

@johlju
Copy link
Member

johlju commented Jul 17, 2019

Issue #1311 is more to find a way to detect what edition is currently installed so Test-TargetResource can handle the desired state. Since there is currently no check, EditionUpgrade = $true will not work if there is for example a Eval version installed, it will think it is in desired state even if the desired state is to have 'Dev' installed.

This issue, what I understood is that Set-TargetResource is adding the /EditionUpgrade argument even if it is $false.

johlju added a commit that referenced this issue Jan 6, 2021
- SqlRSSetup
  - If parameter `EditionUpgrade` is set to `$false` the `/EditionUpgrade`
    argument is no longer wrongly added (issue #1398).
@johlju johlju removed the help wanted The issue is up for grabs for anyone in the community. label Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants