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.

Sign up for GitHub

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

Jump to bottom

lazy load azure credentials for app and fix cache ctx cancel #41648

Merged
merged 3 commits into from
May 27, 2024
Merged

lazy load azure credentials for app and fix cache ctx cancel #41648

merged 3 commits into from
May 27, 2024

Conversation

greedy52
Copy link
Contributor

No description provided.

@greedy52 greedy52 added backport/branch/v13 backport/branch/v14 no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 labels May 16, 2024
@greedy52 greedy52 requested a review from Tener May 16, 2024 14:47
@greedy52 greedy52 self-assigned this May 16, 2024
@greedy52 greedy52 requested a review from GavinFrazar May 16, 2024 14:47
Tener
Tener approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

Looks good to me. What was the original issue that gets fixed here?

@greedy52 greedy52 requested a review from GavinFrazar May 17, 2024 13:32
@greedy52 greedy52 marked this pull request as ready for review May 17, 2024 13:32
@greedy52
Copy link
Contributor Author

Looks good to me. What was the original issue that gets fixed here?

Sorry there are two separate issues:

  • First one is Failed to load workload identity. Using managed identity. info log shows up all the time even when not doing Azure App
  • Second one is the race condition in getToken that it can return nil, nil if input context is canceled.

@Tener
Copy link
Contributor

Tener commented May 24, 2024 •
edited

I think this one is good for merging? Except for missing g1 approval, that is.

@greedy52 greedy52 requested a review from smallinsky May 24, 2024 13:01
@greedy52 greedy52 added this pull request to the merge queue May 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2024
@greedy52 greedy52 enabled auto-merge May 27, 2024 14:02
@greedy52 greedy52 added this pull request to the merge queue May 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2024
@greedy52 greedy52 added this pull request to the merge queue May 27, 2024
Merged via the queue into master with commit 039c7b4 May 27, 2024
36 checks passed
@greedy52 greedy52 deleted the STeve/fix_azure_app branch May 27, 2024 17:09
@public-teleport-github-review-bot
Copy link

@greedy52 See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Failed
branch/v15 Create PR

@public-teleport-github-review-bot
Copy link

@greedy52 See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Failed

@greedy52 greedy52 mentioned this pull request Jun 4, 2024
Merged
greedy52 added a commit that referenced this pull request Jun 4, 2024
@greedy52
lazy load azure credentials for app and fix cache ctx cancel (#41648)
249fc0d 
* lazy load azure credentials for app and fix cache ctx cancel

* fix race again
greedy52 added a commit that referenced this pull request Jun 4, 2024
@greedy52
fix app cache ctx cancel (#41648)
c9db70d 
* lazy load azure credentials for app and fix cache ctx cancel

* fix race again
This was referenced Jun 4, 2024
Merged
Closed
github-merge-queue bot pushed a commit that referenced this pull request Jun 5, 2024
@greedy52
fix app cache ctx cancel (#41648) (#42376)
97ec41e 
* lazy load azure credentials for app and fix cache ctx cancel

* fix race again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tener Tener Tener approved these changes

@GavinFrazar GavinFrazar GavinFrazar approved these changes

@smallinsky smallinsky smallinsky approved these changes

Assignees

@greedy52 greedy52

Labels
application-access backport/branch/v14 backport/branch/v15 backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@greedy52 @Tener @GavinFrazar @smallinsky