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

[identity] simplify code to check IMDS availability #33041

Merged

Conversation

jeremymeng
Copy link
Member

The helper prepareRequestOptions() is used once internally in imdsMsi.ts. It is only used to create a request to invoke a response from IMDS endpoint. This PR simplifies the code to remove the { skipMetadataHeader: true, skipQuery: true, } options argument and creates the request directly.

Packages impacted by this PR

@azure/identity

The helper `prepareRequestOptions()` is used once internally in imdsMsi.ts. It is only used to create a
request to invoke a response from IMDS endpoint. This PR simplifies the code to remove the `{
skipMetadataHeader: true, skipQuery: true, }` options argument and creates the request directly.
@jeremymeng
Copy link
Member Author

I didn't find the helper useful outside of this file.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@minhanh-phan minhanh-phan left a comment

Choose a reason for hiding this comment

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

The change looks good to me, seems like a legacy code. I'll defer the approval to @KarishmaGhiya since she approved this change originally here, so she might know more about the context for this

@KarishmaGhiya
Copy link
Member

KarishmaGhiya commented Feb 18, 2025

@jeremymeng If we are skipping the query from the request for all scenarios, then how will the client ID / resourceID if set, be passed as part of the IMDS request? We haven't accommodated that scenario under the code refine, right? Am I missing something?

@jeremymeng
Copy link
Member Author

@jeremymeng If we are skipping the query from the request for all scenarios, then how will the client ID / resourceID if set, be passed as part of the IMDS request? We haven't accommodated that scenario under the code refine, right? Am I missing something?

The only use of this helper is to send an invalid request to get an error response quickly. skipQuery: true was passed so no query parameters were added to the url as the whole if (!skipQuery) { } block was not executed. When we remove that block, clientId and resourceId parameters become unused so they are also removed.

@jeremymeng jeremymeng merged commit 51c897f into Azure:main Feb 18, 2025
14 checks passed
@jeremymeng jeremymeng deleted the identity/simplify-imds-availability-request branch February 18, 2025 23:29
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.

4 participants