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

oidc authenticator: allow http.Client to be overridden #106141

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

enj
Copy link
Member

@enj enj commented Nov 4, 2021

This change allows the http.Client used by the OIDC authenticator to
be overridden. This is useful when this code is being used as a
library outside of core Kubernetes. For example, a downstream
consumer may want to override the http.Client's internals such as
its TLS configuration.

Signed-off-by: Monis Khan mok@vmware.com

/kind cleanup
/milestone v1.23
/triage accepted
/priority backlog
/assign @liggitt

xref: #99765

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Nov 4, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Nov 4, 2021
@k8s-ci-robot k8s-ci-robot added priority/backlog Higher priority than priority/awaiting-more-evidence. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 4, 2021
@k8s-ci-robot k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 4, 2021
@@ -81,9 +81,12 @@ type Options struct {
// See: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
ClientID string

// PEM encoded root certificate contents of the provider.
// PEM encoded root certificate contents of the provider. Ignored when Client is not nil.
Copy link
Member

Choose a reason for hiding this comment

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

would it be better to make this and Client mutually exclusive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I think so. Updated and added unit test.

ClientID: "my-client",
UsernameClaim: "username",
GroupsClaim: "groups",
Client: http.DefaultClient, // cause distributed claims fetching to fail with an unknown CA error
Copy link
Member

Choose a reason for hiding this comment

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

any way to verify the error or verify this client gets called?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the test logic so that I could assert the returned error.

@enj enj added this to Needs Triage in SIG Auth Old Nov 6, 2021
@enj enj moved this from Needs Triage to Changes Requested in SIG Auth Old Nov 6, 2021
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 8, 2021
This change allows the http.Client used by the OIDC authenticator to
be overridden.  This is useful when this code is being used as a
library outside of core Kubernetes.  For example, a downstream
consumer may want to override the http.Client's internals such as
its TLS configuration.

Signed-off-by: Monis Khan <mok@vmware.com>
@liggitt
Copy link
Member

liggitt commented Nov 17, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, liggitt

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 Nov 17, 2021
@k8s-ci-robot k8s-ci-robot merged commit 7eb502b into kubernetes:master Nov 17, 2021
SIG Auth Old automation moved this from Changes Requested to Closed / Done Nov 17, 2021
margocrawf added a commit to vmware-tanzu/pinniped that referenced this pull request Apr 18, 2022
Kube 1.23 introduced a new field on the OIDC Authenticator which
allows us to pass in a client with our own TLS config. See
kubernetes/kubernetes#106141.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
margocrawf added a commit to vmware-tanzu/pinniped that referenced this pull request Apr 19, 2022
Kube 1.23 introduced a new field on the OIDC Authenticator which
allows us to pass in a client with our own TLS config. See
kubernetes/kubernetes#106141.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
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. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Archived in project
SIG Auth Old
Closed / Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants