fix review comments
Signed-off-by: Jan Larwig <jan@larwig.com> Signed-off-by: Adel Salakh <adel@zetico.io>
This commit is contained in:
Adel Salakh
Jan Larwig
parent
225ace3a2c
commit
20bb8e8b19
|
|
@ -14,6 +14,7 @@
|
|||
- [#3304](https://github.com/oauth2-proxy/oauth2-proxy/pull/3304) fix: added conditional so default is not always set and env vars are honored fixes 3303 (@pixeldrew)
|
||||
- [#3264](https://github.com/oauth2-proxy/oauth2-proxy/pull/3264) fix: more aggressively truncate logged access_token (@MartinNowak / @tuunit)
|
||||
- [#3267](https://github.com/oauth2-proxy/oauth2-proxy/pull/3267) fix: Session refresh handling in OIDC provider (@gysel)
|
||||
- [#3068](https://github.com/oauth2-proxy/oauth2-proxy/pull/3068) feat: graceful shutdown to prevent errors when oauth2-proxy is load balanced (@adelsz)
|
||||
|
||||
# V7.13.0
|
||||
|
||||
|
|
|
|||
|
|
@ -495,6 +495,7 @@ Server represents the configuration for an HTTP(S) server
|
|||
| `bindAddress` | _string_ | BindAddress is the address on which to serve traffic.<br/>Leave blank or set to "-" to disable. |
|
||||
| `secureBindAddress` | _string_ | SecureBindAddress is the address on which to serve secure traffic.<br/>Leave blank or set to "-" to disable. |
|
||||
| `tls` | _[TLS](#tls)_ | TLS contains the information for loading the certificate and key for the<br/>secure traffic and further configuration for the TLS server. |
|
||||
| `shutdownDuration` | _duration_ | Duration of time to continue serving traffic after receiving an exit signal.<br/>During this time the readiness endpoint will be returning HTTP 503 errors.<br/>Leave blank to disable. |
|
||||
|
||||
### TLS
|
||||
|
||||
|
|
|
|||
|
|
@ -235,6 +235,8 @@ Provider specific options can be found on their respective subpages.
|
|||
| flag: `--tls-key-file`<br/>toml: `tls_key_file` | string | path to private key file | |
|
||||
| flag: `--tls-cipher-suite`<br/>toml: `tls_cipher_suites` | string \| list | Restricts TLS cipher suites used by server to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times). If not specified, the default Go safe cipher list is used. List of valid cipher suites can be found in the [crypto/tls documentation](https://pkg.go.dev/crypto/tls#pkg-constants). | |
|
||||
| flag: `--tls-min-version`<br/>toml: `tls_min_version` | string | minimum TLS version that is acceptable, either `"TLS1.2"` or `"TLS1.3"` | `"TLS1.2"` |
|
||||
| flag: `--shutdown-duration`<br/>toml: `shutdown_duration` | duration | Duration of time to continue serving traffic after receiving an exit signal. During this duration the readiness endpoint will be returning HTTP 503 errors. | `"0s"` |
|
||||
|
||||
|
||||
### Session Options
|
||||
|
||||
|
|
|
|||
|
|
@ -271,7 +271,7 @@ func (p *OAuthProxy) setupServer(opts *options.Options) error {
|
|||
BindAddress: opts.Server.BindAddress,
|
||||
SecureBindAddress: opts.Server.SecureBindAddress,
|
||||
TLS: opts.Server.TLS,
|
||||
ShutdownDuration: opts.ShutdownDuration,
|
||||
ShutdownDuration: opts.Server.ShutdownDuration,
|
||||
}
|
||||
|
||||
// Option: AllowQuerySemicolons
|
||||
|
|
|
|||
|
|
@ -471,16 +471,17 @@ func getXAuthRequestAccessTokenHeader() Header {
|
|||
}
|
||||
|
||||
type LegacyServer struct {
|
||||
MetricsAddress string `flag:"metrics-address" cfg:"metrics_address"`
|
||||
MetricsSecureAddress string `flag:"metrics-secure-address" cfg:"metrics_secure_address"`
|
||||
MetricsTLSCertFile string `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"`
|
||||
MetricsTLSKeyFile string `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"`
|
||||
HTTPAddress string `flag:"http-address" cfg:"http_address"`
|
||||
HTTPSAddress string `flag:"https-address" cfg:"https_address"`
|
||||
TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file"`
|
||||
TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file"`
|
||||
TLSMinVersion string `flag:"tls-min-version" cfg:"tls_min_version"`
|
||||
TLSCipherSuites []string `flag:"tls-cipher-suite" cfg:"tls_cipher_suites"`
|
||||
MetricsAddress string `flag:"metrics-address" cfg:"metrics_address"`
|
||||
MetricsSecureAddress string `flag:"metrics-secure-address" cfg:"metrics_secure_address"`
|
||||
MetricsTLSCertFile string `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"`
|
||||
MetricsTLSKeyFile string `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"`
|
||||
HTTPAddress string `flag:"http-address" cfg:"http_address"`
|
||||
HTTPSAddress string `flag:"https-address" cfg:"https_address"`
|
||||
TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file"`
|
||||
TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file"`
|
||||
TLSMinVersion string `flag:"tls-min-version" cfg:"tls_min_version"`
|
||||
TLSCipherSuites []string `flag:"tls-cipher-suite" cfg:"tls_cipher_suites"`
|
||||
ShutdownDuration time.Duration `flag:"shutdown-duration" cfg:"shutdown_duration"`
|
||||
}
|
||||
|
||||
func legacyServerFlagset() *pflag.FlagSet {
|
||||
|
|
@ -496,6 +497,7 @@ func legacyServerFlagset() *pflag.FlagSet {
|
|||
flagSet.String("tls-key-file", "", "path to private key file")
|
||||
flagSet.String("tls-min-version", "", "minimal TLS version for HTTPS clients (either \"TLS1.2\" or \"TLS1.3\")")
|
||||
flagSet.StringSlice("tls-cipher-suite", []string{}, "restricts TLS cipher suites to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times)")
|
||||
flagSet.Duration("shutdown-duration", 0, "Amount of time to continue serving traffic after receiving an exit signal with readiness endpoint set to false.")
|
||||
|
||||
return flagSet
|
||||
}
|
||||
|
|
@ -650,6 +652,7 @@ func (l LegacyServer) convert() (Server, Server) {
|
|||
appServer := Server{
|
||||
BindAddress: l.HTTPAddress,
|
||||
SecureBindAddress: l.HTTPSAddress,
|
||||
ShutdownDuration: l.ShutdownDuration,
|
||||
}
|
||||
if l.TLSKeyFile != "" || l.TLSCertFile != "" {
|
||||
appServer.TLS = &TLS{
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ package options
|
|||
import (
|
||||
"crypto"
|
||||
"net/url"
|
||||
"time"
|
||||
|
||||
ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"
|
||||
internaloidc "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/providers/oidc"
|
||||
|
|
@ -30,8 +29,6 @@ type Options struct {
|
|||
RawRedirectURL string `flag:"redirect-url" cfg:"redirect_url"`
|
||||
RelativeRedirectURL bool `flag:"relative-redirect-url" cfg:"relative_redirect_url"`
|
||||
|
||||
ShutdownDuration time.Duration `flag:"shutdown-duration" cfg:"shutdown_duration"`
|
||||
|
||||
AuthenticatedEmailsFile string `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"`
|
||||
EmailDomains []string `flag:"email-domain" cfg:"email_domains"`
|
||||
WhitelistDomains []string `flag:"whitelist-domain" cfg:"whitelist_domains"`
|
||||
|
|
@ -164,7 +161,6 @@ func NewFlagSet() *pflag.FlagSet {
|
|||
flagSet.Int("redis-connection-idle-timeout", 0, "Redis connection idle timeout seconds, if Redis timeout option is non-zero, the --redis-connection-idle-timeout must be less then Redis timeout option")
|
||||
flagSet.String("signature-key", "", "GAP-Signature request signature key (algorithm:secretkey)")
|
||||
flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints")
|
||||
flagSet.Duration("shutdown-duration", 0, "Amount of time to continue serving traffic after receiving an exit signal with readiness endpoint set to false.")
|
||||
|
||||
flagSet.AddFlagSet(cookieFlagSet())
|
||||
flagSet.AddFlagSet(loggingFlagSet())
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
package options
|
||||
|
||||
import "time"
|
||||
|
||||
// Server represents the configuration for an HTTP(S) server
|
||||
type Server struct {
|
||||
// BindAddress is the address on which to serve traffic.
|
||||
|
|
@ -13,6 +15,11 @@ type Server struct {
|
|||
// TLS contains the information for loading the certificate and key for the
|
||||
// secure traffic and further configuration for the TLS server.
|
||||
TLS *TLS `yaml:"tls,omitempty"`
|
||||
|
||||
// Duration of time to continue serving traffic after receiving an exit signal.
|
||||
// During this time the readiness endpoint will be returning HTTP 503 errors.
|
||||
// Leave blank to disable.
|
||||
ShutdownDuration time.Duration `yaml:"shutdownDuration,omitempty"`
|
||||
}
|
||||
|
||||
// TLS contains the information for loading a TLS certificate and key
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ func NewReadynessCheck(ctx context.Context, path string, verifiable Verifiable)
|
|||
|
||||
func readynessCheck(ctx context.Context, path string, verifiable Verifiable, next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
|
||||
// Check the server context (not request).
|
||||
// Has the context been canceled because of SIGTERM?
|
||||
if ctx.Err() != nil {
|
||||
rw.WriteHeader(http.StatusServiceUnavailable)
|
||||
fmt.Fprintf(rw, "Shutting down")
|
||||
|
|
|
|||
|
|
@ -222,9 +222,7 @@ func (s *server) startServer(ctx context.Context, listener net.Listener) error {
|
|||
g.Go(func() error {
|
||||
<-groupCtx.Done()
|
||||
logger.Printf("Context canceled. Waiting %s before shutting down the listeners.", s.shutdownDuration)
|
||||
|
||||
time.Sleep(s.shutdownDuration)
|
||||
|
||||
logger.Printf("Shutting down listener.")
|
||||
|
||||
if err := srv.Shutdown(context.Background()); err != nil {
|
||||
|
|
|
|||
Loading…
Reference in New Issue