Directory Test

This topic contains 2 replies, has 2 voices, and was last updated by  Mort 3 months ago.

  • Author
    Posts
  • #93642

    Mort
    Participant

    Hello,
    I am completely new to using Powershell and have started an online class to learn to use it. I am working on a project that is unrelated to the learning site that I am struggling with. The project goal is to add modify rights to the 'Admins' and 'Users' groups of 3 folders; one at the root of 'c:\mydirectory', one in 'Program Files\mydirectory', and one in 'Program Files (x86)\mydirectory' if they exist. This is what I have so far:

    #region section globals
    # Declare and set globals
    $RootPath =0
    $32Path =0
    $64Path =0
    # Call the VarTest function to reset the above variables
    VarTest
    #endregion
    
    #region section varcheck
    # Clear Variables
    function VarTest ()
    {
    if ($RootPath -ne 0) {$RootPath -eq 0; write-host "The RootPath Variable has been Reset" + Write-Host $RootPath}
    else {Write-Host "The RootPath Variable is Empty" + $RootPath};
    if ($32Path -ne 0) {$32Path =0; write-host "The 32Path Variable has been Reset" + Write-Host $32Path}
    else {Write-Host "The 32Path Variable is Empty" + $32Path};
    if ($64Path -ne 0) {$64Path -eq 0; write-host "The 64Path Variable has been Reset" + Write-Host $64Path}
    else {Write-Host "The 64Path Variable is Empty" + $64Path};
    # Call the PathTest function
    PathTest
    }
    #endregion
    
    #region section pathtest
    # Test the directory paths to make sure they exist and set the variable to add the modify rights to the AD Groups
    function PathTest () {
    # Test Root mydirectory Folder
    if  (Test-Path -Path "C:\mydirectory" -IsValid) {$RootPath =1; write-host "Root mydirectory is here" + $RootPath}
    Else {write-host "Root mydirectory is not here"; Exit;}
    # Test Program Files\mydirectory Folder
    if (Test-Path -Path "c:\program files\mydirectory" -IsValid) {$32Path =1; Write-Host "program files\mydirectory is here" + $32Path}
    Else {write-host "32Bit mydirectory folder is not here"}
    # Test Program Files x86\mydirectory Folder
    if (Test-Path -Path "C:\program files (x86)\mydirectory" -IsValid) {$64Path =1; Write-Host "program files x86\mydirectory is here" + $64Path}
    Else {Write-Host "64Bit mydirectory folder is not here"}
    PermissionChange
    }
    #endregion
    
    #region section root permissions
    # Set the permissions on the root directory
    function PermissionChange () {
    $Acl = Get-Acl "C:\mydirectory"
    
    $Ar = New-Object System.Security.AccessControl.FileSystemAccessRule("Users", "Modify", "ContainerInherit,ObjectInherit", "None", "Allow")
    $Ar2 = New-Object System.Security.AccessControl.FileSystemAccessRule("Admins", "Modify", "ContainerInherit,ObjectInherit", "None", "Allow")
    
    $Acl.SetAccessRule($Ar)
    Set-Acl "c:\mydirectory" $Acl
    
    $Acl.SetAccessRule($Ar2)
    Set-Acl "c:\mydirectory" $Acl
    Write-Host "Root Directory Updated"
    }
    #endregion
    
    #region section 32permissions
    # Set the permissions on the 32bit directory
    function 32Permissions () {
    if ($32Path =1) {
    $Acl = Get-Acl "C:\mydirectory"
    Set-Acl "C:\Program Files\mydirectory" $Acl
    Write-Host "32bit mydirectory folder has been update"
    }
    }
    #endregion
    
    #region section 64permissions
    # Set the permissions on the 64bit directory
    function 64Permissions () {
    if ($64Path =1) {
    $Acl = Get-Acl "C:\mydirectory"
    Set-Acl "C:\Program Files (x86)\mydirectory" $Acl
    Write-Host "64bit mydirectory folder has been update"
    }
    }
    #endregion
    

    What I have seems way more complex than I think it should be (I'd rather use a ForEach statement if I could but I haven't gotten that far yet), and when I test it the variables seem to get stuck with 1 after it is run. I am also having an issue on the test system that if the 'Program Files\mydirectory' exists when the script is ran and then the directory is deleted, on subsequent runs of the script it will still say the directory is there and set the variable to 1 (I have not checked to see if that is the case with the other 2 directories).

  • #93666

    postanote
    Participant

    Everyone will / can have an opinion, and what follows is just mine. There are always better more definitive ways to do just about anything, but that is directly relevant to and of knowledge, training, experience, belief, preference and acceptance.

    All that being said. You need to work under the concepts of:

    Is this code easily readable / understandable?
    Is this code well formatted?
    Is this code easily maintained?
    Is this code follow a development standard?
    Is this code going to be share / used by others who need to understand and maintain it?
    Is this code as optimized as it cade be?
    Is this code as short as it can be without being recondite / abstruse?
    Was unit testing leveraged?
    Was code validation leveraged?
    Was proper error handling implemented?
    Was code logging implemented?
    Was built-in help added?
    Was version control leveraged?
    etc...

    As for the code itself...

    • You have some formatting issues, which make the code hard to read / follow.
    • You are calling functions before they are ever loaded.
    • You are calling variables with an assignment that already exists.
    • You are using Write-Host unnecessarily and incorrectly.
    • Functions should follow the normal verb-noun construct of normal cmdlets.
    • If you are using a whole bunch of If/Else statements, it's time to look at select case.
    • Overuse if double quotes, you only really need them when you need variable expansion in a quoted string. Plain text strings use single quotes.
    • You are using semicolons for no real purpose. They, mean, in the case you are using them, that the next command is independent of the proceeding one. If it's a separate command, then put it on the next line. Cramming things on one line, because you can is really not a thing in scripts. At the command line, I get that, but not scripts / code.
    • Are you sure you wan to use the EXIT where you have it?
    • You have function in the code which are never used.
    • When building code, never do it all at once. That is why modular programming / unit testing exists.
    • You do this one line / segment at a time, test that and make sure you are getting what you'd expect before moving on.

    As far as this...

    'when I test it the variables seem to get stuck with 1 after it is run.'

    It is that way, because that is what you told the code to do. You are not using the comparison and assignment operators correctly for what you appear to be trying to accomplish. I made that adjustment below.

    This would be my take on what you show here.
    Note, this can be optimized more, to reduce this code, and you do have ohter errors in the code becasue of the behaviour you are seeing.
    That is another homework assignment for you to resolve.

    If you make needed position changes with the functions and some code clean-up, you'd end up with something like this, which is far more readable, though I did not correct all the errors, just some of them.
    Well, and used style that I commonly use for the code layout. Everyone has their take on that though.

        #region Begin section globals
    
            # Declare and set globals
            $RootPath = 0 
            $32Path   = 0 
            $64Path   = 0
    
        #endregion  End section globals
    
    
    
        #region Begin section root permissions
    
            # Set the permissions on the root directory
            function Set-PermissionChange() 
            {
                [cmdletbinding()]
    
                Param
                (
                    $Acl = (Get-Acl -Path 'C:\mydirectory')
                )
    
                $AccessRuleUser = New-Object System.Security.AccessControl.FileSystemAccessRule('Users', 'Modify', 'ContainerInherit,ObjectInherit', 'None', 'Allow')
                $AccessRuleAdmin = New-Object System.Security.AccessControl.FileSystemAccessRule('Admins', 'Modify', 'ContainerInherit,ObjectInherit', 'None', 'Allow')
    
                $Acl.SetAccessRule($AccessRuleUser)
                Set-Acl -Path "C:\mydirectory $Acl" -WhatIf
    
                $Acl.SetAccessRule($AccessRuleAdmin)
                Set-Acl -Path "C:\mydirectory $Acl" -WhatIf
                'Root Directory Updated'
            }
    
        #endregion End section root permissions
    
        #region Begin section 32permissions
    
            # Set the permissions on the 32bit directory
            function Set-32Permissions() 
            {
                [cmdletbinding()]
    
                Param
                (
            
                )
    
                if ($32Path = 1) 
                {
                    $Acl = Get-Acl -Path 'C:\mydirectory'
                    Set-Acl -Path "C:\Program Files\mydirectory $Acl" -WhatIf
                    '32bit mydirectory folder has been update'
                }
            }
    
        #endregion End section 32permissions
    
    
        #region Begin section 64permissions
    
            # Set the permissions on the 64bit directory
            function Set-64Permissions() 
            {
                [cmdletbinding()]
    
                Param
                (
            
                )
    
                if ($64Path = 1) 
                {
                    $Acl = Get-Acl -Path 'C:\mydirectory'
                    Set-Acl -Path "C:\Program Files (x86)\mydirectory $Acl" -WhatIf
                    '64bit mydirectory folder has been update'
                }
            }
    
        #endregion End section 64permissions
    
    
    
        #region Begin section pathtest
    
            # Test the directory paths to make sure they exist and set the variable to add 
            # the modify rights to the AD Groups
    
            function New-PathTest() 
            {
                [cmdletbinding()]
    
                Param
                (
            
                )
    
                # Test Root mydirectory Folder
                if  (Test-Path -Path 'C:\mydirectory') 
                {
                    $RootPath = 1
                    'Root mydirectory is here $RootPath'
                }
                Else 
                {
                    Write-Warning -Message 'Root mydirectory is not here'
                    Exit
                }
    
                # Test Program Files\mydirectory Folder
                if (Test-Path -Path 'C:\Program Files\mydirectory') 
                {
                    $32Path = 1
                    'program files\mydirectory is here $32Path'
                }
                Else 
                {
                    Write-Warning -Message '32Bit mydirectory folder is not here'
                }
    
                # Test Program Files x86\mydirectory Folder
                if (Test-Path -Path 'C:\Program Files (x86)\mydirectory') 
                {
                    $64Path = 1
                    'program files x86\mydirectory is here $64Path'
                }
                Else 
                {
                    Write-Warning -Message '64Bit mydirectory folder is not here'
                }
                Set-PermissionChange
            }
    
        #endregion End section pathtest
    
    
    
    
    
        #region Begin section varcheck
    
            # Clear Variables
    
            function New-VarTest()
            {
                [cmdletbinding()]
    
                Param
                (
            
                )
    
                if ($RootPath -gt 0) 
                {
                    $RootPath = 0
                    Write-Warning -Message 'The RootPath Variable has been Reset $RootPath'
                }
                else 
                {
        
                }
    
    
                if ($32Path -gt 0) 
                {
                    $32Path = 0
                    Write-Warning -Message 'The 32Path Variable has been Reset $32Path'
                }
                else 
                {
                    "The 32Path Variable is Empty $32Path"
                }
    
    
                if ($64Path -gt 0) 
                {
                    $64Path = 0
                    'The 64Path Variable has been Reset $64Path'
                }
                else 
                {
                    "The 64Path Variable is Empty $64Path"
                }
    
                # Call the PathTest function
                New-PathTest
            }
    
        #endregion End section varcheck
    
    
        New-VarTest
    
    • #94237

      Mort
      Participant

      postanote Thank you for the reply and suggestions. As you can see from my script, my knowledge on powershell is minimal at best and I don't know that I am getting my $$ worth from the online class... then again maybe some of this is covered in later modules and I am just being overzealous in what I am trying to do at my present level. Thanks again for the response, suggestions, and the clean up on the code.

You must be logged in to reply to this topic.