Add ADcomputer to securitygroup using foreach and .txt files

This topic contains 8 replies, has 5 voices, and was last updated by Profile photo of TechGismo TechGismo 1 year, 3 months ago.

  • Author
    Posts
  • #32970
    Profile photo of TechGismo
    TechGismo
    Participant

    I have this text file called Missing_Adgroups.txt, which contains 1 AD group.
    The other text file "PC2ouTest.txt contains several computernames like this
    PC1$
    PC2$
    and so on.
    This is my humble script block:

    $Missing_Programs = Get-Content C:\Scripts\Missing_AdGroups.txt
    $Computers = Get-Content C:\Scripts\Pc2ouTest.txt
    foreach ($pc in $Computers) {
    
    Add-ADGroupMember $Missing_Programs -Members $Computers
    }
    

    My problem is that my code works as it is, as long only 1 AD-group is specified in the "Missing_ADgroup.txt" If I add another AD group to "Missing_Adgroups" it does not work and I get this error:

    Add-ADGroupMember : Cannot convert 'System.Object[]' to the type 'Microsoft.ActiveDirectory.Management.ADGroup' required by parameter 'Identity'. Specified method is not supported.
    At C:\Scripts\Update-PCs-Missing-SW.ps1:5 char:19
    + Add-ADGroupMember $Missing_Programs -Members $Computers
    +                   ~~~~~~~~~~~~~~~~~
        + CategoryInfo          : InvalidArgument: (:) [Add-ADGroupMember], ParameterBindingException
        + FullyQualifiedErrorId : CannotConvertArgument,Microsoft.ActiveDirectory.Management.Commands.AddADGroupMember
  • #32975
    Profile photo of Bob McCoy
    Bob McCoy
    Participant

    Since you're doing a foreach loop, inside the loop you want to add $pc to the group, not $computers.

  • #32979
    Profile photo of Curtis Smith
    Curtis Smith
    Participant

    Additionally, the Add-ADGroupMember cmdlet does not accept a collection for the -Identity parameter.

  • #32983
    Profile photo of Jason Yoder
    Jason Yoder
    Participant

    TechGismo,

    I made a few corrections.

    Inside you ForEach block, you were referencing the collection when you need to reference the tempoary variable of $PC.

    In the help file for Add-ADGroupMember, you need to utilize a property of the computer object:

    Distinguished Name
    Example: CN=saradavisreports,OU=europe,CN=users,DC=corp,DC=contoso,DC=com
    GUID (objectGUID)
    Example: 599c3d2e-f72d-4d20-8a88-030d99495f20
    Security Identifier (objectSid)
    Example: S-1-5-21-3165297888-301567370-576410423-1103
    Security Accounts Manager (SAM) Account Name (sAMAccountName)
    Example: saradavisreports

    $Missing_Programs = Get-Content C:\Scripts\Missing_AdGroups.txt
    $Computers = Get-Content C:\Scripts\Pc2ouTest.txt
    foreach ($pc in $Computers) {
    
    Add-ADGroupMember -Identity $Missing_Programs -Members (Get-ADComputer -Identity $PC).SamAccountName
    }
    
  • #33027
    Profile photo of TechGismo
    TechGismo
    Participant

    @ Bob and Curtis, Thanks for the input I will look into this.
    @ Jason

    I tried to run the script with your corrections, but it failed. I also tried to change $pc to $Computers, but same result.

    Add-ADGroupMember : Cannot convert 'System.Object[]' to the type 'Microsoft.ActiveDirectory.Management.ADGroup' required by parameter 'Identity'. Specified method is not supported.
    At line:5 char:29
    + Add-ADGroupMember -Identity $Missing_Programs -Members (Get-ADCompute ...
    +                             ~~~~~~~~~~~~~~~~~
        + CategoryInfo          : InvalidArgument: (:) [Add-ADGroupMember], ParameterBindingException
        + FullyQualifiedErrorId : CannotConvertArgument,Microsoft.ActiveDirectory.Management.Commands.AddADGroupMember
  • #33030
    Profile photo of Erik Sundin
    Erik Sundin
    Participant

    It seems like you´ve gotten things mixed up, since (as Curtis Smith wrote) -identity does not accept Collections you need a foreach loop for $Missing_Programs.

    Something like (script not tested by the way):

    
    $Missing_Programs = Get-Content C:\Scripts\Missing_AdGroups.txt
    $Computers = Get-Content C:\Scripts\Pc2ouTest.txt
    foreach ($M in $Missing_Programs) {
    
    $Computers | Get-ADComputer | Add-ADGroupMember -Identity $M
    }
    
  • #33035
    Profile photo of TechGismo
    TechGismo
    Participant

    @Erik
    Your not tested script gave this result:

    $Missing_Programs = Get-Content C:\Scripts\Missing_AdGroups.txt
    $Computers = Get-Content C:\Scripts\Pc2ouTest.txt
    foreach ($M in $Missing_Programs) {
    
    $Computers | Get-ADComputer | Add-ADGroupMember -Identity $M
    }
    cmdlet Add-ADGroupMember at command pipeline position 2
    Supply values for the following parameters:
    Members[0]: 
    

    But you kind of inspired me to go on and try something myself. I came up with this:
    Every thing but the last line is working.

    $Missing_Programs = Get-Content C:\Scripts\Missing_AdGroups.txt
    $Computers = Get-Content C:\Scripts\Pc2ouTest.txt
    foreach ($M in $Missing_Programs) {
    
    $SAM = Get-ADComputer $Computers -Properties samAccountName | FT samAccountName
    $SAM
    Add-ADGroupMember -Identity $M -Members $SAM
    }
    
  • #33045
    Profile photo of Erik Sundin
    Erik Sundin
    Participant

    I´m sure there is a much better way, but I guess you could always add another foreach loop inside the first one.

    Something like:
    foreach ($M in $Missing_Programs) {
    foreach ($pc in $computers) {
    }
    }

  • #33103
    Profile photo of TechGismo
    TechGismo
    Participant

    @Erik

    I made it work now and I didn't even had to do double foreach loops. I may have messed up my textfiles because after I moved those to another subfolder. So I kew which files had the correct data, it acutally worked. I would like to thank everybody for helping me this is how it looks now:

    $Apps = Get-Content C:\Scripts\NewFolder\Apps.txt
    $Computers = Get-Content C:\Scripts\NewFolder\Computers.txt
    
    foreach ($App in $Apps) {
    
      Add-ADGroupMember -Identity $App -Members $Computers
    
    }
    

You must be logged in to reply to this topic.