JulianHayward / Azure-MG-Sub-Governance-Reporting

Azure Governance Visualizer aka AzGovViz is a PowerShell script that captures Azure Governance related information such as Azure Policy, RBAC (a lot more) by polling Azure ARM, Storage and Microsoft Graph APIs.
MIT License
845 stars 304 forks source link

Azure DevOps pipeline - Explicit 'false' instead of default (implicit 'false') for at least two switch parameters cause AzGovViz to crash early #186

Closed jsandquist closed 1 year ago

jsandquist commented 1 year ago

First and foremost - Thanks for the great work with AzGovViz!

This is a very minor issue I recently stumbled upon - and with an easy fix (work-around) in the 'variables' file to just leave the variables empty instead of explicitly setting a value equal to the default value (for switch parameters).

AzGovViz version 6.2.0

CodeRunPlatform Azure DevOps - self-hosted agent on Windows. Due to issues with PowerShell => pwsh 7.3.3 I have also set pwsh: true for the step at https://github.com/JulianHayward/Azure-MG-Sub-Governance-Reporting/blob/master/.azuredevops/pipelines/AzGovViz.pipeline.yml#L99

Describe the bug This is really minor but If I explicitly set any of the following (last) variables to false AzGovViz errors out in some surprising ways, e.g. NoPIMEligibility as follows:

# Do not report on PIM (Privileged Identity Management) Eligible Role assignments
  - name: NoPIMEligibility
    # Switch | example: value: true
    value: false

# Ignore the current scope (ManagementGrouId) and get all PIM (Privileged Identity Management) eligible Role assignments
  - name: PIMEligibilityIgnoreScope
    # Switch | example: value: true
    value:

# Prevent integration of PIM eligible assignments with RoleAssignmentsAll (HTML, CSV)
  - name: NoPIMEligibilityIntegrationRoleAssignmentsAll
    # Switch | example: value: true
    value:

# Do not execute Azure Landing Zones (ALZ) Policy Version Checker
  - name: noALZPolicyVersionChecker
    # Switch | example: value: true
    value:

# Create a dedicated DefinitionInsights HTML file
  - name: NoDefinitionInsightsDedicatedHTML
    # Switch | example: value: true
    value:

# Do not execute Storage Account Access Analysis
  - name: NoStorageAccountAccessAnalysis
    # Switch | example: value: true
    value:

# Do not execute Network analysis / Virtual Network and Virtual Network Peerings
  - name: NoNetwork
    # Switch | example: value: true
    value:

# Dynamic Variables - Do Not Modify Anything Below this line!

If you have previously set it to true and then change it to false (instead of no value to pick up the default) you might see some strange errors.

Occasionally we also saw this (due to the wiki folder not being present I suppose as the logic then does some extra checks but I might be way off here):

VERBOSE: Command [Connect-AzAccount] succeeded.
##[error]Cannot process argument transformation on parameter 'HtmlTableRowsLimit'. Cannot convert value "false" to type "System.Int32". Error: "The input string 'false' was not in a correct format."
##[error]PowerShell exited with code '1'.

A clear and concise description of what the bug is / Paste the error from the script output (replace your tenantId and subscriptionIds)

Verify 'AzAPICall' (false)
AzAPICall - Deviating module version 1.1.72
'AzAPICall false' not installed
Installing AzAPICall module (false)
##[error]  Installing 'AzAPICall' module (false) failed
##[error]PowerShell exited with code '1'.

Screenshots If applicable, add screenshots to help explain your problem. Screenshot is only an addition paste the error output as text, please.

image

Additional context I suspect the pipeline logic at https://github.com/JulianHayward/Azure-MG-Sub-Governance-Reporting/blob/master/.azuredevops/pipelines/AzGovViz.pipeline.yml#L45 but I have not tried to pinpoint the issue yet.

JulianHayward commented 1 year ago

@jsandquist nice find so if it is a switch parameter but not true it passes on: -switchParameter false Will you create a PR?

file: AzGovViz.pipeline.yml

image
jsandquist commented 1 year ago

@JulianHayward Sure - that looks like a bit-sized task. :-) All the excessive parameters (false here) fill in values from the start of the argument list so $Product = false and then $AzAPICallVersion = false and so on. Makes sense now.

JulianHayward commented 1 year ago

hi @jsandquist will you PR, or should we close?