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

gcloud: Use GCE_PROJECT for project always, if specified #750

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

bzub
Copy link
Contributor

@bzub bzub commented Jan 7, 2019

Currently if GCE_SERVICE_ACCOUNT_FILE is set, GCE_PROJECT is ignored. This PR changes the behavior to use GCE_PROJECT for the project in all cases, if it is set.

I believe in practice the most common scenario is that a provided service account file does not specify a project, which is the default when generating a service account key via gcloud CLI or web console. So this change helps avoid forcing users to add a project ID to their service account file by simply specifying GCE_PROJECT as I think a user would expect.

This change also allows a user to override a project id in a service account file, which is probably a less common scenario but is the use case described in #604.

To use the project ID from a service account file, the user simply specifies GCE_SERVICE_ACCOUNT_FILE and not GCE_PROJECT. The new behavior seems most intuitive.

Fixes #604

@ldez ldez self-requested a review January 7, 2019 22:04
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez added this to the v2.0 milestone Jan 8, 2019
@ldez ldez merged commit c938de6 into go-acme:master Jan 8, 2019
@ldez ldez changed the title Use GCE_PROJECT for project always, if specified gcloud: Use GCE_PROJECT for project always, if specified Jan 8, 2019
bzub added a commit to bzub/lego that referenced this pull request Jan 8, 2019
Fixes a typo from go-acme#750 so that GCE_PROJECT is actually used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants