Skip to content

Commit

Permalink
Manual cherry-pick of openshift#73
Browse files Browse the repository at this point in the history
Bug 1924258: fluentd fails to deliver message with Server returned nothing
  • Loading branch information
syedriko committed Feb 2, 2021
1 parent 37116ff commit 553bf90
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 27 deletions.
13 changes: 13 additions & 0 deletions pkg/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,18 @@ func newFlagSet() *flag.FlagSet {
flagSet.String("auth-admin-role", "", "The name of the only role that will be passed on the request if it is found in the list of roles")
flagSet.String("auth-default-role", "", "The role given to every request unless it has the auth-admin-role")

//net/http.Server timeouts for the server side of the proxy
flagSet.Duration("http-read-timeout", time.Duration(1)*time.Minute, "The maximum duration for reading the entire HTTP request. Zero means no timeout.")
flagSet.Duration("http-write-timeout", time.Duration(1)*time.Minute, "The maximum duration before timing out writes of the response. Zero means no timeout")
flagSet.Duration("http-idle-timeout", time.Duration(1)*time.Minute, "The maximum amount of time to wait for the next request. Zero means no timeout.")

//net/http.Transport limits and timeouts
flagSet.Int("http-max-conns-per-host", 25, "The total number of connections per host. Zero means no limit.")
flagSet.Int("http-max-idle-conns", 25, "The maximum number of idle (keep-alive) connections across all hosts. Zero means no limit.")
flagSet.Int("http-max-idle-conns-per-host", 25, "The maximum number of idle (keep-alive) connections per host. Zero means no limit.")
flagSet.Duration("http-idle-conn-timeout", time.Duration(1)*time.Minute, "The maximum amount of time to wait for the next request. Zero means no timeout.")
flagSet.Duration("http-tls-handshake-timeout", time.Duration(10)*time.Second, "The maximum amount of time to wait for a TLS handshake. Zero means no timeout.")
flagSet.Duration("http-expect-continue-timeout", time.Duration(1)*time.Second, "The amount of time to wait for a server's first response headers if the request has an \"Expect: 100-continue\" header. Zero means no timeout.")

return flagSet
}
67 changes: 58 additions & 9 deletions pkg/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
log "github.com/sirupsen/logrus"
)

//Options are that can be set by Command Line Flag, or Config File
//Options that can be set by Command Line Flag, or Config File
type Options struct {
ProxyWebSockets bool `flag:"proxy-websockets"`
ListeningAddress string `flag:"listening-address"`
Expand Down Expand Up @@ -56,6 +56,19 @@ type Options struct {

//OCP Cluster Logging configs
cltypes.ExtConfig

//net/http.Server timeouts for the server side of the proxy
HTTPReadTimeout time.Duration `flag:"http-read-timeout"`
HTTPWriteTimeout time.Duration `flag:"http-write-timeout"`
HTTPIdleTimeout time.Duration `flag:"http-idle-timeout"`

//net/http.Transport limits and timeouts
HTTPMaxConnsPerHost int `flag:"http-max-conns-per-host"`
HTTPMaxIdleConns int `flag:"http-max-idle-conns"`
HTTPMaxIdleConnsPerHost int `flag:"http-max-idle-conns-per-host"`
HTTPIdleConnTimeout time.Duration `flag:"http-idle-conn-timeout"`
HTTPTLSHandshakeTimeout time.Duration `flag:"http-tls-handshake-timeout"`
HTTPExpectContinueTimeout time.Duration `flag:"http-expect-continue-timeout"`
}

//Init the configuration options based on the values passed via the CLI
Expand Down Expand Up @@ -83,14 +96,23 @@ func Init(args []string) (*Options, error) {

func newOptions() *Options {
return &Options{
ProxyWebSockets: true,
ListeningAddress: ":443",
Elasticsearch: "https://localhost:9200",
UpstreamFlush: time.Duration(5) * time.Millisecond,
RequestLogging: false,
AuthBackEndRoles: map[string]BackendRoleConfig{},
AuthWhiteListedNames: []string{},
AuthAdminRole: "",
ProxyWebSockets: true,
ListeningAddress: ":443",
Elasticsearch: "https://localhost:9200",
UpstreamFlush: time.Duration(5) * time.Millisecond,
RequestLogging: false,
AuthBackEndRoles: map[string]BackendRoleConfig{},
AuthWhiteListedNames: []string{},
AuthAdminRole: "",
HTTPReadTimeout: time.Duration(1) * time.Minute,
HTTPWriteTimeout: time.Duration(1) * time.Minute,
HTTPIdleTimeout: time.Duration(1) * time.Minute,
HTTPMaxConnsPerHost: 25,
HTTPMaxIdleConns: 25,
HTTPMaxIdleConnsPerHost: 25,
HTTPIdleConnTimeout: time.Duration(1) * time.Minute,
HTTPTLSHandshakeTimeout: time.Duration(10) * time.Second,
HTTPExpectContinueTimeout: time.Duration(1) * time.Second,
}
}

Expand Down Expand Up @@ -144,7 +166,34 @@ func (o *Options) Validate() error {
}
o.AuthBackEndRoles[name] = *roleConfig
}
}

if o.HTTPReadTimeout < 0 {
msgs = append(msgs, "http-read-timeout can not be negative")
}
if o.HTTPWriteTimeout < 0 {
msgs = append(msgs, "http-write-timeout can not be negative")
}
if o.HTTPIdleTimeout < 0 {
msgs = append(msgs, "http-idle-timeout can not be negative")
}
if o.HTTPMaxConnsPerHost < 0 {
msgs = append(msgs, "http-max-conns-per-host can not be negative")
}
if o.HTTPMaxIdleConns < 0 {
msgs = append(msgs, "http-max-idle-conns can not be negative")
}
if o.HTTPMaxIdleConnsPerHost < 0 {
msgs = append(msgs, "http-max-idle-conns-per-host can not be negative")
}
if o.HTTPIdleConnTimeout < 0 {
msgs = append(msgs, "http-idle-conn-timeout can not be negative")
}
if o.HTTPTLSHandshakeTimeout < 0 {
msgs = append(msgs, "http-tls-handshake-timeout can not be negative")
}
if o.HTTPExpectContinueTimeout < 0 {
msgs = append(msgs, "http-expect-continue-timeout can not be negative")
}

//Cluster Logging Handler Validations
Expand Down
90 changes: 90 additions & 0 deletions pkg/config/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config_test
import (
"net/url"
"strings"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -159,4 +160,93 @@ var _ = Describe("Initializing Config options", func() {
})
})

// HTTPMaxIdleConnsPerHost
Describe("when defining HTTP transport max idle connections per host", func() {
Describe("to be non-negative", func() {
It("should succeed", func() {
args := []string{"--http-max-idle-conns-per-host=1"}
options, err := config.Init(args)
Expect(err).Should(BeNil())
Expect(options).Should(Not(BeNil()))
Expect(options.HTTPMaxIdleConnsPerHost).Should(Equal(1))
})
})
Describe("to be negative", func() {
It("should fail", func() {
args := []string{"--http-max-idle-conns-per-host=-1"}
options, err := config.Init(args)
Expect(options).Should(BeNil())
Expect(err.Error()).Should(
Equal(errorMessage("http-max-idle-conns-per-host can not be negative")))
})
})
})

// HTTPIdleConnTimeout
Describe("when defining negative HTTP transport idle connection timeout", func() {
Describe("to be non-negative", func() {
It("should succeed", func() {
args := []string{"--http-idle-conn-timeout=2m"}
options, err := config.Init(args)
Expect(err).Should(BeNil())
Expect(options).Should(Not(BeNil()))
Expect(options.HTTPIdleConnTimeout).Should(Equal(2 * time.Minute))
})
})
Describe("to be negative", func() {
It("should fail", func() {
args := []string{"--http-idle-conn-timeout=-2m"}
options, err := config.Init(args)
Expect(options).Should(BeNil())
Expect(err.Error()).Should(
Equal(errorMessage("http-idle-conn-timeout can not be negative")))
})
})
})

// HTTPTLSHandshakeTimeout
Describe("when defining HTTP transport TLS handshake timeout", func() {
Describe("to be non-negative", func() {
It("should succeed", func() {
args := []string{"--http-tls-handshake-timeout=3h"}
options, err := config.Init(args)
Expect(err).Should(BeNil())
Expect(options).Should(Not(BeNil()))
Expect(options.HTTPTLSHandshakeTimeout).Should(Equal(3 * time.Hour))
})
})
Describe("to be negative", func() {
It("should fail", func() {
args := []string{"--http-tls-handshake-timeout=-3h"}
options, err := config.Init(args)
Expect(options).Should(BeNil())
Expect(err.Error()).Should(
Equal(errorMessage("http-tls-handshake-timeout can not be negative")))
})
})
})

// HTTPExpectContinueTimeout
Describe("when defining HTTP transport expect continue timeout", func() {
Describe("to be non-negative", func() {
It("should succeed", func() {
args := []string{"--http-expect-continue-timeout=1h2m3s4ms5us6ns"}
options, err := config.Init(args)
Expect(err).Should(BeNil())
Expect(options).Should(Not(BeNil()))
Expect(options.HTTPExpectContinueTimeout).Should(Equal(
1*time.Hour + 2*time.Minute + 3*time.Second + 4*time.Millisecond +
5*time.Microsecond + 6*time.Nanosecond))
})
})
Describe("to be negative", func() {
It("should fail", func() {
args := []string{"--http-expect-continue-timeout=-1h2m3s4ms5us6ns"}
options, err := config.Init(args)
Expect(options).Should(BeNil())
Expect(err.Error()).Should(
Equal(errorMessage("http-expect-continue-timeout can not be negative")))
})
})
})
})
7 changes: 3 additions & 4 deletions pkg/proxy/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package proxy
import (
"crypto/tls"
"net/http"
"time"

log "github.com/sirupsen/logrus"

Expand Down Expand Up @@ -51,9 +50,9 @@ func (s *Server) ListenAndServe() {
srv := &http.Server{
Addr: addr,
Handler: s.Handler,
ReadTimeout: 5 * time.Second,
WriteTimeout: 5 * time.Second,
IdleTimeout: 60 * time.Second,
ReadTimeout: s.Opts.HTTPReadTimeout,
WriteTimeout: s.Opts.HTTPWriteTimeout,
IdleTimeout: s.Opts.HTTPIdleTimeout,
TLSConfig: cfg,
}
srv.SetKeepAlivesEnabled(true)
Expand Down
26 changes: 12 additions & 14 deletions pkg/proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"net/http/pprof"
"net/url"
"strings"
"time"

"golang.org/x/net/http2"

Expand Down Expand Up @@ -46,23 +45,22 @@ func (u *UpstreamProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
} else {
u.handler.ServeHTTP(w, r)
}

}

func NewReverseProxy(target *url.URL, upstreamFlush time.Duration, rootCAs []string) (*httputil.ReverseProxy, error) {
func NewReverseProxy(target *url.URL, opts *configOptions.Options) (*httputil.ReverseProxy, error) {
proxy := httputil.NewSingleHostReverseProxy(target)
proxy.FlushInterval = upstreamFlush
proxy.FlushInterval = opts.UpstreamFlush

transport := &http.Transport{
MaxConnsPerHost: 25,
MaxIdleConns: 25,
MaxIdleConnsPerHost: 25,
IdleConnTimeout: 60 * time.Second,
TLSHandshakeTimeout: 10 * time.Second,
ExpectContinueTimeout: 1 * time.Second,
MaxConnsPerHost: opts.HTTPMaxConnsPerHost,
MaxIdleConns: opts.HTTPMaxIdleConns,
MaxIdleConnsPerHost: opts.HTTPMaxIdleConnsPerHost,
IdleConnTimeout: opts.HTTPIdleConnTimeout,
TLSHandshakeTimeout: opts.HTTPTLSHandshakeTimeout,
ExpectContinueTimeout: opts.HTTPExpectContinueTimeout,
}
if len(rootCAs) > 0 {
pool, err := util.GetCertPool(rootCAs, false)
if len(opts.UpstreamCAs) > 0 {
pool, err := util.GetCertPool(opts.UpstreamCAs, false)
if err != nil {
return nil, err
}
Expand All @@ -71,7 +69,7 @@ func NewReverseProxy(target *url.URL, upstreamFlush time.Duration, rootCAs []str
}
}
if err := http2.ConfigureTransport(transport); err != nil {
if len(rootCAs) > 0 {
if len(opts.UpstreamCAs) > 0 {
return nil, err
}
log.Warnf("Failed to configure http2 transport: %v", err)
Expand All @@ -94,7 +92,7 @@ func setProxyUpstreamHostHeader(proxy *httputil.ReverseProxy, target *url.URL) {

func NewWebSocketOrRestReverseProxy(u *url.URL, opts *configOptions.Options) (restProxy http.Handler) {
u.Path = ""
proxy, err := NewReverseProxy(u, opts.UpstreamFlush, opts.UpstreamCAs)
proxy, err := NewReverseProxy(u, opts)
if err != nil {
log.Fatal("Failed to initialize Reverse Proxy: ", err)
}
Expand Down

0 comments on commit 553bf90

Please sign in to comment.