Skip to content

Conversation

@maorleger
Copy link
Member

@maorleger maorleger commented Mar 10, 2025

Packages impacted by this PR

@azure/identity
@azure/keyvault-admin
@azure/keyvault-keys

Issues associated with this PR

Describe the problem that is addressed by this PR

Builds on previous work in #31335
This PR adds support for PersistOidcToken parameter passing, allowing one to
refresh their credential using Connect-AzAccount or az login

@maorleger
Copy link
Member Author

/azp run js - identity - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maorleger
Copy link
Member Author

/azp run js - keyvault-admin - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@maorleger maorleger changed the title wip [engSys] Use common OIDC token env vars for live tests Mar 10, 2025
@maorleger
Copy link
Member Author

KeyVault Admin Managed HSM successfully deployed after this change: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4635988, test failures are related to test pollution and unrelated to this PR

Identity deployment succeeds as well: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4636004

Live test errors are pre-existing on main

@maorleger maorleger marked this pull request as ready for review March 10, 2025 21:00
Copilot AI review requested due to automatic review settings March 10, 2025 21:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR adds support for the PersistOidcToken parameter across various pipeline templates and test configurations for live tests. The key changes include:

  • Adding a new PersistOidcToken boolean parameter in multiple pipeline templates.
  • Propagating the PersistOidcToken parameter into job definitions.
  • Updating test configurations in the SDK packages to leverage the new parameter.

Reviewed Changes

File Description
eng/pipelines/templates/stages/archetype-sdk-tests.yml Added PersistOidcToken parameter and passed it to the extended stage settings
eng/pipelines/templates/stages/archetype-sdk-tests-isolated.yml Added PersistOidcToken parameter and passed it along for isolated test stages
eng/pipelines/templates/jobs/live.tests.yml Defined PersistOidcToken as a parameter and injected it into job configurations
sdk/identity/identity/tests.yml Removed the inline script that set OIDC variables and replaced it with PersistOidcToken set to true
sdk/keyvault/keyvault-admin/tests.yml Added PersistOidcToken set to true in the test configuration

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

sdk/identity/identity/tests.yml:10

  • The inline script setting OIDC variables has been removed in favor of using PersistOidcToken. Please verify that the new mechanism correctly establishes and propagates the OIDC environment variables, ensuring no regression in test behavior.
PersistOidcToken: true

eng/pipelines/templates/jobs/live.tests.yml:48

  • There is an inconsistency where the live.tests pipeline defaults PersistOidcToken to false, yet tests in SDK configurations override it to true. Please ensure that the intended behavior is consistent across environments.
- name: PersistOidcToken

@maorleger maorleger force-pushed the persist-oidc-token branch from 3782227 to f38774d Compare March 10, 2025 21:30
@github-project-automation github-project-automation bot moved this from Untriaged to In Progress in Azure Identity SDK Improvements Mar 10, 2025
@maorleger maorleger merged commit 7df8e99 into Azure:main Mar 10, 2025
24 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Azure Identity SDK Improvements Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants