rubrikinc / rubrik-sdk-for-powershell

Rubrik Module for PowerShell
https://build.rubrik.com/sdks/powershell/
MIT License
102 stars 87 forks source link

Connect-Rubrik fails when using PowerShell version lower than 6 #817

Closed ChrisCarpenter-Grijzen closed 1 year ago

ChrisCarpenter-Grijzen commented 1 year ago

Current Behavior:

When using Connect-Rubrik to connect to a Rubrik cluster, using a Rubrik Service Account (new functionality in Rubrik PowerShell module version 6.0.1), the connection fails to be established if an "old" PowerShell version is used. Connect-Rubrik.ps1 uses the SkipCertificateCheck parameter of the Invoke-RestMethod cmdlet. The SkipCertificateCheck was introduced in PowerShell version 6, and is not available in older PowerShell versions, causing failure in connection establishment.

Expected Behavior:

The Connect-Rubrik should probe the available PowerShell version (using Test-PowerShellSix?), and the the SkipCertificateCheck parameter of the Invoke-RestMethod cmdlet should only be used if PowerShell version 6 or higher is available. The SkipCertificateCheck functionality may be emulated in another way, should PowerShell version 6 or higher not be available.

Steps to Reproduce:

Connect to a Rubrik cluster, using the Connect-Rubrik cmdlet, using a Rubrik Service Account, using a PowerShell version below 6.

Context:

Failure Logs

Please include any relevant log snippets or files here, IMPORTANT all information will be visible publicly on GitHub. Do not include computer or user names, passwords, API tokens or any identifiable information when submitting failure logs.

ChrisCarpenter-Grijzen commented 1 year ago

A quick search indicates that both Invoke-RubrikGraphQLCall.ps1 and Start-RubrikDownload.ps1 also use the SkipCertificateCheck parameter of the Invoke-RestMethod cmdlet, but in these cases a check on the available PowerShell version is correctly performed (using Test-PowerShellSix).

Bryan-Meier commented 1 year ago

@guirava, we found an issue in the code that was added in Connect-Rubrik for AccountId and Secret.

After debugging the code in Connect-Rubrik, I found that the "SkipCertificateCheck" attribute in the code below is not compatible with PS 5.1.x. Would it be possible to wrap the $RestSplat with an if statement where we use Test-PowerShellSix again and not include the SkipCertificateCheck attribute when the PS version is not 6 and higher? I have tested this and it does in fact fix the issue for those of us still running PS 5.1.x.

Line 150 in Connect-Rubrik - Version 6.0.1 $RestSplat = @{ Method = 'Post' ContentType = "application/json" URI = "https://$Server/api/v1/service_account/session" SkipCertificateCheck = $true Body = @{ serviceAccountId = "$($Id)" secret = "$($Secret)" } | ConvertTo-Json

Please let me know if this is not clear.

StefanBPS commented 1 year ago

Would it be possible to wrap the $RestSplat with an if statement where we use Test-PowerShellSix again and not include the SkipCertificateCheck attribute when the PS version is not 6 and higher?

I would really like this to be implemented since it would be a hassle to upgrade all powershells to higher version than 5.1 that is the standard on all Windows systems.

WeigelJ commented 1 year ago

It would be great if support for the "default" powershell version was added. Upgrading to version 7 turns out to be not as easy as I thought, especially if another system runs some scripts to monitor the results and uses the "old" Powershell version by default.

tonypags commented 1 year ago

I have the same issue. Please add handling for PS5 wherever irm is used.

To work around this, I have commented out the line in the splat so that the command will run in PS5. (this is Connect-Rubrik)

  $RestSplat = @{
      Method = 'Post'
      ContentType = "application/json"
      URI = "https://$Server/api/v1/service_account/session"
      #SkipCertificateCheck = $true #<------------ WORKAROUND HERE! 
      Body = @{
          serviceAccountId = "$($Id)"
          secret = "$($Secret)"
      } | ConvertTo-Json
  }
  $response = Invoke-RestMethod @RestSplat -Verbose

Of course this might break something else, but for my needs it works fine.

If I had to PR it, I would do this:

  $RestSplat = @{
      Method = 'Post'
      ContentType = "application/json"
      URI = "https://$Server/api/v1/service_account/session"
      Body = @{
          serviceAccountId = "$($Id)"
          secret = "$($Secret)"
      } | ConvertTo-Json
  }
  if ($PSVersiontable.PSVersion.Major -gt 5) {$RestSplat.SkipCertificateCheck = $true}
  $response = Invoke-RestMethod @RestSplat -Verbose

... But you have your own custom function for handling PS6+ which has this flaw in its implementation.

tonypags commented 1 year ago

PR submitted, but please enjoy your weekend first! (Please squash merge)