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

Reimplement ManagedIdentityCredential with composition #9302

Closed
chlowell opened this issue Jan 2, 2020 · 4 comments
Closed

Reimplement ManagedIdentityCredential with composition #9302

chlowell opened this issue Jan 2, 2020 · 4 comments
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated

Comments

@chlowell
Copy link
Member

chlowell commented Jan 2, 2020

ManagedIdentityCredential exists to hide MsiCredential and ImdsCredential behind a facade that has the right surface for documentation and code completion. It accomplishes this by overriding its constructor to return an instance of one of those classes, and defining empty methods with docstrings.

It could instead hold another credential and wrap its methods, as does EnvironmentCredential.

@chlowell chlowell added Azure.Identity Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated labels Jan 2, 2020
@HankiGreed
Copy link
Contributor

HankiGreed commented Feb 2, 2020

I want to take this issue for my first contribution. Would appreciate if someone could give me a little headstart about it.

@chlowell
Copy link
Member Author

chlowell commented Feb 3, 2020

The gist is that ManagedIdentityCredential is currently a factory that can't itself be instantiated. It overrides __new__ to return one of the implementation credentials. So, you call ManagedIdentityCredential() and get something else:

>>> ManagedIdentityCredential()
<azure.identity._credentials.managed_identity.ImdsCredential object at 0x011EAF88>

ManagedIdentityCredential could instead have an ordinary __init__ and hold an instance of the appropriate implementation credential. It could implement get_token by wrapping that credential's get_token method, similarly to EnvironmentCredential. I suggest starting with a look at EnvironmentCredential.

@chlowell
Copy link
Member Author

Addressed by #9302, thanks @HankiGreed!

@HankiGreed
Copy link
Contributor

Thanks for giving me a chance to do this. This is my first contribution on GitHub and it felt good doing it. Thanks @chlowell for bearing with me throughout. !

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. help wanted This issue is tracking work for which community contributions would be welcomed and appreciated
Projects
None yet
Development

No branches or pull requests

2 participants