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

AD Module bug re-introduced in v2.6.0 #948

Closed
bodybybuddha opened this issue Feb 21, 2022 · 11 comments
Closed

AD Module bug re-introduced in v2.6.0 #948

bodybybuddha opened this issue Feb 21, 2022 · 11 comments

Comments

@bodybybuddha
Copy link

I've been playing around with this PowerShell Module since 2.5.2 - awesome work! However, I think I may have uncovered an issue with the latest version 2.6.0 and the activedirectory module - with multithreading. Seems to be the same issue reported in #702. Maybe... as the module seems to work most of the time.

Using the exact same code for both v2.5.2 and v2.6.0 of Pode, the older version returns back the expected results every time. Whereas the latest version will give me less results when it hits the second thread.

Here's the code:

import-module ActiveDirectory
Start-PodeServer -threads 4 {
   # Server Configuration
    $port = (Get-PodeConfig).Port
    Add-PodeEndpoint -Address localhost -Port $port -Protocol Http
    Add-PodeRoute -Method Get -Path '/api/v1/hddata/account/:id' -ScriptBlock {
        # get the user
        try {    
            $acctInfo = Get-adUser $WebEvent.parameters.id -properties extensionattribute2 -Verbose    
            $rtnObj = [Ordered]@{
                SubmittedAccountAlias = $WebEvent.parameters.id
                Data                  = $acctInfo 
            }    
            Write-PodeJsonResponse -Value $rtnObj    
        }
        catch {    
            $objRestErr = @{
                SubmittedAccountAlias = $WebEvent.parameters.id
            }
            Write-ErrorJsonResponse -StatusCode 404 -Message "Account not found" -JSONObject ($objRestErr | convertto-json -compress)    
        }
    }    
} #-verbose

If I run the v2.6.0 of Pode, the first, third, and fourth attempt will show me that I got a value for extensionattribute2. However, the second attempt will only show the normal AD fields. The extensionattribute2 shows up in the PropertyNames, but you can't access the data.

This occurs no matter if I use 2 threads or 5 threads. "Thread" number 2 will always give just the normal default fields. It appears that there is some sequential round robin mechanism in place for which thread to hit next. B/c I can consistently hit the wrong information on the 2nd, 6th, 10th time if I use 4 threads (2 to 3 second pause per hit as I'm manually looking for results in return.)

If I use 2.5.2 - it works like a champ every time I hit the endpoint regardless of the number of threads. I'm not sure if the older version was just hitting the same working thread all the time, or what. I was trying to find a way of having the thread id captured in the returned information to see if I was hitting different threads or not. (no success there yet..)

I've tried importing the activedirectory module before loading pode, before starting the script, using the Import-PodeModule, etc. I've also tried putting a "| select-object *" to force a noteproperty on the items return. This didn't have any affect. Also, as mentioned in #702 - I tried accessing the information via .psobject.properties and the field is missing there as well.

Any thoughts on what 2.6.0 could have introduced? Or did I miss something in my testing?

@Badgerati
Copy link
Owner

Hi @bodybybuddha,

v2.6.0 did change the way in which Pode's runspaces are started (async, rather than sync).

In a route, you can check the current thread via $ThreadId | Out-Default, which is a simple number ID for the thread/runspace. In theory, if you're making calls to the route just one-after-another, it should always be ThreadId=1, or possibly even 1,2,1,2,etc. depending on the speed you're making the calls.

I'll try and test it myself, but what happens if you move the Import-Module ActiveDirectory call to directly within the route? Possibly also do Get-Module | Out-Default alongside the ThreadId; if the AD module isn't being imported properly that could trigger the issue. The way modules are imported with the new async logic did change - to using Import-Module directly - but the logic to detect modules to import didn't.

One last thought, do you happen to have a server.psd1 file as well? If that has Server.AutoImport.Modules.ExportOnly set to $false, set it to $true (likewise if Enabled is $false).

@bodybybuddha
Copy link
Author

Thanks for the quick reply @Badgerati!

I added both the ThreadId and Import-module lines that you recommended and did a bunch of tests, results below. I can confirm that the system is doing a round robin on the threads: 1,2,3,4,1,2.... This is with at 2-3 second wait between calls. I even waited 30 seconds in one set of tests.

For each test, I started with a clean PowerShell environment (v5.1 btw) and there was no modules other than the appropriate version of the Pode module.

Here are the test results:

Version Import-module location AutoImport.Modules.Exportonly Thread success Thread failed Results
2.6.0 Before start-podeserver Doesn't exists 1,3,4 2 AD Module in each thread
2.6.0 Route Doesn't exists 1 2,3,4 AD Module in each thread
2.6.0 Before start-podeserver $True 1 2,3,4 1st Run - Only Pode (v0.0.0) existed in all four threads. Second run through - Pode and AD Module were loaded. Data results the same as the first run through.
2.6.0 Route $True 1 2,3,4 AD Module in each thread
2.6.0 Before start-podeserver $False 1,3,4 2 AD Module in each thread
2.6.0 Route $False 1 2,3,4 AD Module in each thread
2.5.2 Before start-podeserver Doesn't exists 1,2,3,4 AD Module in each thread
2.5.2 Route Doesn't exists 1 2,3,4 AD Module in each thread
2.5.2 Before start-podeserver $True 1 2,3,4 1st Run - Only Pode (v0.0.0) existed in all four threads. Second run through - Pode and AD Module were loaded. Data results the same as the first run through.
2.5.2 Route $True 1 2,3,4 AD Module in each thread
2.5.2 Before start-podeserver $False 1,2,3,4 AD Module in each thread
2.5.2 Route $False 1,2,3,4 AD Module in each thread

A couple of the tests were pretty interesting in that the AD module didn't appear till after I went through a full cycle (hit all the threads) and went for another run. I also added v2.5.2 for comparsion.

Here is my server.psd1 - with the entry for AutoImport.Modules.ExportOnly added:

@{
    Port    = 4321
    Server  = @{  
        FileMonitor = @{
            Enable  = $true
            Exclude = @("logs/*")
        }
        AutoImport  = @{
            Modules = @{
                ExportOnly = $false
            }
        }        
    }
    Service = @{ }
    Web     = @{ }
    Smtp    = @{ }
    Tcp     = @{ }
}

Please let me know what else I can do to help!

@Badgerati
Copy link
Owner

Hi @bodybybuddha,

Thanks for the info! Do you have the script you used when you ran Import-Module in the route? I tried myself and each time it imported the first time 🤔

That is very bizarre for it to only fail on thread 2, and not 1, 3, 4 😕 The server I usually test AD on is down at the moment (credits on Azure 😂), it should be back up tomorrow so I can test further.

If you're in a position to edit the module locally, would you be able to try out the below?

change this line:

$PodeContext.Server.Modules[$module] = $path

to:

$PodeContext.RunspaceState.ImportPSModule($path)

and, comment out this line:

Import-PodeModules

That's the change to how modules were imported in v2.6.0.

Thanks! :)

@bodybybuddha
Copy link
Author

Hey @Badgerati -

Made the changes to the Context & Helpers files in the v2.6.0 version.

Here is the test script I'm using:

Start-PodeServer -threads 4 {
   # Server Configuration
    $port = (Get-PodeConfig).Port
    Add-PodeEndpoint -Address localhost -Port $port -Protocol Http

    Add-PodeRoute -Method Get -Path '/api/v1/hddata/account/:id' -ScriptBlock {
        import-module ActiveDirectory
        # get the user
        try {
            $ThreadId | Out-Default
            get-module | Out-Default

            $acctInfo = Get-adUser $WebEvent.parameters.id -properties extensionattribute2 | Select-Object *    
            $rtnObj = [Ordered]@{
                SubmittedAccountAlias = $WebEvent.parameters.id
                ThreadId               = $ThreadId
                Data                  = $acctInfo
            }    
            Write-PodeJsonResponse -Value $rtnObj    
        }
        catch {    
            $objRestErr = @{
                SubmittedAccountAlias = $WebEvent.parameters.id
            }
            Write-ErrorJsonResponse -StatusCode 404 -Message "Account not found" -JSONObject ($objRestErr | convertto-json -compress)    
        }
    }    
} -verbose

The above is of course, calling the import-module in the Add-PodeRoute. I simply move the import-module line to be the first line in the script. The script is similar to my original post, only with some additions of outputting the thread and modules and I return the thread id in the json response.

All 6 tests of the modified v2.6.0 came out with the exact same results as that of the v2.5.2 tests. So changes successfully rolled back. 😅

Here is an image of the output of the instances the module isn't loaded in the first run:
image
and here is an image of the output when the AD module is loaded in each thread:
image
Notice that the first image is missing both the Microsoft.WSMan.Managment and PSReadline modules. All other executions have all four modules loaded. A detail I missed in my previous testing.

Let me know what else I can do!
Thanks!

@Badgerati
Copy link
Owner

Badgerati commented Feb 24, 2022

Hey @bodybybuddha, and thanks!

Good news is I've managed to reproduce the issue, so I can investigate it now as well :). Looking at the code for .ImportPSModule in PowerShell, it's not really any different to how the new import logic works in v2.6.0.

This is a weird one for sure! I'll dig through the ImportPSModule code a bit more, hopefully there's a small setting they're using I can set as well 🤞

Oh, I also tested with Import-Module in the route, and it loaded first time; so not too sure what's happening there 🤔

Thank you for all the testing and information as well, really helpful! 😊 If you happen upon anything else, lemme know, I've a feeling this could be one of "those" bugs, hah!

@Badgerati
Copy link
Owner

Hey @bodybybuddha,

I've got a possible hotfix for this one, which is to default back to .ImportPSModule for Manifest modules. I have a potential real fix in mind, which uses some custom runspace pooling logic I'm building for another module at the moment; though the change here would be quite large!

It seems to be OK for me, but if you're able to change the following function:

function Import-PodeModulesIntoRunspaceState

To be:

function Import-PodeModulesIntoRunspaceState
{
    # do nothing if disabled
    if (!$PodeContext.Server.AutoImport.Modules.Enabled) {
        return
    }

    # if export only, and there are none, do nothing
    if ($PodeContext.Server.AutoImport.Modules.ExportOnly -and ($PodeContext.Server.AutoImport.Modules.ExportList.Length -eq 0)) {
        return
    }

    # load modules into runspaces, if allowed
    $modules = Get-Module |
        Where-Object {
            ($_.Name -ine 'pode') -and ($_.Name -inotlike 'microsoft.powershell.*')
        } | Sort-Object -Unique

    foreach ($module in $modules) {
        # only exported modules? is the module exported?
        if ($PodeContext.Server.AutoImport.Modules.ExportOnly -and ($PodeContext.Server.AutoImport.Modules.ExportList -inotcontains $module.Name)) {
            continue
        }

        # import the module
        $path = Find-PodeModuleFile -Name $module.Name

        if ($module.ModuleType -ieq 'Manifest') {
            $PodeContext.RunspaceState.ImportPSModule($path)
        }
        else {
            $PodeContext.Server.Modules[$module] = $path
        }
    }
}

and let me know if that's all right? 😄 If it's good, I'll release that fix and leave this issue open while I scope the potential real fix.

Thanks!

@bodybybuddha
Copy link
Author

Sorry - wasn't able to get back to this just yet. I'll try this latest hot fix a bit later today (probably in the evening EST) and get back to you.

@bodybybuddha
Copy link
Author

I started with a clean 2.6.0 install and made the change to the function.

Interesting results.... More in line with the working 2.5.2 for sure. Except the last test case:

Version Import-module location AutoImport.Modules.Exportonly Thread success Thread failed Results
Hotfix 2.6.0 Before Start-PodeServer Doesn't Exists 1,2,3,4 AD Module in each thread
Hotfix 2.6.0 Route Doesn't Exists 1 2,3,4 AD Module in each thread
Hotfix 2.6.0 Before Start-PodeServer $True 1 2,3,4 1st Run - Only Pode (v0.0.0) existed in all four threads. Second run through - Pode and AD Module were loaded. Data results the same as the first run through.
Hotfix 2.6.0 Route $True 1 2,3,4 AD Module in each thread
Hotfix 2.6.0 Before Start-PodeServer $False 1,2,3,4 AD Module in each thread
Hotfix 2.6.0 Route $False 1 2,3,4 AD Module in each thread

First, I'm still getting the weird missing ActiveDirectory module in test scenario 3. So consistency is good, right? 😃

Compared to the v2.5.2 and Modified v2.6.0 testing, the only change is the last test scenario. Only the first thread worked and the others failed. Not sure if failing is the expected outcome.

However, it is inline with the original v2.6.0 tests, in that test scenarios 1 and 5 are similar. Just in the original v2.6.0, thread 2 failed. So as far as I'm concern this is fixed! 🥇

Thoughts?

@Badgerati
Copy link
Owner

Hey @bodybybuddha,

Thanks, glad to here it works! That's actually more in line with what I expect 😄 The ActiveDirectory module is really weirdly built, bit like the MicrosoftTeams one! They don't work properly when imported directly into a route, unlike almost every other module 🙈

The best way to import is before Start-PodeServer, so those scenarios all look good now.

I'll try and get this out as a v2.6.2 as soon as I can; I'm out tonight, so hopefully tomorrow.

Thanks, again!

@Badgerati Badgerati added this to the 2.6.2 milestone Mar 1, 2022
@bodybybuddha
Copy link
Author

Thanks for looking into this and for the module itself! 💯 🥇

@bodybybuddha
Copy link
Author

Hotfix is working great! Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants