creste commented on PR #5953:
URL: https://github.com/apache/hadoop/pull/5953#issuecomment-1904433062

   @sugibuchi - Thank you for the additional comments.
   
   >About the descriptions of the four properties, I think we can simply 
copy-paste the descriptions provided by ADD Workload identity documentation.
   > -    fs.azure.account.oauth2.msi.tenant: The tenant ID of the registered 
AAD application or user-assigned managed identity.
   > -    fs.azure.account.oauth2.client.id: The client ID of the AAD 
application or user-assigned managed identity.
   > -    fs.azure.account.oauth2.token.file: The path of the projected service 
account token file.
   
   
   The current descriptions of the properties were copied from other parts of 
the README. For example, see the property descriptions for 
[MSITokenProvider](https://github.com/apache/hadoop/blob/329e3c900bc2668651aa812fa075501c494652df/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md?plain=1#L544).
  @steveloughran or @anmolanmol1234 - what descriptions should the README use 
for those properties?
   
   > 
   > About the description of the auth method:
   > 
   >>    OAuth 2.0 tokens are written to a file that is only accessible from 
the executing pod (`/var/run/secrets/azure/tokens/azure-identity-token`). The 
issued credentials can be used to authenticate.
   > 
   > This is not precise. The token files injected by the AAD workload identity 
webhook are files of "projected service account tokens" issued by Kubernetes 
clusters. They are not OAuth2 access tokens for accessing Azure resources.
   >
   > I propose to update the description of this auth method like:
   >>    With a projected service account token injected by the Azure Workload 
Identity webhook, make a request of the Azure Active Directry endpoint to 
retrieve access tokens.
   >>    The required properties for this authentication method are 
automatically injected into the executing pod as environment variables by the 
AAD Workload Identity webhook.
   
   
   I have no preference, but since this text was also based on other 
descriptions in the README I would appreciate input from a maintainer before 
making the change.  @steveloughran  or @anmolanmol1234 - any thoughts on this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to