runner-images icon indicating copy to clipboard operation
runner-images copied to clipboard

Add new AzureCLI authentication options for GenerateResourcesAndImage and Packer templates

Open feliasson opened this issue 1 year ago • 3 comments

Description

This PR introduces new authentication options for the GenerateResourcesAndImage.ps1 helper script and the Packer templates for ubuntu and windows. By leveraging the use_azure_cli_auth optional value in Packer azure-arm builder (ref) this PR provides new ways to authenticate while building the runner-images.

  • A new switch called UseAzureCliAuth is introduced in the helper script.
    • Solely relies on the credentials that you set in AzureCLI using az login.
    • Skips the SPN-registration even if no client id is provided in AzureClientId parameter
    • Ignores all other authentication configurations if enabled (see parameter explanation)
    • It defaults to false in both the helper-script and packer-templates and does not break the approach of using SPN authentication

What advantages does using the new switch give?

  • No longer needed to have privileged Microsoft Entra roles such as Application Developer or Application Administrator to run script / build without AzureClientId and AzureClientSecret inputs.
    Directory permission is needed for the current user to register the application. For how to configure, please refer 
    'https://docs.microsoft.com/azure/azure-resource-manager/resource-group-create-service-principal-portal'. Original error: 
    Insufficient privileges to complete the operation.
    
  • No longer needed to preregister a Service Principal with Secret and input it for the AzureClientId and AzureClientSecret inputs.
  • Solves having to rotate SPN secrets
  • Can easily be used with AzureCLI@2 task for Azure Pipelines
  • Can easily be used with GitHub Action for Azure CLI

Azure Pipeline example using new UseAzureCliAuth switch

Service-connection is using Azure managed identity and federated credentials

steps:

  # Using Azure CLI credentials for Packer
  - task: AzureCLI@2
    displayName: Generate ${{ parameters.imageType }} Image
    inputs:
      azureSubscription: ${{ parameters.azureSubscription }}
      scriptType: pscore
      scriptLocation: inlineScript
      inlineScript: |
        git clone https://github.com/feliasson/runner-images.git # Temp add for my fork
        cd ./runner-images
        git checkout UseAzureCliAuth-1 # Temp add for my branch
        cd ./helpers
        Import-Module ./GenerateResourcesAndImage.ps1
        
        GenerateResourcesAndImage `
        -SubscriptionId ${{ parameters.subscriptionId }} `
        -ResourceGroupName temp-azcli-runner-image-rg `
        -ImageType ${{ parameters.imageType }} `
        -AzureLocation ${{ parameters.location }} `
        -ImageGenerationRepositoryRoot $(Build.SourcesDirectory)/runner-images `
        -ReuseResourceGroup `
        -UseAzureCliAuth `
        -Verbose

image

Azure Pipeline example using old SPN method

Service-connection is using Azure managed identity and federated credentials

  steps:

  # Using SPN credentials for Packer
  - task: AzureCLI@2
    displayName: Generate ${{ parameters.imageType }} Image
    inputs:
      azureSubscription: ${{ parameters.azureSubscription }}
      scriptType: pscore
      scriptLocation: inlineScript
      inlineScript: |
        git clone https://github.com/feliasson/runner-images.git # Temp add for my fork
        cd ./runner-images
        git checkout UseAzureCliAuth-1 # Temp add for my branch
        cd ./helpers
        Import-Module ./GenerateResourcesAndImage.ps1
        
        GenerateResourcesAndImage `
        -SubscriptionId ${{ parameters.subscriptionId }} `
        -ResourceGroupName temp-spn-runner-image-rg `
        -ImageType ${{ parameters.imageType }} `
        -AzureLocation ${{ parameters.location }} `
        -ImageGenerationRepositoryRoot $(Build.SourcesDirectory)/runner-images `
        -AzureClientId $(appId) `
        -AzureClientSecret $(appSecret) `
        -ReuseResourceGroup `
        -Verbose

image

Running locally using my az login credentials only

image

Related issue:

#10236 - I added the suggestion to let active az login be used if found

Check list

  • [x] Related issue / work item is attached
  • [ ] Tests are written (if applicable)
  • [x] Documentation is updated (if applicable) - I didn't see any other optional parameters documented
  • [x] Changes are tested and related VM images are successfully generated

feliasson avatar Sep 12 '24 11:09 feliasson

Fixed slight adjustment to suggestion in #10236 to properly handle the error if not logged in, it would not enter the catch block otherwise

feliasson avatar Sep 12 '24 13:09 feliasson

@mikhailkoliada @shamil-mubarakshin You guys made the latest commits to the GenerateResourcesAndImage.ps1 helper script. What do I need to do to get some traction on this PR? I have waited 3 weeks already.

feliasson avatar Oct 04 '24 10:10 feliasson

We are currently waiting for this as well, since we prefer to use OIDC authentication (federated) instead of client secrets in our devops pipelines. Would be great if this can be released short term.

Thanks!

flannoo avatar Oct 07 '24 08:10 flannoo

@subir0071 - After nearly 6 months without any review or even a comment from any approvers, maintainers, or code owners, you're just closing this PR?

This approach is incredibly ridiculous.

Why not remove CONTRIBUTING.md altogether instead of wasting everyone's time?

feliasson avatar Jan 27 '25 17:01 feliasson

@subir0071 What the hell are you doing here?! This PR was verified by several people, and standard Azure CLI was used, for crying out loud! Do you think we're all incompetent or something?!

kedar-1 avatar Jan 30 '25 19:01 kedar-1

My apologies and we really believe all our contributors are much important. However, the concern in this PR as below -

  • actual business logic is pushed to the catch block.
  • when Variable AzureClientId is Not Null and UseAzureCliAuth is False the application will create a new service Principal needlessly. code.

Please let me know if I am missing something here.

subir0071 avatar Jan 30 '25 21:01 subir0071

@subir0071

  • actual business logic is pushed to the catch block.

If you know of any other way to check if you're already logged in to the Azure CLI (without making it interactive) and without generating unnecessary exceptions or error outputs, please let me know. I couldn't find any alternative method to check for an existing login to Azure CLI without using a try/catch approach.

If you're not logged in, it always produces output to stderr, which is why I redirect it to $null.

I can't quite remember why I included || Write-Error $_ though, it's been a while and I can't recall the exact reason.

  • when Variable AzureClientId is Not Null and UseAzureCliAuth is False the application will create a new service Principal needlessly. code.

Yes, if you're not providing your own AzureClientId and not using UseAzureCliAuth, it should automatically attempt to create the Service Principal for you. Creating this SPN has been your default approach, so I’m not sure what you mean by 'needlessly' here. If I'm missing something, please elaborate.

feliasson avatar Jan 31 '25 07:01 feliasson

@subir0071

I’d also like to point out that if the changes in GenerateResourcesAndImage.ps1 are a concern, the modifications in the Packer templates can be easily implemented without causing any breaking changes.

This also provides the flexibility to use your Packer templates with Azure CLI authentication, rather than being required to use a Service Principal.

feliasson avatar Jan 31 '25 08:01 feliasson

I'd REALLY like to see this PR re-opened and accepted. OIDC is the way everything is headed - it's not reasonable to leave it unsupported.

ELeeScape avatar Feb 04 '25 23:02 ELeeScape

Since the PR was closed without merging, I guess it's probably dead, but for what it's worth: I'm using feliasson's PR branch to build images via Managed Identities in my Azure subscription and it works great! Much thanks to feliasson. Much less thanks to the people who have killed at least two community PRs for this feature so far, with no real explanation.

ELeeScape avatar Feb 05 '25 21:02 ELeeScape

Hi approvers, maintainers, or code owners Is it possible to reopen this great PR and merge it ? Thank you

kevindelmont avatar Mar 11 '25 13:03 kevindelmont