-
Notifications
You must be signed in to change notification settings - Fork 65
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
GKE Default Credentials and Temporary Access Tokens #136
GKE Default Credentials and Temporary Access Tokens #136
Conversation
@bradkwadsworth-mw thanks a lot for your contribution 🙌 |
cf7c2ee
to
b8143f4
Compare
Modified my PR to not require changes to the other package. |
@turkenh Just wondering if you could take another look at this when you get a chance? Thanks. |
b8143f4
to
09044d0
Compare
998cb79
to
135bbbd
Compare
135bbbd
to
01ec75c
Compare
@turkenh just wondering if I could get this looked at. This will mimic the functionality of this crossplane-contrib/provider-gcp#461. |
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.
@bradkwadsworth-mw apologizes for the delay here; I missed your comments.
This looks good to me, just left a non-blocking comment.
Please rebase your PR by resolving conflicts so that we can merge it.
@bradkwadsworth-mw there is an interest in this feature, would you be able to continue working on this? I tried to resolve conflicts but failed to push to the branch of this PR due to the lack of permissions. |
Sure, I can try to take a look at it today.
…On Mon, Jul 31, 2023 at 1:12 AM Hasan Turken ***@***.***> wrote:
@bradkwadsworth-mw <https://github.com/bradkwadsworth-mw> there is an
interest in this feature, would you be able to continue working on this?
I tried to resolve conflicts but failed to push to the branch of this PR
due to the lack of permissions.
—
Reply to this email directly, view it on GitHub
<#136 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ATBY2ZNHTUOH6YDI4S4FAN3XS5EDPANCNFSM555HRL4Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
01ec75c
to
f693c5a
Compare
@bradkwadsworth-mw, could you also fix the |
Allow the default credential source to be used for authenticated to a GKE cluster. Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
7e7c224
to
5fc9240
Compare
DCO fixed. |
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.
Thank you @bradkwadsworth-mw 🙌
Signed-off-by: Brad Wadsworth brad.wadsworth@mavenwave.com
Description of your changes
Allow the default credential source to be used for authenticating to a GKE cluster.
A prerequisite for this change is crossplane/crossplane-runtime#337 in order for InjectedIdentity to be used as an option for the CommonCredentialExtractor function.
Fixes #135
I have:
make reviewable
to ensure this PR is ready for review.How has this code been tested
Tested forked controller on a GKE cluster which was successful in applying the helm chart to a remote GKE cluster that had the appropriate IAM permissions for the provider-helm workload identity service account.