Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new AzureCLI authentication options for GenerateResourcesAndImage and Packer templates #10602

Closed
wants to merge 30 commits into from

Conversation

feliasson
Copy link

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

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

@feliasson feliasson marked this pull request as draft September 12, 2024 13:32
@feliasson
Copy link
Author

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 feliasson marked this pull request as ready for review September 12, 2024 13:54
Copy link

@kedar-1 kedar-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested UseAzureCliAuth option based on the code above and works as expected.

@feliasson
Copy link
Author

@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.

@flannoo
Copy link

flannoo commented Oct 7, 2024

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!

Copy link

@invisiblepancake invisiblepancake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i got that azufe certific in an milestone oslt. check where its from =) its been an few days bender.

@subir0071 subir0071 closed this Jan 27, 2025
@feliasson
Copy link
Author

@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?

@kedar-1
Copy link

kedar-1 commented Jan 30, 2025

@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?!

@subir0071
Copy link
Contributor

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.

@feliasson
Copy link
Author

@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
Copy link
Author

@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.

Copy link

@invisiblepancake invisiblepancake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes:
Adds a new parameter UseAzureCliAuth to the GenerateResourcesAndImage.ps1 script.
Updates Packer templates for various OS versions to include the use_azure_cli_auth variable.
Modifies authentication logic to use Azure CLI credentials if UseAzureCliAuth is enabled.
Benefits:
Simplifies Authentication: No need for privileged Microsoft Entra roles or pre-registered Service Principals.
Reduces Overhead: Eliminates the need to manage and rotate Service Principal secrets.
Integration: Easily integrates with Azure CLI tasks in Azure Pipelines and GitHub Actions.
Flexibility: Provides an option to use existing Azure CLI login credentials for authentication.
These changes enhance the flexibility and ease of use for authenticating with Azure, particularly in automated environments.

Copy link

@kedar-1 kedar-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants