When to place variables inside\outside of functions?

Welcome Forums General PowerShell Q&A When to place variables inside\outside of functions?

Viewing 2 reply threads
  • Author
    Posts
    • #282961
      Participant
      Topics: 3
      Replies: 3
      Points: 46
      Rank: Member

      Hey all, I am relatively new to PowerShell and am trying to learn to use functions. What confuses me is variable scope. My brain constantly wrestles with the question of where to place variables. The same issue pops up with Scriptblocks as well to be honest. I’ve read the MS documentation regarding scopes but it’s still not ringing any bells for me. I also never see their examples in other peoples’ scripts (e.x. $script:variable). I am hoping someone can take my very incorrect code below and give me some real-world examples of how to properly do this. Here is my code (it’s for obtaining the Installation ID of Windows 7 and Server 2008 machines in a offline environment for Microsoft’s Extended Support activation):

      function Get-InstallationID {
      [cmdletbinding()]
      param (
      [string]$ComputerName = (Get-ADComputer -Filter { OperatingSystem -Like '*Windows 7*' -or OperatingSystem -Like '*Server 2008*' } -Properties OperatingSystem | Select-Object -ExpandProperty Name),
      [string]$ProductKey = '22VMR-WVR6H-2H4CX-G2Y9M-R9RFW', #This changes each year.
      [string]$ActivationID = '553673ed-6ddf-419c-a153-b760283472fd' #This changes each year.
      )

      #---------------------------------------------------------------------------------Install ESU Product Key if Not Already Installed---------------------------------------------------------------------------------

      #Get current year.
      $Year = (Get-Date -Format 'yyyy')
      #Location of the 'LicenseInfo.txt'.
      $licenseinfo = "C:\LicenseInfo_$Year.txt"

      #If 'LicenseInfo.txt' already exists, write to console'
      if (-not(Test-Path -Path $licenseinfo)) {
      try {
      Invoke-Command -ComputerName $PC -ScriptBlock {
      # NOTE: This Product Key needs to be updated each year & will be provided by the Infrastructure team.
      cscript.exe C:\Windows\System32\slmgr.vbs /ipk $ProductKey
      # NOTE: This Activation ID needs to be updated each year: https://techcommunity.microsoft.com/t5/windows-it-pro-blog/archived-how-to-get-extended-security-updates-for-eligible/ba-p/917807
      cscript.exe C:\Windows\System32\slmgr.vbs /dlv $ActivationID > $licenseinfo }
      }
      catch {
      $PSItem.Exception.Message | Tee-Object -FilePath $Log -Append
      Write-Output "$licenseinfo could not be generated on $env:COMPUTERNAME."
      break
      }

      #---------------------------------------------------------------------------------Generate Installation ID and Output to File---------------------------------------------------------------------------------

      #Additional check to make sure the correct product is being activated.
      $PartialProductKey = (Get-Content $licenseinfo -TotalCount 19 | Select-String -Pattern ("Partial Product Key")).ToString()
      #Remove the first 21 characters, leaving only the actual PartialProductKey.
      $PartialProductKey = $PartialProductKey.substring(21)

      #This checks the 'Partial Product Key:' line of C:\LicenseInfo.txt to ensure the 'ESU-Year1' product is listed first since the InstallationID gets pulled from the first product.
      if ($PartialProductKey -ne $ProductKey.Substring(24)) {
      Write-Output "Partial Product Key does not = $($ProductKey.Substring(24)) on $PC. Check C:\LicenseInfo.txt on that machine and ensure 'ESU-Year1' product is listed first!"
      }
      else {
      #Pull the Installation ID from 'LicenseInfo.txt' and output to $outfile.
      $InstallationID = Get-Content $licenseinfo -TotalCount 19 | Select-String -Pattern ("Installation ID") | ForEach-Object { $_.line }
      #Create a custom object so we can output data to Excel in organized columns.
      $CSVobject = [PSCustomObject]@{
      Hostname = $env:COMPUTERNAME
      InstallationID = $InstallationID
      ConfirmationID = ''
      }
      $CSVobject | Export-Csv -NoTypeInformation -Path $Using:Outfile -Force
      }
      }
      }

      #---------------------------------------------------------------------------------Install ESU Product Key if Not Already Installed---------------------------------------------------------------------------------

      $Outfile = "C:\Users\$env:USERNAME\Desktop\InstallationIDs.csv"
      $Log = "C:\Windows\appslogs\ESU_Activation_$(Get-Date -Format yyyyMMdd_HHmmss).txt"
      $OnlinePCs = [System.Collections.ArrayList]::new()

      foreach ($PC in $ComputerName) {
      if (Test-NetConnection -ComputerName $PC -Port 5985 -InformationLevel Quiet) {
      [void]$online.Add("$PC")
      }
      else {
      Write-Output "$PC could not be contacted on port 5985. Try pinging it." | Tee-Object -FilePath $Log -Append
      }
      }
      try {
      Invoke-Command -ComputerName $OnlinePCs -ScriptBlock ${function:Get-InstallationID} -ErrorAction Stop
      Write-Output "Successfully pulled Installation ID from $PC"
      }
      catch{
      $PSItem.Exception.Message | Tee-Object -FilePath $Log -Append
      Write-Output "Something went wrong - check error message\log." | Tee-Object -FilePath $Log -Append
      }
      finally{
      $Error.Clear()
      }

      My 3 main concerns are the following variables $CSVobject, $Outfile, and $Log. I have a feeling all 3 are wrong. Not sure how the data obtained within the function is passed back to the PC running the script, essentially. Thanks in advance for any and all help!

    • #282970
      Participant
      Topics: 0
      Replies: 9
      Points: 55
      Helping Hand
      Rank: Member

      I am not sure, I got, what you are talking about.

      If you use invoke-command you start a new PSSession on the other Computer. The other Computer didn not know the Variable $Produktkey (and the other variables).

      yout should use:

      But then:

      You starting this lines on the other Computer. You give the Variable $licenseinfo with $USING
      That means, that your command:

      cscript.exe C:\Windows\System32\slmgr.vbs /dlv $ActivationID >$licenseinfo

      Writes the output to $licenseinfo = “C:\LicenseInfo_$Year.txt”
      This is written on the Remote-Computer and later you reading the localComputer file $licenseinfo = “C:\LicenseInfo_$Year.txt”

      • #282988
        Participant
        Topics: 3
        Replies: 3
        Points: 46
        Rank: Member

        Ah I see. I think maybe I should just remove that Invoke-Command altogether correct? Since I am passing the function to remote computers the function should be written as if it’s running locally? So instead of:

        try {
        Invoke-Command -ComputerName $PC -ScriptBlock {
        # NOTE: This Product Key needs to be updated each year & will be provided by the Infrastructure team.
        cscript.exe C:\Windows\System32\slmgr.vbs /ipk $ProductKey
        # NOTE: This Activation ID needs to be updated each year: https://techcommunity.microsoft.com/t5/windows-it-pro-blog/archived-how-to-get-extended-security-updates-for-eligible/ba-p/917807
        cscript.exe C:\Windows\System32\slmgr.vbs /dlv $ActivationID > $licenseinfo }

        }

        I could just use:

        try {
        # NOTE: This Product Key needs to be updated each year & will be provided by the Infrastructure team.
        cscript.exe C:\Windows\System32\slmgr.vbs /ipk $ProductKey
        # NOTE: This Activation ID needs to be updated each year: https://techcommunity.microsoft.com/t5/windows-it-pro-blog/archived-how-to-get-extended-security-updates-for-eligible/ba-p/917807
        cscript.exe C:\Windows\System32\slmgr.vbs /dlv $ActivationID > $licenseinfo
        }

        Does that sound right? This wasn’t even the part of the code I was concerned about but thank you very much for pointing that out!!

         

    • #282997
      Participant
      Topics: 17
      Replies: 1951
      Points: 3,986
      Helping Hand
      Rank: Community Hero

      First, there are methods to get and set Product keys with Powershell using WMI. This would alleviate the parsing required to make the output usable in Powershell as an object:

      https://social.technet.microsoft.com/Forums/ie/en-US/9fcdb039-f9f0-48a9-8253-1f52c0257a80/activate-windows-using-powershell?forum=winservergen

      As far as scope, there are most likely many many articles explaining scope. The most common is function scope. For instance…

      This is wrong. Does it work? Of course! Because Powershell is meant to be simple for administrators and a bazillion people always asked the same question why can I not see $someVariable? Scope. The quick fix would always be add $script or $global to it, which is almost always wrong. General rule of thumb, you should never really use Global or Script variables. If you do, then it is most likely not best practice.

      A lot of arguments are going to be “does it work” vs “is it best practice”. If a function needs variable information, it should be a parameter. Parameters can be used to make it mandatory or not, validate the data, check if it’s null, etc. before it runs the function. If you are using global variables, some unlucky soul is going to to have to dig all over your 1000 line script trying to understand why $someVariable is null and figure out why it’s need in the function, how it’s set and a small kitten is killed every time you use variable incorrectly. Again, general rule thumb is global\script scope should never need to be defined, or you’re probably doing it incorrectly.

      As Chris was saying for Invoke-Command, a script block is like a function, you are saying whatever is between these curly brackets gets run on the REMOTE computer. How is the poor remote computer going to know what $someVariable is if you are only sending what is in the curly brackets?

      It cannot as $someVariable is on your computer and not on the remote system. So, you need to use the using: keyword (Chris’ example) or pass as an argument:

      Some other things of note:

      Look at other Powershell cmdlets and how they work. If you wanted to do many items like computer, it should be framed out like below. One big thing it was defined as string, not an array (i.e. string[]), so it would consider all computers to be one big gigantic string:

      Another one is:

      Would not recommend doing an export in the function. You don’t see Get-Process, Get-Service, Get-ADUser or any GET command having export options to a file. This is a normal Powershell approach. The GET is returning information, that is all:

      • #284080
        Participant
        Topics: 3
        Replies: 3
        Points: 46
        Rank: Member

        Thank you Rob. Your answer was extremely educational and insightful. I’ve made many improvements thanks to your advice!

Viewing 2 reply threads
  • You must be logged in to reply to this topic.