Question Optimisation d'un script

Plus d'informations
il y a 2 semaines 5 jours #30310 par Damien Commenge
Bonjour,

J'ai plutôt pas mal avancé dans l'élaboration d'un script qui me permet de faire un Audit Active directory avec pas mal de choses que je récupère.
Toutefois, je suis certain que d'un point de vue "code" on est loin d'une version optimisée.

Mon script fait environs 2000 lignes, je pense que si je mets le script complet, j'ai peu de chance d'avoir une réponse donc je vais plutot mettre la structure que j'ai utilisé dans mon script et quelque exemple pour avoir des avis.

Je suis quasiment sur que quelqu'un qui sait coder serait en mesure de diviser le nombre de ligne par 2.
Je pense que je n'utilise pas forcément la bonne façon de faire bien qu'elle soit fonctionnelle. J'ai des fonctions qui récupèrent les informations que je souhaite. Je stocke le résultat de chaque fonction dans une variable. Je met à jour cette variable pour qu'elle contienne la conversion en HTML de ce résultat. J'avias utilisé cette méthode dans l'optique d'un jour avoir la possibilité de faire un export vers du PDF également et pas uniquement du HTML mais cela n'est plus à l'ordre du jour.
Actuellement, ce rapport me permet de construire les tableau que je copie / colle dans un document word "préformaté" à ce genre d'audit.

Voici comment la script est structuré :

1) Commentaire pour présenter le script et à quoi il sert.
2) Quelque variables globales pour indiquer principalement où seront stockés les fichier + variable CSS pour le fichier CSS généré par la suite.
3) Listing de toutes les fonctions (1 fonction = 1 action)
4) Code permettant l'exécution de ces fonctions.

Il y a une fonction qui permet la construction d'un fichier html qui est la sortie de ce script.

Voici à peu près ce que cela donne :
# =======================================================
# NAME: auditAD.ps1
# AUTHOR: xxxxxx
# DATE: 22/12/2020
##
# 
# This script is used to created HTML report for Active directory Audit
# =======================================================
#   18/04/2020 
# -Add last update times for all DC
#   22/04/2020
# -Optimisation code get-recyclebinstate
# -Suppression de nombreuses variables à l'intérieur des fonction
# -Suppression des return des fonctions
#....

#Requires -Version 3.0
#Requires -Modules ActiveDirectory,GroupPolicy

################Global Variables #####################


#region HTML style
$HTMLCSS=@"
<style>
table {
   # border-collapse: collapse;
   border-width: 1px; border-style: solid; border-color: black; border-collapse: collapse;
}
h2 {text-align:center}
th, td {
    #padding: 8px;
    #text-align: left;
    #border-bottom: 1px solid #ddd;
    border-width: 1px; padding: 3px; border-style: solid; border-color: black;
}
body {
    font-family:"Vinci Sans Light";Font-Size:10pt 
}

tr:hover{background-color:#f5f5f5}
</style>
"@

#Variable initialization  
#$domainName = All forest domains
#$ReportPath = Path where script datas will be stored
#$date = Current date to format text and be sure they are unique
#$dnsfilename is dns file name for dns configuration
#$usersCSV is file name for exported users in each domain
#$computerCSV is file name for exported computers in each domains
#$logfilename is file name for file contains script log for troubleshoot.
$domainName=(get-adforest).domains
$ReportPath = "c:\temp\audit"
$date = (get-date -Format "dd_MM_yyyy_HH_mm")
$htmlReportFileName = "ADReport_$date.html"
$DNSFileName = "Dns_$date.txt"
$usersCSV = "users_$date.csv"
$computersCSV = "computers_$date.csv"

$logFileName = "AuditADps1_$date.log"


################ Functions #####################

#region globalcatalogs
#Check if DCs are Global Catalogs 
Function Get-GlobalCatalogServers
{
    param (
        [string]$domain 
    )
    $DCsInDomain = Get-ADDomainController -Filter * -Server $domain
    Write-EZLog -Category INF -Message "### Start Get-GlobalCatalogServers ###"
    Write-EZLog -Category INF -Message "get DC globalcataog"
    foreach ($DC in $DCsInDomain)
    {
        Write-EZLog -Category INF -Message "Test $($DC.hostname)"
        [PSCustomObject]@{
            ServerName = $DC.hostname
            IsGlobalCatalog = $DC.IsGlobalCatalog
        }
    }
    Write-EZLog -Category INF -Message "object created. ServerName = $($obj.ServerName) ; IsGlobalCatalog = $($obj.IsGlobalCatalog)"
    Write-EZLog -Category INF -Message "End Test $($DC.hostname)"
    Write-EZLog -Category INF -Message "### End Get-GlobalCatalogServers ###"
} 

#function to check last time AD partitions were backuped
#Source : https://adamtheautomator.com/last-backup-domain-controller/

#Find 1 DC in domain and ask it for the last backup date.
#This script give last backup for AD and not for the server because attribute is updated each time backup happens.
function Get-LastBackupDate
{
    param (
        [string]$domain 
    )
    #Find 1 domain controller in current domain
    $DomainDC = @(Get-ADDomainController -Filter {domain -eq $domain} -Server $domain)
    [PSCustomObject]@{
        DomainName = $domain
        LastBackupDate = (Get-ADReplicationAttributeMetadata -Object (get-addomain $domain).distinguishedname -Properties dsasignature -Server $DomainDC[0].hostname).LastOriginatingChangeTime
    }
}


#region hardware DC configuration
#Hardware Configuration of all DCs (Logical CPU number,RAM, physical or virtual, OS, last boot)
function Get-AllDCHardware
{
    param (
        [string]$domain 
    )
    Write-EZLog -Category INF -Message "### Start Get-AllDCHardware ###"

    (Get-ADDomainController -Filter {domain -eq $domain} -Server $domain) | Foreach-Object {
        Write-EZLog -Category INF -Message "Process Domain controller $($_.hostname)"
           [PSCustomObject]@{
            ServerName = $_.name
            LogicalCPUNb = (Get-CimInstance win32_computersystem -ComputerName $_.hostname).numberOfLogicalProcessors
            'RAM(GB)' = (Get-CimInstance win32_computersystem -ComputerName $_.hostname).TotalPhysicalMemory/1GB -as [INT]
            Type = (get-CimInstance win32_computersystem -ComputerName $_.hostname).model
            OS = (Get-CimInstance Win32_OperatingSystem -ComputerName $_.hostname).caption
            LastBoot = (Get-CimInstance -ClassName win32_operatingsystem -ComputerName $_.hostname).lastbootuptime
        }
        Write-EZLog -Category INF -Message "object created. ServerName = $($obj.ServerName) ; LogicalCPUNb = $($obj.LogicalCPUNb) ; RAM(GB) = $($obj.'RAM(GB)')"
        Write-EZLog -Category INF -Message "Type = $($obj.Type) ; OS = $($obj.OS) ; LastBoot = $($obj.LastBoot)"
        Write-EZLog -Category INF -Message "End Domain controller $($_.hostname)"
    }
    Write-EZLog -Category INF -Message "### End Get-AllDCHardware ###"
}
#endregion



################ HTML Build function #####################
<#
.Synopsis
   Generate HTML report
.DESCRIPTION
   Use all variable to build html report with CSS style written on the top of this page
.EXAMPLE
   Get-HTMLReport -Path "c:\temp\report.html"
#>
function Get-HTMLReport
{
    [CmdletBinding()]

    Param
    (
        #HTML file path
        [Parameter(Mandatory=$true)]
        [string] $Path,

        #HTML file name
        [Parameter(Mandatory=$true)]
        [string] $FileName
    )
    begin   
    {
        if(!(Test-Path $Path))
        {
            New-Item -Path $Path -ItemType directory
        }
        $HTMLfilename="$path\$filename.html"
    }
    process
    {
    #HTML generation

    $HTMLTitle = "<h1>Active Directory Report for domain $domain</h1>"



    $LastBackupDate = $LastBackupDate | ConvertTo-Html -Fragment
    $GlobalCatalogServers = $GlobalCatalogServers | ConvertTo-Html -Fragment
    $DCHardware = $DCHardware | ConvertTo-Html -Fragment

    #region HTML body

    $HTML = @"
    <!DOCTYPE html>
    <html>
    <head>
    <title>Report</title>
    <meta name="generator" content="PowerShell" />
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">

    $HTMLCSS
    </head>
    <body>
    $HTMLTitle
    
    <h3> Global Catalogs </h3>
    $GlobalCatalogServers

    <h3> Last Backup Date</h3>
    $LastBackupDate

    <h3>Hardware configuration</h3>
    $DCHardware <br />
    
    $End Date $(get-date -Format "dd/MM/yyyy HH:mm")
    </body>
    </html>
"@

    $HTML | out-file -FilePath $HTMLfilename
    #endregion

    Invoke-Item $HTMLfilename

    }
}

################ Main #####################
#region main function
###### Exécution des commandes

#Create working directory c:\temp\audit folder
if (-not(Test-Path $ReportPath))
{
    New-Item -Path $ReportPath -Type Directory | Out-Null
}

Write-EZLog -Header -LogFile "$ReportPath\$logFileName"


foreach ($domain in (Get-ADForest).domains)
{
    Write-EZLog -Category INF -Message "-------------- Start domain $domain --------------" -ToScreen

    Write-EZLog -Category INF -Message "Test ReportFolder exist" -ToScreen

    if (-not(Test-Path $ReportPath\$domain))
    {
        New-Item -Path "$ReportPath\$domain" -Type Directory | Out-Null
    }
	Write-EZLog -Category INF -Message "GlobalCatalogServers" -ToScreen
    $GlobalCatalogServers = Get-GlobalCatalogServers -domain $domain
    Write-EZLog -Category INF -Message "LastBackupDate" -ToScreen
    $LastBackupDate = Get-LastBackupDate -domain $domain
    Write-EZLog -Category INF -Message "DCHardware" -ToScreen
    $DCHardware = Get-AllDCHardware -domain $domain
	
	
    #End
    Write-EZLog -Category INF -Message "----End to Export Files----" -ToScreen
    Write-EZLog -Category INF -Message "Build HTML report" -ToScreen
    Get-HTMLReport -Path "$ReportPath\$domain" -FileName "$($domain)_$htmlReportFileName"
    Write-EZLog -Category INF -Message "-------------- End domain $domain --------------" -ToScreen

}
#endregion

C'est un script que j'ai commencé à créer en aout 2019 et que j'essaye d'enrichir en fonction des informations que je trouve mais je pense qu'il est également nécessaire de mieux l'écrire.
Je posterai probablement certaines fonctions que je souhaiterais également optimiser en terme de temps si je ne trouve pas ma réponse tout seul.

Je remercie par avance les personnes qui vont m'aider la dessus :)

Connexion ou Créer un compte pour participer à la conversation.

Plus d'informations
il y a 2 semaines 5 jours #30311 par Arnaud Petitjean
Réponse de Arnaud Petitjean sur le sujet Optimisation d'un script
Hello !

Super exercice et super script au passage ! Bravo ! :woohoo:

Ca pourrait être bien d'avoir le script complet afin de pouvoir jouer avec. Tu devrais peut-être, soit le mettre en pièce jointe de ton post, ou mieux le publier dans GitHub.

En première lecture rapide, je note les points suivants:
  • Ton script devrait accepter des paramètres plutôt que des valeurs en dur,
  • Tu devrais pour ta Here String $HTMLCSS=@"... mettre des quotes simples car je vois qu'elle contient des quotes doubles. Cela peut poser des problèmes. Donc je te conseille plutôt ceci $HTMLCSS=@'...
  • Tu devrais ajouter un Header et un Footer à ton fichier de log. Le module EZLog que je connais très bien offre nativement cette fonctionnalité ;-)
  • J'ajouterais une directive #Requires supplémentaire en début de script afin d'indiquer clairement les dépendances de modules dont ton script a besoin ainsi que leur numéro de version minimale (ou exacte). Le mieux est de spécifier une hashtable.

Sinon pour le reste, ça me semble plutôt correct ;-). Mais je n'ai peut-être pas poussé assez l'analyse...

Tiens, ça me fait penser que tu devrais passer à la moulinette PowerShell Script Analyzer. Ca te donnerais encore davantage de points d'amélioration c'est certain !

Arnaud

MVP PowerShell et créateur de ce magnifique forum :-)
Auteur de 6 livres PowerShell aux éditions ENI
Fondateur de la société Start-Scripting
Besoin d'une formation PowerShell ou d'un conseil ?

Connexion ou Créer un compte pour participer à la conversation.

Plus d'informations
il y a 2 semaines 5 jours - il y a 2 semaines 5 jours #30312 par Damien Commenge
Réponse de Damien Commenge sur le sujet Optimisation d'un script
Bonjour et merci beaucoup pour ton retour.
  • C'est noté pour les paramètres. Je peux en effet permettre l'utilisation des paramètres si besoin tout en positionnant une valeur par défaut qui serait celle indiquée ici.
  • Concernant le here string pour le CSS, je modifie ca tout de suite :)
  • Pour le header et le footer, ils ne sont pas intégrés dans l'extrait que j'ai mis ici mais sont bien présent dans mon script. J'ai copié collé la fonction que tu as écrite qui permet de faire le logging dans ce script (ce qui n'est peut être pas une bonne chose en soit...
  • Que veux tu dire par ajouter un require pour les dépendances de module supplémentaire ?
    Il n'y a qu'active directory, group policy et ensuite ton module de gestion des logs mais vu que j'ai intégré directement la fonction, je n'utilise pas le "module".


Pour le côté organisationnel, le code est disponible ici en version complète:
auditAD

Ce sera plus simple pour analyser un peu mieux mais je n'étais pas sur que quelqu'un souhaite s'y pencher dessus. Si tu le fais, je t'en remercie grandement car
ce script la représente un grand investissement de mon côté et me sert lors de l'élaboration d'audits AD chez les clients.
J'ai fait une formation chez microsoft pour avoir les bases en powershell, c'est comme tout, faut pratiquer, et je suis pas développeur mais ingé système, donc je manque énormément de pratiques et tous les conseils seront très bon à prendre.

Je devrai bientot terminer d'avoir intégrer tout ce que je souhaitais dedans.
Il me reste à mieux organiser tout ce que j'extrais au niveau des fichiers parce qu'il y en a encore beaucoup et peut être certains inutilement.
Il me reste également à mieux documenter ce que chaque fonction réalise ainsi qu'a vérifier que le fichier de log généré fonctionne correctement car je n'ai toujours pas vérifié.
Enfin, il me reste à m'assurer que les valeurs que je récupèrent correspondent bien au valeurs que je dois récupérer :) Je me suis rendu compte que parfois j'ai fais l'inverse de ce que je voulais. Ex : Vérifier le nombre de compte dont la préauthentification kerberos n'est pas nécessaire j'ai filtrer sur la valeur false alors que c'était true :)
Dernière édition: il y a 2 semaines 5 jours par Damien Commenge.

Connexion ou Créer un compte pour participer à la conversation.

Plus d'informations
il y a 2 semaines 4 jours #30317 par Laurent Dardenne
Réponse de Laurent Dardenne sur le sujet Optimisation d'un script
Salut,
une remarque en passant, pour ceci :
   Write-EZLog -Category INF -Message "object created. ServerName = $($obj.ServerName) ; IsGlobalCatalog = $($obj.IsGlobalCatalog)"
    Write-EZLog -Category INF -Message "End Test $($DC.hostname)"
    Write-EZLog -Category INF -Message "### End Get-GlobalCatalogServers ###"
tu peux utiliser une seul appel avec une here-string :
   Write-EZLog -Category INF -Message @"
object created. ServerName = $($obj.ServerName) ; IsGlobalCatalog = $($obj.IsGlobalCatalog)"
End Test $($DC.hostname)
### End Get-GlobalCatalogServers ###
"@

Ensuite on trouve des références à $obj dans ton code mais je vois pas leurs création. Je me trompe ?

Tutoriels PowerShell

Connexion ou Créer un compte pour participer à la conversation.

Plus d'informations
il y a 2 semaines 4 jours - il y a 2 semaines 4 jours #30320 par Damien Commenge
Réponse de Damien Commenge sur le sujet Optimisation d'un script
Bonjour et merci pour ce retour.
Je vais tenter de consolider en effet tous les logs pour utiliser les here-string qui sont plus propre.
Je ne sais pas jusqu'à quel point je suis sensé logué en réalité, et j'ai l'impression d'en faire trop la... je logue quasiment chaque commande, ca me parait énorme pour l'utilisation que je vais avoir .... C'est pas une application métier que je vais créer ^^.
Donc avec un peu de recul, j'ai surtout tendance à me dire que je perds beaucoup de temps la dessus surtout que pour etre sincère, je n'ai du lire ce fichier de logs qu'une poignée de fois sur la centaine de fois ou j'ai du lancé mon scripts dans mon lab et chez les clients.

Ensuite, pour $obj, c'était un array / arraylist d'objet que retournaient mes fonctions mais au final j'avais décidé de ne pas utilisé cette variable car elle ajoutait du code non utile si je ne souhaite pas loger cette partie. Si je souhaite la logger, je ne sais pas comment le faire sans $obj.
Ex :
function Get-LastBackupDate
{
    param (
        [string]$domain 
    )
$obj=@()
    #Find 1 domain controller in current domain
    $DomainDC = @(Get-ADDomainController -Filter {domain -eq $domain} -Server $domain)
   $obj+= [PSCustomObject]@{
        DomainName = $domain
        LastBackupDate = (Get-ADReplicationAttributeMetadata -Object (get-addomain $domain).distinguishedname -Properties dsasignature -Server $DomainDC[0].hostname).LastOriginatingChangeTime
    }
  Write-EZLog -Category INF -Message "object created. DomainName = $($obj.DomainName ) ; LastBackupDate = $($obj.LastBackupDate )"
}

Est il possible d'avoir ceci :
Write-EZLog -Category INF -Message "object created. DomainName = $($obj.DomainName ) ; LastBackupDate = $($obj.LastBackupDate )"
Sans stocker l'objet précédement crée dans une variable ?
Je ne sais pas comment récupérer cet objet et ses attributs pour loger leur création si je ne les ai pas stocké dans une variable.


PS : Si jamais tu as un peu de temps à consacrer à la lecture du script complet pour faire quelque remarque, j'en serai ravi :)
Je fais tout dans mon coin aujourd'hui et j'essaye surtout d'obtenir les informations que je veux récupérer, mais je suis persuadé que je suis assez souvent 'maladroit' dans ma façon de faire.
Il y a des fonctions que j'avais trouvé sur des blogs perso, d'autre de microsoft, et une grande quantité que j'ai crée moi même.
Dernière édition: il y a 2 semaines 4 jours par Damien Commenge.

Connexion ou Créer un compte pour participer à la conversation.

Plus d'informations
il y a 2 semaines 4 jours #30324 par Laurent Dardenne
Réponse de Laurent Dardenne sur le sujet Optimisation d'un script
>>les here-string qui sont plus propre.
Pas spécialement, ça dépend du mécanisme de log, s'il est 'buffériser' ce n'est pas nécessaire.Il faut éviter de 'stresser' les accès disque.
Ensuite reste à déterminer si on loggue une instruction ou une étape d'un traitement.

>>je logue quasiment chaque commande, ca me parait énorme pour l'utilisation que je vais avoir
>>C'est pas une application métier que je vais créer

Certes, mais ton métier, qui tend à s'automatiser, nécessite de savoir ce qui passe.

>>j'ai surtout tendance à me dire que je perds beaucoup de temps la dessus
Les logs c'est comme les sauvegardes ou la ceinture de sécurité, ça sert à rien; sauf quand on a besoin :-)
Mieux vaut trop que pas assez, j'ai ce soucis sur une 'application' PS codée en WPF/SqlServer, je loggue tout.
Cela génére du bruit, au sens documentaire, mais c'est nécessaire.

>>je ne sais pas comment le faire sans $obj.
On peut pas référencer ce qui n'existe pas.

>>Est il possible d'avoir ceci :
Oui, sous réserve de créer un objet.

>>Sans stocker l'objet précédemment crée dans une variable ?
Si tu m'expliques comment on fait, c'est sûrement possible ;-)

>>Je ne sais pas comment récupérer cet objet et ses attributs pour loger leur création si je ne les ai pas stocké dans une variable.
On peut pas référencer ce qui n'existe pas.

>>mais je suis persuadé que je suis assez souvent 'maladroit' dans ma façon de faire.
L'objectif est que ton code réponde au besoin exprimé et qu'il soit maintenable.
Si tu arrives à faire ça la maladresse n'entre pas en considération.
Le développement ce n'est pas d'être adroit, mais compréhensible.
Certaines personnes ne sont ni l'un ni l'autre, là tu pleures quand tu dois reprende ce type de code...

Tutoriels PowerShell

Connexion ou Créer un compte pour participer à la conversation.

Temps de génération de la page : 0.226 secondes
Propulsé par Kunena