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

Add basic support for Exec auth #54

Merged
merged 3 commits into from
Jul 24, 2019
Merged

Conversation

johnpoth
Copy link
Collaborator

Adds basic support for Exec Auth (browser is not supported). Tested on EKS. #32 #53
Thanks!

Copy link
Owner

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

That's awesome! At some point, we need to update the README to document support for AWS EKS and GCP that's brought by this PR.

lib/config/user.js Outdated Show resolved Hide resolved
lib/config/user.js Outdated Show resolved Hide resolved
lib/config/user.js Outdated Show resolved Hide resolved
lib/config/user.js Outdated Show resolved Hide resolved
lib/auth/exec-auth.js Outdated Show resolved Hide resolved
lib/auth/exec-auth.js Outdated Show resolved Hide resolved
lib/auth/exec-auth.js Outdated Show resolved Hide resolved
lib/auth/exec-auth.js Outdated Show resolved Hide resolved
lib/auth/exec-auth.js Outdated Show resolved Hide resolved
@johnpoth
Copy link
Collaborator Author

Thanks for reviewing! I've addressed the comments and I added another commit to support Digital Ocean. Let me know if you prefer them squashed?

Should we add something to the readme now or wait when we're about to release? I was thinking of a support matrix browser/no browser auth support. Thanks!

lib/auth/gcp.js Outdated Show resolved Hide resolved
lib/config/user.js Outdated Show resolved Hide resolved
lib/config/user.js Outdated Show resolved Hide resolved
@astefanutti
Copy link
Owner

Thanks for reviewing! I've addressed the comments and I added another commit to support Digital Ocean. Let me know if you prefer them squashed?

Better keeping the two commits.

Should we add something to the readme now or wait when we're about to release? I was thinking of a support matrix browser/no browser auth support. Thanks!

I'm fine with updating the README now, even if it should theoretically be done post-release. Anyway, I was thinking cutting a release ASAP, with all the awesome stuff you've added. I like the idea of the support matrix. I would suggest to put it into a dedicated section in the README.

@johnpoth
Copy link
Collaborator Author

Thanks for reviewing! I've addressed the comments and I added another commit to support Digital Ocean. Let me know if you prefer them squashed?

Better keeping the two commits.

Should we add something to the readme now or wait when we're about to release? I was thinking of a support matrix browser/no browser auth support. Thanks!

I'm fine with updating the README now, even if it should theoretically be done post-release. Anyway, I was thinking cutting a release ASAP, with all the awesome stuff you've added. I like the idea of the support matrix. I would suggest to put it into a dedicated section in the README.

Added a note in the README

Copy link
Owner

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

Could you also update the Authentication support item from the Features section, to have EKS, and GKE listed here too?

README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
README.adoc Outdated Show resolved Hide resolved
@johnpoth
Copy link
Collaborator Author

Updated PR, thanks!

@astefanutti astefanutti merged commit 0767573 into astefanutti:master Jul 24, 2019
@astefanutti
Copy link
Owner

Thanks! I'll try to cut a release ASAP.

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.

2 participants