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

Load client-auth plugins #5513

Merged
merged 1 commit into from
Jul 29, 2018

Conversation

ripta
Copy link
Contributor

@ripta ripta commented Jul 24, 2018

Without this change, for example, kops connections using OIDC to the cluster will receive this error message:

cannot build kube client for "$CLUSTER_NAME": No Auth Provider found
for name "oidc"

kubernetes/client-go#345 suggests importing the package path plugin/pkg/client/auth/oidc from client-go. However, looking at the code at plugin/pkg/client/auth, it will actually handle loading all known auth plugins for us.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 24, 2018
@ripta
Copy link
Contributor Author

ripta commented Jul 24, 2018

/assign kashifsaadat

This is the first time I've touched BUILD.bazel, so let me know if I missed anything. That said, make version-dist completes successfully and this change made OIDC auth work correctly.

@ripta
Copy link
Contributor Author

ripta commented Jul 24, 2018

/assign @KashifSaadat

Whoops, left off the @.

@mikesplain
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 25, 2018
@mikesplain
Copy link
Contributor

Hi @ripta, in order to get this to pass you'll need to reorder your insert in BUILD.bazel. Please checkout the failed test.

If you run make bazel-gazelle this should be automatically fixed for you. Then amend your commit with git commit --amend. Let us know if you need more help!

Without this change, for example, kops connections using OIDC to the
cluster will receive this error message:

> cannot build kube client for "$CLUSTER_NAME": No Auth Provider found
> for name "oidc"

kubernetes/client-go#345 suggests importing the package path
`plugin/pkg/client/auth/oidc` from `client-go`, but looking at the code
`plugin/pkg/client/auth` will actually handle loading all known auth
plugins for us.
@ripta
Copy link
Contributor Author

ripta commented Jul 25, 2018

@mikesplain - Thanks for the pointer. I amended my commit. I'll retest and check back.

/retest

@chrisz100
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 29, 2018
@KashifSaadat
Copy link
Contributor

Thanks for the contribution @ripta :)

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrisz100, KashifSaadat, ripta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2018
@k8s-ci-robot k8s-ci-robot merged commit 94d960c into kubernetes:master Jul 29, 2018
@ripta ripta deleted the load-client-auth-plugins branch July 30, 2018 21:07
justinsb added a commit that referenced this pull request Dec 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants