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

do not poll for availability if MSI type is not IMDS #685

Merged
merged 2 commits into from May 4, 2022

Conversation

scott-the-programmer
Copy link
Contributor

@scott-the-programmer scott-the-programmer commented Feb 24, 2022

Thank you for your contribution to Go-AutoRest! We will triage and review it as soon as we can.

As part of submitting, please make sure you can make the following assertions:

  • I've tested my changes, adding unit tests if applicable.
  • I've added Apache 2.0 Headers to the top of any new source files.

Description

This PR tries to solve #684 by leveraging getMSIType() to ensure that we're not pinging for availability for non-IMDS endpoints (as this is should instead manifest itself as an auth error)

In an app service environment, the endpoint will be set to $MSI_ENDPOINT. If $MSI_ENDPOINT does not exist, it will assume that the msi type is IMDS and behave as it did previously

Validation in Azure

Validated in an Azure App Service by running

	authorizer, err := kvauth.NewAuthorizerFromEnvironment() 
	if err != nil {
		return nil, fmt.Errorf("key vault authorizer error: %w", err)
	}

@ghost
Copy link

ghost commented Feb 24, 2022

CLA assistant check
All CLA requirements met.

autorest/adal/token.go Outdated Show resolved Hide resolved
@scott-the-programmer scott-the-programmer changed the title use MSI_ENDPOINT when required for availability do not poll for availability if MSI type is not IMDS Apr 22, 2022
@jhendrixMSFT jhendrixMSFT merged commit 4990699 into Azure:master May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants