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

fix(auth): Fixed the ability to discover the project ID when running on GCP managed environments #755

Merged
merged 2 commits into from Jan 10, 2020

Conversation

hiranya911
Copy link
Contributor

Support for auto discovering the project ID when running on Compute Engine, Cloud Run etc.

Verified by running a build on a Compute Engine instance.

Copy link
Contributor

@bojeil-google bojeil-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of nits. Otherwise looks good.

@@ -56,14 +56,15 @@ export function addReadonlyGetter(obj: object, prop: string, value: any): void {
}

/**
* Determines the Google Cloud project ID associated with a Firebase app by examining
* the Firebase app options, credentials and the local environment in that order.
* Returns the Google Cloud project ID associated with a Firebase app, if its explicitly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -328,6 +329,47 @@ describe('Credential', () => {
});
});
});

it('should cache discovered project id', () => {
const expected = 'test-project-id';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Rename expected to expectedProjectId here and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@hiranya911
Copy link
Contributor Author

@bojeil-google I've made the suggested changes.

@lahirumaramba lahirumaramba removed their assignment Jan 10, 2020
@hiranya911 hiranya911 merged commit 2cfa51a into master Jan 10, 2020
@hiranya911 hiranya911 deleted the hkj-project-id branch January 10, 2020 22:19
@hiranya911 hiranya911 changed the title Discovering project ID from the local metadata service fix(auth): Fixed the ability to discover the project ID when running on GCP managed environments Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants