-
Notifications
You must be signed in to change notification settings - Fork 39
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: allow customizing context_name, default to the same as gcloud #159
Conversation
src/main.ts
Outdated
const location = getInput('location', { required: true }); | ||
const credentials = getInput('credentials'); | ||
const projectId = getInput('project_id'); | ||
const useAuthProvider = getBooleanInput('use_auth_provider'); | ||
const useInternalIP = getBooleanInput('use_internal_ip'); | ||
const contextName = getInput('context_name') || `gke_${projectId}_${location}_${clusterName}`; |
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.
🐉 for discussion. This changes the default to match gcloud's default naming, but it changes the existing behavior for default naming.
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.
IMO switching to gcloud default should be okay. However projectId
is not a required input and maybe empty here as we support discovery via GCLOUD_PROJECT
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.
Good point. Can we merge #160 first since it has the nicer project ID extraction at this layer?
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.
@sethvargo Just merged #160
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.
Done - this logic gets a little circular, so please take a close look
b18ce26
to
2b69107
Compare
2b69107
to
f98ea52
Compare
@@ -57,15 +60,42 @@ async function run(): Promise<void> { | |||
|
|||
// Pick the best project ID. | |||
if (!projectID) { | |||
if (credentialsJSON && isServiceAccountKey(credentialsJSON)) { | |||
if (clusterName.projectID) { |
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.
I set the order of precedence here to be:
- Project ID as an input
- Project ID in the resource name
- Project ID from credentials
- Environment variable
Copied description from https://github.com/google-github-actions/get-gke-credentials/blob/ddd7b291443a04993b1497fc36583a21ec67a5fe/action.yaml#L58-L62 #86 #159 Signed-off-by: Chenyu Zhao <[email protected]>
Fixes #86