-
Notifications
You must be signed in to change notification settings - Fork 311
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
feat: making iam endpoint universe-aware #1604
Conversation
Use monkeypatch to test whether the correct urls are used. One place to do is test_sign_bytes in test_impersonated_credentials.py. For example, follow pattern in test__get_explicit_environ_credentials in test__default.py |
id_creds = impersonated_credentials.IDTokenCredentials( | ||
credentials, target_audience=target_audience, include_email=True | ||
) | ||
id_creds = id_creds.from_credentials(target_credentials=new_credentials) | ||
id_creds = id_creds.from_credentials(target_credentials=target_credentials) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this line existing? IIUC, removing this will have no impact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to remove the target_audience?
credentials = self.make_credentials( | ||
lifetime=None, source_credentials=source_credentials | ||
) | ||
target_credentials = self.make_credentials( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is same cred created 2 times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good mostly. Have that extra line in id token test. I think there is no need for the 2 creds created in same way. But that seems same way in existing test as well.
This adds Impersonated credential support of universe domain. So they call endpoint in proper universes.