For Loop and export-csv with a variable

I have a script I’ve written and it does almost everything I want, except one little thing.

I pass it a client name, and a domain name and then it queries AD for the groups that match, and then iterates through them to get the groupmember feeds the samaccountname to get the first, last name of the user and then writes to a csv file based on the group name. Works!

I want the same information + group name written into a master.csv file…and I can’t get it to work to save my life.

Param([string]$Client,[string]$Dom)
# Check variables were set, and provide feedback
# if client isn't set the script exits
# Domain can be empty, if so it defaults to the current domain
 
if (!$Client) {
write-host "Pass the pattern of the group you wish to look for"
write-host "Domain is optional, client is not"
write-host "Usage is  CLIENT DOMAIN"
Exit
}
if ($DOM) {
write-host "Using the Domain " $DOM
set-variable -name DOM2 -value "-server $DOM"
}
if (!$Dom) {
write-host "No Domain specfied using" (Get-WmiObject Win32_ComputerSystem).Domain
set-variable -name DOM -value (Get-WmiObject Win32_ComputerSystem).Domain
set-variable -name DOM2 -value "-server $DOM"}

# Create directory structure if needed
if(!(Test-Path -Path $Client )){New-Item -ItemType directory -Path $Client}
if(!(Test-Path -Path $Client\Users )){New-Item -ItemType directory -Path $Client\Users}

#Get the group list based on client name and domain
$look= "Get-ADGroup -fil {SamAccountName -like '*" + $Client + "*'} -server "+ $Dom + "|select samaccountname -expandproperty samaccountname"
#Read the groups in a variable
$groups=@(invoke-expression $look)
#Create lists by group
foreach ($group in $groups) {get-adgroupmember -identity $group -server $dom | where {$_.objectclass -eq 'user'} |Get-ADUSer -server $DOM -properties sAMAccountName, GivenName, Surname |Select-object sAMAccountName, GivenName, Surname |export-csv -append -path $client"\"$group".csv" -NoTypeInformation}
#Create master list
foreach ($group in $groups) {
	get-adgroupmember -identity $group -server $dom | where {
		$_.objectclass -eq 'user'} |Get-ADUSer -server $DOM -properties samaccountName, GivenName, Surname, $Group |Select-object samaccountname, GivenName, Surname|export-csv -append -path $client"\Master.csv" -NoTypeInformation
							}

#zip and email
cd $client
..\7z a -sdel .\$Client.zip .\*.csv
$SMTPServer = "appmailrelay"
$EmailFrom = "NoReply@"
$EmailTo = @("deleted")
$EmailSubject = "UserGroupListing for " + " " + $Client
$EmailAttach = $Client+".zip"
Send-MailMessage -From $EmailFrom -To $EmailTo -Subject $EmailSubject -Attachments $EmailAttach -SmtpServer $SMTPServer

So… could you maybe drill down to just the bit of code that you think isn’t working? I’m not quite following what you mean by “master CSV file.”

Fair question…and I’m not sure if the code isn’t working, or I’m not thinking about it right.
I collect a list of group names with this:

$look= "Get-ADGroup -fil {SamAccountName -like '*" + $Client + "*'} -server "+ $Dom + "|select samaccountname -expandproperty samaccountname"
#Read the groups in a variable
$groups=@(invoke-expression $look)

Then I go through the list of groups, get group member, and then get aduser and export to csv based on the group name and that works. (ie 5 groups, 5 spreadsheets)

foreach ($group in $groups) {get-adgroupmember -identity $group -server $dom | where {$_.objectclass -eq 'user'} |Get-ADUSer -server $DOM -properties sAMAccountName, GivenName, Surname |Select-object sAMAccountName, GivenName, Surname |export-csv -append -path $client"\"$group".csv" -NoTypeInformation}

Now I want to export all of that data into one spreadsheet with an additional column for the group name it came from, but so far nothing I’ve tried has worked. This was my last idea to try, but it didn’t work either. IN fact it caused that entire command to blow up.

foreach ($group in $groups) {
	get-adgroupmember -identity $group -server $dom | where {
		$_.objectclass -eq 'user'} |Get-ADUSer -server $DOM -properties samaccountName, GivenName, Surname, $Group |Select-object samaccountname, GivenName, Surname|export-csv -append -path $client"\Master.csv" -NoTypeInformation

I have a work around in place where I take all of the $group".csv" files and then read them into a Master.csv file, but that feels like a kludge.
}

What do you expect this final CSV to look like? Maybe a couple example rows?

Have you thought of using a PS Custom Object ?

I have not, since I don’t know what those are (kinda neophyte here). I’ll do some research and see if that helps. Thanks.

so, the first script outputs a file:
$group".csv"

If I queried groups My users, my users2 it would create:
My users.csv
my users2.csv

with the following data:
My users.csv
samaccountname givenname surname
sam1 Bob Roberts
sam2 John smith

my users2.csv
samaccountname givenname surname
sam112 Keyser soze
sam115 Dean keaton

then I’d like:
master.csv
samaccountname givennname surname adgroup
sam1 Bob Roberts My users
sam2 John Smith My users
sam112 Keyser soze my users2
sam115 Dean keaton my users2

“File1.csv”,”file2.csv” |
ForEach {
$n = $_
Import-Csv $n |
ForEach {
$_ |
Add-Member -Membertype NoteProperty -name SourceFile -Value $n -pass
}
} |
Export-csv Master.csv

Something like that. Typing on my phone. Combines your cab files and adds a column indicating which file each row came from.

$look= "Get-ADGroup -fil {SamAccountName -like '*" + $Client + "*'} -server "+ $Dom + "|select samaccountname -expandproperty samaccountname"

Doing this and then calling Invoke-Expression on it is likely going to be a habit that’ll bite you sooner or later. Main reason being because you’re constructing the entire thing as a simple string, no IDE will ever highlight that syntax correctly. You’re far more likely to miss your own errors.

Instead, try:

$ADGroups = Get-ADGroup -Filter "SamAccountName -like '*$Client*'" -Server $Dom | Select-Object -ExpandProperty SamAccountName

I have a hunch the reason you did it is due to the weird behaviour of -Filter when you’re using braces – don’t; it’s a [string] typed parameter (despite all the documentation and help files making your life unnecessarily hard by pretending it’s a script block); just use double quotes. :slight_smile:

Thank you kind stranger!

I was struggling with the " and the ', and the ’ and the ". When I was testing the output I realized it built it correctly as a string, so I went with it. I wasn’t proud of it, but it worked…

I used what you gave me and that’s much cleaner. Thanks.

I’m sorta doing that now.

I’m taking the resultant .CSV files reformatting them to remove spaces, merging them + filename into master.csv. I’m just thinking I’m missing SOMETHING in my logic, I can’t quite put my finger on it though.

Yeah, the syntax for that whole command is way overcomplicated in my opinion. :slight_smile:

Okay, so I went through and had a thorough look at your script (late at night because it was fun, sue me, I’m weird with code alright). I changed quite a bit, removed a lot of redundant or unused variables, and pulled the double foreach loops into a single one to consolidate it a bit better.

I reformatted a lot of your long pipelines to use nice neat line breaks (you can break lines after a pipe character to continue the pipeline on the next line, if you weren’t aware!) and keep things more readable.

I renamed a couple variables for readability and streamlined your queries – there didn’t appear to be a real need to query AD twice here, since you were pulling the same information.

Also splatted the email command since you were halfway there anyway… and added some begin{} process{} end{} blocks for syntactic sugar. May also make it easier to put direct pipeline input logic into if you ever decide to go that route as well!

Finally, I think I actually stumbled across your actual issue. You were trying to add a new property containing the group name to the objects you’re getting back from AD for the master CSV file, as I read it.

In order to do that, there are a few ways, but by far one of the quickest and most clear syntactically is with Select-Object:

$ADUsers | Select-Object *, @{
    Name = "Group"
    Expression = {$Group}
}

Basically, you construct a simple hashtable containing the name of the property you’re adding and its value. The extra braces are, to my memory, necessary. The * is there to say “I want all the properties it already has” and then we add an extra one. This is known as a ‘calculated property’ if you wanna do some more reading.

And if you’re curious to how I managed to find that in that script, well… it doesn’t look the same to me anymore :wink:

[CmdletBinding()]
param(
    [Parameter(Position = 0, Mandatory, HelpMessage = "Pass the pattern of the group you wish to look for")]
    [ValidateNotNullOrEmpty()]
    [string]
    $Client, 
    
    [Parameter()]
    [ValidateNotNullOrEmpty()]
    [string]
    $Domain = (Get-WmiObject Win32_ComputerSystem).Domain
)
begin {
    $WorkingDirectory = Join-Path -Path $PSScriptRoot -ChildPath $Client
    $MasterCsvPath = Join-Path -Path $WorkingDirectory -ChildPath 'Master.csv'
    $ClientZipPath = Join-Path -Path $WorkingDirectory -ChildPath "$Client.zip"

    # Create directory structure if needed
    if (-not (Test-Path -Path $WorkingDirectory )) {
        New-Item -ItemType Directory -Path $WorkingDirectory
    }
}
process {
    # Get the group list based on client name and domain
    $GroupList = Get-ADGroup -Filter "SamAccountName -like '*$Client*'" -Server $Domain |
        Select-Object -ExpandProperty SamAccountName

    # Create list by group
    foreach ($Group in $GroupList) {
        $GroupCsvPath = Join-Path -Path $WorkingDirectory -ChildPath "$Group.csv"

        # Compile group data that is needed for both CSV exports
        $GroupData = Get-ADGroupMember -Identity $Group -Server $Domain | 
            Where-Object ObjectClass -eq 'User' |
            Get-ADUSer -Server $Domain -Properties SamAccountName, GivenName, Surname |
            Select-Object -Property SamAccountName, GivenName, Surname

        # Export the per-group CSV
        $GroupData |
            Export-Csv -Path $GroupCsvPath -NoTypeInformation

        # Export the master CSV after tacking on a property that references the current group
        $GroupData | Select-Object *, @{
            Name = "Group"
            Expression = {$Group}
        } | Export-Csv -Path $MasterCsvPath -NoTypeInformation
    }
}
end {
    Compress-Archive -Path "$WorkingDirectory\*.csv" -DestinationPath $ClientZipPath

    $MailParams = @{
        To          = @("deleted")
        From        = "NoReply@"
        Subject     = "User-Group Listing for $Client"
        Attachments = $ClientZipPath
        SmtpServer  = "appmailrelay"
    }
    Send-MailMessage @MailParams
}

Couple final notes:

  • You can actually just "drop a $variable in a string" like that. As long as it's a double quoted string, PS will take care of the ins and outs of how to get the value in there for you. Single quoted strings are strictly literal, though.
  • Although I've severely overused it here, I would recommend utilising Join-Path to join two path portions together. While you can mostly keep track of it in your own code, as soon as you start taking user input that might have, for example, backslashes in it, you need to think about using Join-Path to save you the hassle of properly splicing the paths together.
  • Speaking of paths... I note you're using a lot of implied relative paths. In general, avoid this. It's very unclear what you're trying to do. If you must use a relative path, I'd advise being clear about it using the '.\Thing\File.txt' form -- that first dot specifies the current working directory, and it'll save you from strange edge cases. That said, most times when you're talking current directory you actually mean the directory the script is in -- these can be different, you can call a script using its full path from anywhere. As a result, you should use $PSScriptRoot in order to properly pull the script's directory and work from there.
  • 7zip works really well, but I would caution against using it unless you really need to. You should generally seek to make your scripts as versatile as possible, and that means minimising external dependencies. If you need 7zip (for example, the native Compress-Archive cmdlets don't support files over 2GB in size) for an edge case or specific purpose, by all means, use it. But where it's not really necessary, try to stick to native PS methods.
  • Finally, rather than trying to work directly with your parameters, I would advise simply letting PowerShell's built in frameworks handle it for you. You can apply a variety of attributes to your parameters in order to define exactly what input you'll accept, and PS will enforce it for you. If you fail to enter a mandatory parameter, you'll be prompted for it. If you enter invalid input as you define it for your parameters, it will spit out an error and make you try again.

    Wow…First, this is awesome.

    and…when you were re-writing my script, so was I! Mine still doesn’t look nearly as nice as your’s, but I added some logging to it so I can check up on it after it runs. I’m going to take the new pieces of what I wrote and fold it into what you gave me.

    Is there a place you would recommend for an ambitious newbie to start reading about best practices with powershell?