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

Talaria panic to auth Azure when runs on AWS #88

Closed
atlas-comstock opened this issue May 6, 2022 · 8 comments
Closed

Talaria panic to auth Azure when runs on AWS #88

atlas-comstock opened this issue May 6, 2022 · 8 comments

Comments

@atlas-comstock
Copy link
Collaborator

Talaria panic to auth Azure when runs on AWS

Background

169.254.169.254 is a magic IP(Link-local address) used by most cloud services (AWS & Azure) for metadata.

This line of code introduced in the latest MR will set the URL http://169.254.169.254/metadata/identity/oauth2/token to get the metadata and send HTTP requests when refreshing. The Azure library did this.

However, when you run the Talaria on AWS, there is no such URL/info provided by it; it will be a timeout as there is no route.

Propose

Skip the auth here when Talaria is not running on Azure; directly uses the Env to auth.
@ocassetti
Copy link
Collaborator

Could you share the stack trace because in theory adal.NewServicePrincipalTokenFromManagedIdentity should return and error and then it should use the env credentials

@atlas-comstock
Copy link
Collaborator Author

Here is the log. @ocassetti
NewServicePrincipalTokenFromManagedIdentity just init the options and does not actually request the HTTP endpoint, therefore the error was not raised there. Instead, it raised at the refresh stage.

2022/05/04 09:31:48 azure: acquired Manange Identity Credentials
panic: ServerError: target=storage/writer/azure/azure.go.107, reason=Internal Server Error, msg=azure: unable to get azure storage credential due to adal: Refresh request failed. Status Code = '404'. Response body: <?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
 <head>
  <title>404 - Not Found</title>
 </head>
 <body>
  <h1>404 - Not Found</h1>
 </body>
</html>
 Endpoint http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fstorage.azure.com%2F

@kelindar
Copy link
Collaborator

kelindar commented May 6, 2022

It is trying to hit Azure instance metadata from AWS? Looks like this cannot work

@atlas-comstock
Copy link
Collaborator Author

@kelindar Yes, it can't work, of course. Therefore we should skip this logic and use ENV setting as auth for Talaria not on Azure.

@ocassetti
Copy link
Collaborator

ocassetti commented May 9, 2022 •
edited
Loading

But NewServicePrincipalTokenFromManagedIdentity should return an error then we check for engiroment credentials... that's the logic...

@atlas-comstock
Copy link
Collaborator Author

@ocassetti hi, you can check the implementation of NewServicePrincipalTokenFromManagedIdentity, it just inits the options and does not actually request the HTTP endpoint, therefore the error was not raised there. Instead, it raised at the refresh stage.

@ocassetti
Copy link
Collaborator

Oh I see, then I guess we need to check if the env variable are set then we use the environment otherwise we use the principal

@atlas-comstock
Copy link
Collaborator Author

Good idea, better than skipping on other cloud services.

atlas-comstock pushed a commit to atlas-comstock/talaria that referenced this issue May 18, 2022
@hiohiograb
Fix issue talariadb#88: Use env to set Azure auth if existed
e6a773c
atlas-comstock pushed a commit to atlas-comstock/talaria that referenced this issue May 19, 2022
@hiohiograb
Fix issue talariadb#88: Use env to set Azure auth if existed
95344a7
kelindar pushed a commit that referenced this issue Jun 2, 2022
@atlas-comstock
Fix issue #88: Use env to set Azure auth if existed (#91)
c6fb73c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kelindar @ocassetti @atlas-comstock