From 2399be867e21dd49958899aa39aec749da024dc3 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sat, 4 Mar 2017 16:46:45 -0300 Subject: [PATCH 1/4] Cleanup custom log format configuration --- controllers/nginx/pkg/config/config.go | 28 ++++++++----------- controllers/nginx/pkg/config/config_test.go | 16 +++++------ controllers/nginx/pkg/template/template.go | 7 ++--- .../rootfs/etc/nginx/template/nginx.tmpl | 2 +- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index c095adcb16..9bc9f508d1 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -17,11 +17,11 @@ limitations under the License. package config import ( + "fmt" "runtime" "github.com/golang/glog" - "fmt" "k8s.io/ingress/core/pkg/ingress" "k8s.io/ingress/core/pkg/ingress/defaults" ) @@ -47,9 +47,9 @@ const ( gzipTypes = "application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/plain text/x-component" - logFormatUpstream = "'%v - [$proxy_add_x_forwarded_for] - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\" $request_length $request_time [$proxy_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status'" + logFormatUpstream = `[$proxy_add_x_forwarded_for] - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status"` - logFormatStream = "'$remote_addr [$time_local] $protocol [$ssl_preread_server_name] [$stream_upstream] $status $bytes_sent $bytes_received $session_time'" + logFormatStream = `[$time_local] $protocol [$ssl_preread_server_name] [$stream_upstream] $status $bytes_sent $bytes_received $session_time` // http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_buffer_size // Sets the size of the buffer used for sending data. @@ -264,7 +264,7 @@ func NewDefault() Configuration { KeepAlive: 75, LargeClientHeaderBuffers: "4 8k", LogFormatStream: logFormatStream, - LogFormatUpstream: BuildLogFormatUpstream(false, ""), + LogFormatUpstream: logFormatUpstream, MaxWorkerConnections: 16384, MapHashBucketSize: 64, ProxyRealIPCIDR: defIPCIDR, @@ -307,20 +307,14 @@ func NewDefault() Configuration { return cfg } -// BuildLogFormatUpstream format the log_format upstream based on proxy_protocol -func BuildLogFormatUpstream(useProxyProtocol bool, curLogFormatUpstream string) string { - - // test if log_format comes from configmap - if curLogFormatUpstream != "" && - curLogFormatUpstream != fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr") && - curLogFormatUpstream != fmt.Sprintf(logFormatUpstream, "$remote_addr") { - return curLogFormatUpstream - } - - if useProxyProtocol { - return fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr") +// BuildLogFormatUpstream format the log_format upstream using +// proxy_protocol_addr as remote client address if UseProxyProtocol +// is enabled. +func (cfg Configuration) BuildLogFormatUpstream() string { + if cfg.UseProxyProtocol { + return fmt.Sprintf("$proxy_protocol_addr - %s", cfg.LogFormatUpstream) } - return fmt.Sprintf(logFormatUpstream, "$remote_addr") + return fmt.Sprintf("$remote_addr - %s", cfg.LogFormatUpstream) } // TemplateConfig contains the nginx configuration to render the file nginx.conf diff --git a/controllers/nginx/pkg/config/config_test.go b/controllers/nginx/pkg/config/config_test.go index b198b8adba..28bbf271a3 100644 --- a/controllers/nginx/pkg/config/config_test.go +++ b/controllers/nginx/pkg/config/config_test.go @@ -12,19 +12,19 @@ func TestBuildLogFormatUpstream(t *testing.T) { curLogFormat string expected string }{ - {true, "", fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr")}, - {false, "", fmt.Sprintf(logFormatUpstream, "$remote_addr")}, - {true, "my-log-format", "my-log-format"}, - {false, "john-log-format", "john-log-format"}, + {true, logFormatUpstream, fmt.Sprintf("$proxy_protocol_addr - %s", logFormatUpstream)}, + {false, logFormatUpstream, fmt.Sprintf("$remote_addr - %s", logFormatUpstream)}, + {true, "my-log-format", "$proxy_protocol_addr - my-log-format"}, + {false, "john-log-format", "$remote_addr - john-log-format"}, } for _, testCase := range testCases { - - result := BuildLogFormatUpstream(testCase.useProxyProtocol, testCase.curLogFormat) - + cfg := NewDefault() + cfg.UseProxyProtocol = testCase.useProxyProtocol + cfg.LogFormatUpstream = testCase.curLogFormat + result := cfg.BuildLogFormatUpstream() if result != testCase.expected { t.Errorf(" expected %v but return %v", testCase.expected, result) } - } } diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index c366943633..8262a58735 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -31,7 +31,6 @@ import ( "github.com/golang/glog" "k8s.io/ingress/controllers/nginx/pkg/config" - nginxconfig "k8s.io/ingress/controllers/nginx/pkg/config" "k8s.io/ingress/core/pkg/ingress" ing_net "k8s.io/ingress/core/pkg/net" "k8s.io/ingress/core/pkg/watch" @@ -229,14 +228,12 @@ func buildAuthLocation(input interface{}) string { } func buildLogFormatUpstream(input interface{}) string { - config, ok := input.(config.Configuration) - + cfg, ok := input.(config.Configuration) if !ok { glog.Errorf("error an ingress.buildLogFormatUpstream type but %T was returned", input) } - return nginxconfig.BuildLogFormatUpstream(config.UseProxyProtocol, config.LogFormatUpstream) - + return cfg.BuildLogFormatUpstream() } // buildProxyPass produces the proxy pass string, if the ingress has redirects diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index de140e9aef..e0a1194086 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -79,7 +79,7 @@ http { server_tokens {{ if $cfg.ShowServerTokens }}on{{ else }}off{{ end }}; - log_format upstreaminfo {{ buildLogFormatUpstream $cfg }}; + log_format upstreaminfo '{{ buildLogFormatUpstream $cfg }}'; {{/* map urls that should not appear in access.log */}} {{/* http://nginx.org/en/docs/http/ngx_http_log_module.html#access_log */}} From 3c0fb01ba2022ea1f5fed3e2854ce90824406c61 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sat, 4 Mar 2017 16:50:49 -0300 Subject: [PATCH 2/4] Add warning when the ingress controller uses a custom class --- controllers/nginx/pkg/cmd/controller/nginx.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index f51e01158a..3c798bb1bb 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/controllers/nginx/pkg/cmd/controller/nginx.go @@ -260,6 +260,10 @@ func (n NGINXController) Info() *ingress.BackendInfo { // OverrideFlags customize NGINX controller flags func (n NGINXController) OverrideFlags(flags *pflag.FlagSet) { + ig, err := flags.GetString("ingress-class") + if err == nil && ig != "" && ig != defIngressClass { + glog.Warningf("only Ingress with class %v will be processed by this ingress controller", ig) + } flags.Set("ingress-class", defIngressClass) } From 1473f64fb007c43d562328e6c7809f82d3eb931d Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sat, 4 Mar 2017 17:15:21 -0300 Subject: [PATCH 3/4] Remove SPDY reference --- controllers/nginx/pkg/config/config.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index 9bc9f508d1..518191681d 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -97,11 +97,6 @@ type Configuration struct { //http://nginx.org/en/docs/http/ngx_http_log_module.html DisableAccessLog bool `json:"disable-access-log,omitempty"` - // EnableSPDY enables spdy and use ALPN and NPN to advertise the availability of the two protocols - // https://blog.cloudflare.com/open-sourcing-our-nginx-http-2-spdy-code - // By default this is enabled - EnableSPDY bool `json:"enable-spdy"` - // EnableStickySessions enabled sticky sessions using cookies // https://bitbucket.org/nginx-goodies/nginx-sticky-module-ng // By default this is disabled @@ -255,7 +250,6 @@ func NewDefault() Configuration { ClientHeaderBufferSize: "1k", DisableAccessLog: false, EnableDynamicTLSRecords: true, - EnableSPDY: false, ErrorLogLevel: errorLevel, HSTS: true, HSTSIncludeSubdomains: true, From cd924f552270da4efc687deb193052747f3563ab Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sat, 4 Mar 2017 17:21:19 -0300 Subject: [PATCH 4/4] Avoid duplication of ReadConfig function --- controllers/nginx/pkg/cmd/controller/nginx.go | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index 3c798bb1bb..9d12f3b089 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/controllers/nginx/pkg/cmd/controller/nginx.go @@ -29,7 +29,6 @@ import ( "time" "github.com/golang/glog" - "github.com/mitchellh/mapstructure" "github.com/spf13/pflag" "k8s.io/kubernetes/pkg/api" @@ -186,26 +185,7 @@ func (n NGINXController) BackendDefaults() defaults.Backend { return d.Backend } - return n.backendDefaults() -} - -func (n *NGINXController) backendDefaults() defaults.Backend { - d := config.NewDefault() - config := &mapstructure.DecoderConfig{ - Metadata: nil, - WeaklyTypedInput: true, - Result: &d, - TagName: "json", - } - decoder, err := mapstructure.NewDecoder(config) - if err != nil { - glog.Warningf("unexpected error merging defaults: %v", err) - } - err = decoder.Decode(n.configmap.Data) - if err != nil { - glog.Warningf("unexpected error decoding: %v", err) - } - return d.Backend + return ngx_template.ReadConfig(n.configmap.Data).Backend } // isReloadRequired check if the new configuration file is different