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

[FEATURE REQ] Support Azure IMDS with spring #23424

Closed
stliu opened this issue Aug 9, 2021 · 12 comments
Closed

[FEATURE REQ] Support Azure IMDS with spring #23424

stliu opened this issue Aug 9, 2021 · 12 comments
Assignees
Labels
azure-spring All azure-spring related issues Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.
Milestone

Comments

@stliu
Copy link
Member

stliu commented Aug 9, 2021

Azure IMDS provides various useful metadata for applications, and we should have a property source to consume them and provide to customer application

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 9, 2021
@stliu stliu added the azure-spring All azure-spring related issues label Aug 9, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 9, 2021
@stliu stliu added Client This issue points to a problem in the data-plane of the library. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Aug 9, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 9, 2021
@stliu stliu added feature-request This issue requires a new behavior in the product in order be resolved. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Aug 9, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 9, 2021
@stliu stliu added this to the Backlog milestone Aug 9, 2021
@alzimmermsft alzimmermsft modified the milestones: Backlog, [2022] May Jan 28, 2022
@alzimmermsft
Copy link
Member

alzimmermsft commented Jan 28, 2022

IMDS also provides a Swagger specification which could be leveraged by our code generators to create the public API to support this functionality. Though it may not be required to use the Swagger specification to create this functionality, it'll just help greatly and also may provide a better ability to offer multi-version support.

This still leaves an open question on where the functionality should live. There are a few potential solutions:

  • Include this functionality in azure-core. This provides extreme convenience to anyone who would want to leverage this as it would be included implicitly, downside is that not everyone (possibly most everyone) won't need this functionality and it'll be additional bloat.
  • Creating a new plugin library, similar to azure-identity, to provide this functionality. This provides convenience to add the functionality as it is needed without adding additional bloat to any commonly used SDKs, downside is that users will need to find this functionality when it is needed but this follows an existing pattern so this downside's impact should be reduced.

@joshfree
Copy link
Member

@sandeep-sen volunteered to design this feature request; and get agreement on the capabilities with @stliu in Nickel+, before we commit to implementing it in Copper semester.

@alzimmermsft
Copy link
Member

alzimmermsft commented Feb 1, 2022

Another design consideration, IMDS has rate limiting which restricts to 5 requests per second, except for Managed Identity which is limited to 20 per seconds with 5 concurrent calls. In the case of rate limiting a 429 response will be returned which will allow for the appropriate back-off to be applied, but this still causes difficulties as there isn't a guarantee that our calls will be the only made.

@sandeep-sen
Copy link
Member

@stliu could provide a list of metadata you need. It would be helpful if you break the list into 2 sections must have and good to have

@joshfree
Copy link
Member

@sandeep-sen are you able to get this spec complete for April so devs can pick this up in early Copper?

@stliu
Copy link
Member Author

stliu commented Apr 3, 2022

Another design consideration, IMDS has rate limiting which restricts to 5 requests per second, except for Managed Identity which is limited to 20 per seconds with 5 concurrent calls. In the case of rate limiting a 429 response will be returned which will allow for the appropriate back-off to be applied, but this still causes difficulties as there isn't a guarantee that our calls will be the only made.

actually that's exactly our sdk should consider support this, either option list here is fine #23424 (comment)

@stliu
Copy link
Member Author

stliu commented Apr 3, 2022

@stliu Strong Liu FTE could provide a list of metadata you need. It would be helpful if you break the list into 2 sections must have and good to have

@sandeep-sen I like @alzimmermsft suggestion, probably we should just provide all of IMDS has, since we don't know exactly which customer wants and wich customer doesn't

@stliu stliu modified the milestones: [2022] May, Backlog Apr 3, 2022
@sandeep-sen
Copy link
Member

@alzimmermsft we should create a new plugin library, similar to azure-identity as you suggested.

@JonathanGiles
Copy link
Member

@sandeep-sen Can you check to see if this should be done cross-language, and if so, get the work prioritised as appropriate? Thanks

@sandeep-sen
Copy link
Member

sandeep-sen commented Apr 27, 2022

Requirement

  • Get all runtime metadata via IMDS and expose it to the user
  • Spring can expose these metadata objects through config properties
Note: This feature is not present in any other language SDK and we will need arch board review

Next Steps

Create a MVP interfaces and review with spring team. Once spring team has approved push for arch board review

MVP

@lmolkova's proposal:
We have nothing in core
IMDS communication is handled in spring integration (it's just reusing core primitives - HTTP client)
Users can get individual fields by adding @Value annotation

@stliu's and @saragluna's proposal:
Configure in properties like spring.cloud.azure.profile.cloud-type

Estimated Cost

Task Cost
Prototype 2 days
Reviews 5 days
Implementation 5 days
Total 2.5 weeks

References

@sandeep-sen
Copy link
Member

@srnagar is checking with arch board if any other language has a requirement to expose instance metadata

@srnagar
Copy link
Member

srnagar commented Aug 19, 2022

Currently, there's no plan to support track 2 Azure IMDS library.

If Spring libraries need this, we can generate the client from AutoRest and include it in the implementation package (no public API should be added) of Spring libraries until a track 2 data-plane library is released.

@srnagar srnagar closed this as completed Aug 19, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this issue Apr 7, 2023
merging billing fix to public repo (Azure#23424)

Co-authored-by: Gaurav Bang <gauravbang@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
azure-spring All azure-spring related issues Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.
Projects
Status: Done
Archived in project
Development

No branches or pull requests

6 participants