Merge pull request #373 from aledbf/log-format

Cleanup

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()
+	return ngx_template.ReadConfig(n.configmap.Data).Backend
-}
-
-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
 }
 
 // isReloadRequired check if the new configuration file is different
@@ -260,6 +240,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)
 }
 

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.
@@ -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,
@@ -264,7 +258,7 @@ func NewDefault() Configuration {
 		KeepAlive:                75,
 		LargeClientHeaderBuffers: "4 8k",
 		LogFormatStream:          logFormatStream,
-		LogFormatUpstream:        BuildLogFormatUpstream(false, ""),
+		LogFormatUpstream:        logFormatUpstream,
 		MaxWorkerConnections:     16384,
 		MapHashBucketSize:        64,
 		ProxyRealIPCIDR:          defIPCIDR,
@@ -307,20 +301,14 @@ func NewDefault() Configuration {
 	return cfg
 }
 
-// BuildLogFormatUpstream format the log_format upstream based on proxy_protocol
+// BuildLogFormatUpstream format the log_format upstream using
-func BuildLogFormatUpstream(useProxyProtocol bool, curLogFormatUpstream string) string {
+// proxy_protocol_addr as remote client address if UseProxyProtocol
-
+// is enabled.
-	// test if log_format comes from configmap
+func (cfg Configuration) BuildLogFormatUpstream() string {
-	if curLogFormatUpstream != "" &&
+	if cfg.UseProxyProtocol {
-		curLogFormatUpstream != fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr") &&
+		return fmt.Sprintf("$proxy_protocol_addr - %s", cfg.LogFormatUpstream)
-		curLogFormatUpstream != fmt.Sprintf(logFormatUpstream, "$remote_addr") {
-		return curLogFormatUpstream
 	}
-
+	return fmt.Sprintf("$remote_addr - %s", cfg.LogFormatUpstream)
-	if useProxyProtocol {
-		return fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr")
-	}
-	return fmt.Sprintf(logFormatUpstream, "$remote_addr")
 }
 
 // TemplateConfig contains the nginx configuration to render the file nginx.conf

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")},
+		{true, logFormatUpstream, fmt.Sprintf("$proxy_protocol_addr - %s", logFormatUpstream)},
-		{false, "", fmt.Sprintf(logFormatUpstream, "$remote_addr")},
+		{false, logFormatUpstream, fmt.Sprintf("$remote_addr - %s", logFormatUpstream)},
-		{true, "my-log-format", "my-log-format"},
+		{true, "my-log-format", "$proxy_protocol_addr - my-log-format"},
-		{false, "john-log-format", "john-log-format"},
+		{false, "john-log-format", "$remote_addr - john-log-format"},
 	}
 
 	for _, testCase := range testCases {
-
+		cfg := NewDefault()
-		result := BuildLogFormatUpstream(testCase.useProxyProtocol, testCase.curLogFormat)
+		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)
 		}
-
 	}
 }

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

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 */}}