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

SqlPermission: Get-TargetResource expects a mandatory parameter #1761

Closed
wahid99 opened this issue Jun 23, 2022 · 4 comments · Fixed by #1762 or #1778
Closed

SqlPermission: Get-TargetResource expects a mandatory parameter #1761

wahid99 opened this issue Jun 23, 2022 · 4 comments · Fixed by #1762 or #1778
Labels
enhancement The issue is an enhancement request. 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

@wahid99
Copy link
Contributor

wahid99 commented Jun 23, 2022

Problem description

When doing a get on SQLPermission resource it expects the parameter 'permission' to be passed into it. If the permission parameter isn't passed through then we hit an Exception.

Verbose logs

PowerShell DSC resource DSC_SqlPermission  failed to execute Get-TargetResource functionality with error message: System.InvalidOperationException: Unexpected result when trying to get permissions for 'domain\testuser1'. ---> 
System.Management.Automation.ParameterBindingValidationException: Cannot validate argument on parameter
'Permission'. The argument is null or empty. Provide an argument that is not null or empty, and then try the command again. --->
System.Management.Automation.ValidationMetadataException: The argument is null or empty. Provide an argument that is not
 null or empty, and then try the command again.
   at System.Management.Automation.ValidateNotNullOrEmptyAttribute.Validate(Object arguments, EngineIntrinsics engineInt
rinsics)
   at System.Management.Automation.ParameterBinderBase.BindParameter(CommandParameterInternal parameter, CompiledCommand
Parameter parameterMetadata,
ParameterBindingFlags flags)
   --- End of inner exception stack trace ---
   at System.Management.Automation.ExceptionHandlingOps.CheckActionPreference(FunctionContext funcContext, Exception exc
eption)
   at System.Management.Automation.Interpreter.ActionCallInstruction`2.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   --- End of inner exception stack trace ---

DSC configuration

$InvokeParams = @{

Name = 'SqlPermission';
Method = 'get';
Property = @{instancename = 'MSSQLSERVER'; principal = 'domain\testuser1'};
ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/sqlserverdsc/dsc_resources/SqlServerDsc/SqlServerDsc.psd1';
RequiredVersion = '15.2.0'}}
Invoke-DscResource @InvokeParams

Suggested solution

make permission to be a mandatory parameter in the sqlpermission resource. Doing this returns without any problems.

$InvokeParams = @{
Name = 'SqlPermission';
Method = 'get';
Property = @{instancename = 'MSSQLSERVER'; principal = 'domain\testuser1'; permission = 'AlterAnyAvailabilityGroup', 'ViewServerState'};
ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/sqlserverdsc/dsc_resources/SqlServerDsc/SqlServerDsc.psd1';
RequiredVersion = '15.2.0'}}
Invoke-DscResource @InvokeParams
ConfigurationName    :
DependsOn            :
ModuleName           : C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/sqlserverdsc/dsc_resources/SqlServerDsc/SqlServerDsc.psd1
ModuleVersion        : 15.2.0
PsDscRunAsCredential :
ResourceId           :
SourceInfo           :
Ensure               : Absent
InstanceName         : MSSQLSERVER
Permission           : {}
Principal            : domain\testuser1
ServerName           : SQL01
PSComputerName       : localhost

SQL Server edition and version

Microsoft SQL Server 2019 (RTM-CU16-GDR) (KB5014353) - 15.0.4236.7 (X64)   May 29 2022 15:55:47   Copyright (C) 2019 Microsoft Corporation  Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2019 Standard 10.0 <X64> (Build 17763: )

SQL Server PowerShell modules

Name  Version Path
----  ------- ----
SQLPS 15.0    E:\Program Files (x86)\Microsoft SQL Server\150\Tools\PowerShell\Modules\SQLPS\SQLPS.psd1

Operating system

OsName               : Microsoft Windows Server 2019 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture       : 64-bit
WindowsVersion       : 1809
WindowsBuildLabEx    : 17763.1.amd64fre.rs5_release.180914-1434
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

PowerShell version

Name                           Value
----                           -----
PSVersion                      5.1.17763.1490
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17763.1490
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

SqlServerDsc version

15.2.0
@johlju johlju changed the title SQLpermission: Get-TargetResource expects a mandatory parameter SQLPermission: Get-TargetResource expects a mandatory parameter Jun 23, 2022
@johlju johlju added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. good first issue The issue should be easier to fix and can be taken up by a beginner to learn to contribute on GitHub labels Jun 23, 2022
@johlju
Copy link
Member

johlju commented Jun 23, 2022

Agree that it looks like the Permission should be made mandatory.

@johlju
Copy link
Member

johlju commented Jun 23, 2022

Happy to review a PR that fixes this. 🙂

wahid99 added a commit to wahid99/SqlServerDsc that referenced this issue Jun 24, 2022
Added permission parameter as a mandatory param.
dsccommunity#1761
wahid99 added a commit to wahid99/SqlServerDsc that referenced this issue Jun 24, 2022
@wahid99
Copy link
Contributor Author

wahid99 commented Jun 24, 2022

#1762

Kindly review. Thanks

@johlju
Copy link
Member

johlju commented Jul 28, 2022

PR looks okay. But looking at the resource as a whole I think the entire resource should be refactored to remove Ensure parameter. It should instead have the parameters Permission, PermissionToInclude, and PermissionToExclude. The PR #1769 has done such refactor on SqlDatabasePermission. I think we need to do a similar refactor on this resource. I create a new issue to track this. Outside of your PR.

johlju pushed a commit that referenced this issue Jul 28, 2022
- SqlPermission
  - The `Permission` parameter is now mandatory for all `*-TargetResource` (issue #1761).
@johlju johlju removed the help wanted The issue is up for grabs for anyone in the community. label Jul 28, 2022
@johlju johlju changed the title SQLPermission: Get-TargetResource expects a mandatory parameter SqlPermission: Get-TargetResource expects a mandatory parameter Jul 28, 2022
johlju added a commit that referenced this issue Jul 31, 2022
…urce) (#1778)

- SqlServerDsc
  - The following classes were added to the module:
    - `ServerPermission` - complex type for the DSC resource SqlPermission.
  - The following public functions were added to the module (see comment-based
    help for more information):
    - `Test-SqlDscIsLogin`
    - `ConvertFrom-SqlDscServerPermission`
    - `ConvertTo-SqlDscServerPermission`
    - `Get-SqlDscServerPermission`
    - `Set-SqlDscServerPermission`
  - SMO stubs (used in the unit tests)
    - Was updated to remove a bug related to the type `ServerPermissionInfo`
      when used with the type `ServerPermissionSet`. The stubs suggested that
      the property `PermissionType` (of type `ServerPermissionSet`)
      in `ServerPermissionInfo` should have been a array `ServerPermissionSet[]`.
      This conflicted with real SMO as it does not pass an array, but instead
      a single `ServerPermissionSet`. The stubs was modified to mimic the
      real SMO. At the same time some old mock code in the SMO stubs was removed
      as it was no longer in use.
- SqlPermission
  - BREAKING CHANGE: The resource has been refactored. The parameters
    `Permissions` has been replaced by parameters `Permission`,
    `PermissionToInclude`, and `PermissionToExclude`. These permissions
    parameters are now an instance of the type `ServerPermission`.
    The type `ServerPermission` contains two properties; `State` and
    `Permission`. This closes the issue [issue #1761](#1761),
    it also fixes the issues [issue #1773](#1773),
    [issue #1704](#1704),
    and [issue #752](#752).
  - The resource was refactored into a class-based resource.
- SqlDatabasePermission
  - Fixed comment-based help and cleaned up comments.
  - Fix localized string that referenced 'user' instead of 'principal',
    and correct localized string ID for each string.
- `Set-SqlDscDatabasePermission`
  - Minor code cleanup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is an enhancement request. 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
2 participants