jetstack / kube-oidc-proxy

Reverse proxy to authenticate to managed Kubernetes API servers via OIDC.
https://jetstack.io
Apache License 2.0
477 stars 91 forks source link

Adds ability to watch files and exit gracefully if they change #114

Closed JoshVanL closed 4 years ago

JoshVanL commented 4 years ago
Adds options to enable watching files that cause a graceful exit when modified

In favor of #88

/hold /assign fixes #16 /milestone v0.2

jetstack-bot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL

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: - ~~[OWNERS](https://github.com/jetstack/kube-oidc-proxy/blob/master/OWNERS)~~ [JoshVanL] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
JoshVanL commented 4 years ago

Should be good to have a look now /unassign /assign @simonswine /hold cancel /test pull-kube-oidc-proxy-e2e-v1-14 /test pull-kube-oidc-proxy-e2e-v1-13 /test pull-kube-oidc-proxy-e2e-v1-12 /test pull-kube-oidc-proxy-e2e-v1-11

JoshVanL commented 4 years ago

/retest

JoshVanL commented 4 years ago

@simonswine I have updated the docs to make it clear it will terminate the proxy on watch change and mentioned the HPA issue. I think the CLI flag is probably clear enough the exit behavior.

Comma-separated list of files to watch for changes. If a change is detected then send self a SIGHUP signal and exit gracefully.

I've updated the tests and will re-assign once they've passed.

jetstack-bot commented 4 years ago

@JoshVanL: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kube-oidc-proxy-e2e-v1-16 e0d6242ab4b037a2ac3b43f56f764ad254833ec1 link /test pull-kube-oidc-proxy-e2e-v1-16
pull-kube-oidc-proxy-e2e-v1-15 e0d6242ab4b037a2ac3b43f56f764ad254833ec1 link /test pull-kube-oidc-proxy-e2e-v1-15

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
jetstack-bot commented 4 years ago

@JoshVanL: PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
JoshVanL commented 4 years ago

This functionality should be checked whether this has been included, and if not, enabled using apiserver module

/close

jetstack-bot commented 4 years ago

@JoshVanL: Closed this PR.

In response to [this](https://github.com/jetstack/kube-oidc-proxy/pull/114#issuecomment-596701246): >This functionality should be checked whether this has been included, and if not, enabled using apiserver module > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
jescarri commented 4 years ago

What's the status on this PR? is this a feature that's not going to be included in future releases?

Thanks!

JoshVanL commented 4 years ago

Hey @jescarri,

In theory, now that we have updated dependencies on k8s to 1.18, this should be included as default. I haven't tested this however, so if it isn't the case than I will do some investigating to make sure it's there.