Properly Handle io.Copy Errors (#85)

* fix for issue 83 * config flag bug fix * updating docs * updating docs * updating grafana dashboards
northwesternmutual · Nov 13, 2017 · afa4534 · afa4534
1 parent 3d5ef4e
commit afa4534
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 166 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,10 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
+## [1.2.3] - 2017-11-12
 ### Changed
-- Allow for batching of InfluxDB writes
-- Fixed [#83](https://github.com/northwesternmutual/kanali/issues/83)
+- Allow for batching of InfluxDB writes.
+- Fixed [#83](https://github.com/northwesternmutual/kanali/issues/83).
+- Fixed bug that did not properly handle all config value types.
+- No longer indexing `http_method` InfluxDB field as it is not being utilized as a tag.
 
 ## [1.2.2] - 2017-11-02
 ### Changed

diff --git a/README.md b/README.md
@@ -36,7 +36,7 @@ Kanali is an extremely efficient [Kubernetes](https://kubernetes.io/) ingress co
 
 ```sh
 $ git clone git@github.com:northwesternmutual/kanali.git && cd kanali
-$ minikube start
+$ minikube start --kubernetes-version v1.7.5
 $ ./scripts/install.sh # wait until all pods are in running state
 $ kubectl apply -f ./examples/exampleOne.yaml
 $ curl $(minikube service kanali --url --format="https://{{.IP}}:{{.Port}}")/api/v1/example-one
@@ -129,13 +129,17 @@ $ kanali [command] [flags]
 ```sh
 start
     --analytics.influx_addr string                InfluxDB address. Address should be of the form 'http://host:port' or 'http://[ipv6-host%zone]:port'. (default "http://monitoring-influxdb.kube-system.svc.cluster.local:8086")
+    --analytics.influx_buffer_size int            InfluxDB buffer size. Request metrics will be written to InfluxDB when this buffer is full. (default 10)
     --analytics.influx_db string                  InfluxDB database name (default "k8s")
+    --analytics.influx_measurement string          InfluxDB measurement to be used for Kanali request metrics. (default "request_details")
+    --analytics.influx_password string            InfluxDB password
+    --analytics.influx_username string            InfluxDB username
     --plugins.apiKey.decryption_key_file string   Path to valid PEM-encoded private key that matches the public key used to encrypt API keys.
     --plugins.location string                     Location of custom plugins shared object (.so) files. (default "/")
     --process.log_level string                    Sets the logging level. Choose between 'debug', 'info', 'warn', 'error', 'fatal'. (default "info")
     --proxy.enable_cluster_ip                     Enables to use of cluster ip as opposed to Kubernetes DNS for upstream routing.
     --proxy.enable_mock_responses                 Enables Kanali's mock responses feature. Read the documentation for more information.
-    --proxy.header_mask_Value string              Sets the Value to be used when omitting header Values. (default "ommitted")
+    --proxy.header_mask_Value string              Sets the Value to be used when omitting header Values. (default "omitted")
     --proxy.mask_header_keys stringSlice          Specify which headers to mask
     --proxy.tls_common_name_validation            Should common name validate as part of an SSL handshake. (default true)
     --proxy.upstream_timeout string               Set length of upstream timeout. Defaults to none (default "0h0m10s")
@@ -145,7 +149,7 @@ start
     --server.proxy_protocol                       Maintain the integrity of the remote client IP address when incoming traffic to Kanali includes the Proxy Protocol header.
     --tls.ca_file string                          Path to x509 certificate authority bundle for mutual TLS.
     --tls.cert_file string                        Path to x509 certificate for HTTPS servers.
-    --tls.key_file string                         Path to x509 private key matching --tls-cert-file.
+    --tls.key_file string                         Path to x509 private key matching --tls.cert_file.
     --tracing.jaeger_agent_url string             Endpoint to the Jaeger agent (default "jaeger-all-in-one-agent.default.svc.cluster.local")
     --tracing.jaeger_server_url string            Endpoint to the Jaeger server (default "jaeger-all-in-one-agent.default.svc.cluster.local")
 

diff --git a/config/flag.go b/config/flag.go
@@ -75,12 +75,13 @@ func (f flags) AddAll(cmd *cobra.Command) error {
 		case []string:
 			cmd.Flags().StringSliceP(currFlag.Long, currFlag.Short, v, currFlag.Usage)
 		default:
-			return nil
+			viper.SetDefault(currFlag.Long, currFlag.Value)
+			continue
 		}
+		viper.SetDefault(currFlag.Long, currFlag.Value)
 		if err := viper.BindPFlag(currFlag.Long, cmd.Flags().Lookup(currFlag.Long)); err != nil {
 			return err
 		}
-		viper.SetDefault(currFlag.Long, currFlag.Value)
 	}
 
 	return nil

diff --git a/controller/watcher_test.go b/controller/watcher_test.go
@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/northwesternmutual/kanali/spec"
 	"github.com/stretchr/testify/assert"
@@ -99,162 +100,14 @@ func TestMonitor(t *testing.T) {
 	eventCh <- updateEvent
 	eventCh <- deleteEvent
 
+	time.Sleep(500 * time.Millisecond)
+
 	assert.Equal(t, "modified test add", testFuncs.getAddResult())
 	assert.Equal(t, "modified test update", testFuncs.getUpdateResult())
 	assert.Equal(t, "modified test delete", testFuncs.getDeleteResult())
 
 }
 
-// func TestMonitor(t *testing.T) {
-//
-// 	assert := assert.New(t)
-//
-// 	testEventCh := make(chan *event)
-// 	monitor(testEventCh)
-// 	testAPIProxy := getTestAPIProxy()
-// 	testAPIKeyBinding := getTestAPIKeyBinding()
-// 	testAPIKey := getTestAPIKey()
-// 	testService := getTestService()
-// 	testSecret := getTestSecret()
-//
-// 	testEventCh <- &event{added, testAPIProxy}
-// 	time.Sleep(100 * time.Millisecond)
-// 	proxy, _ := spec.ProxyStore.Get(testAPIProxy.Spec.Path)
-// 	assert.Equal(testAPIProxy, proxy)
-//
-// 	testAPIProxy.Spec.Target = "/foo"
-// 	testEventCh <- &event{modified, testAPIProxy}
-// 	time.Sleep(100 * time.Millisecond)
-// 	proxy, _ = spec.ProxyStore.Get(testAPIProxy.Spec.Path)
-// 	assert.Equal(testAPIProxy, proxy)
-//
-// 	testEventCh <- &event{deleted, testAPIProxy}
-// 	time.Sleep(100 * time.Millisecond)
-// 	assert.Nil(spec.ProxyStore.Get(testAPIProxy.Spec.Path))
-//
-// 	testEventCh <- &event{added, testAPIKeyBinding}
-// 	time.Sleep(100 * time.Millisecond)
-// 	binding, _ := spec.BindingStore.Get(testAPIKeyBinding.Spec.APIProxyName, testAPIKeyBinding.ObjectMeta.Namespace)
-// 	assert.Equal(testAPIKeyBinding, binding)
-//
-// 	testAPIKeyBinding.Spec.Keys[0].DefaultRule.Global = false
-// 	testEventCh <- &event{modified, testAPIKeyBinding}
-// 	time.Sleep(100 * time.Millisecond)
-// 	binding, _ = spec.BindingStore.Get(testAPIKeyBinding.Spec.APIProxyName, testAPIKeyBinding.ObjectMeta.Namespace)
-// 	assert.Equal(testAPIKeyBinding, binding)
-//
-// 	testEventCh <- &event{deleted, testAPIKeyBinding}
-// 	time.Sleep(100 * time.Millisecond)
-// 	assert.Nil(spec.BindingStore.Get(testAPIKeyBinding.Spec.APIProxyName, testAPIKeyBinding.ObjectMeta.Namespace))
-//
-// 	// setup a private key
-// 	rawPrivateKeyData := []byte(`-----BEGIN RSA PRIVATE KEY-----
-// MIIEowIBAAKCAQEAqYdnX0jeOX0zZuTG0zDJ+t1qzA63MMxYllwcNdSIuDCvT6Rn
-// wSg0nx+PSWYQQqXCN7q8CSsBgp6QNneCxL3A/1JzV7w/fMyWLIuuSOn7Gi8Iz+7E
-// Mb9dbPzejHbx44TDzjIm++xwpyR56e6Zqi8h+XFfNTxQ1IWsiUQJsEvNOus9km92
-// gTZ9hJNX8GgfCvuP0BjDsXGjQVhUu7tLO4eccXvZjnLLYrOM9qtkkEf8eD/1dh5+
-// nvmgCl2QI9YHq+OvlCLtAc2m1txPxtvCur51RjoPUXkmCgljZdqadSKcURu/DXME
-// cLF13smQl6Jq4gGzQY919PC1EjkhGKZA/EFtUwIDAQABAoIBADN6r5RKr1irwTkk
-// jY/CCAOKywxuB4jk9J2sGNDr2hx8hC/eD7ei+t+7GKrEOHnUlfaQWNs72PiOJ+Ky
-// Rd5ydLHTmrzwqCLAiXW7cNApZRvdXoKt0Zv9rWQUIYxr7iYVwdPSfO4RLWBD/lVg
-// I/9+0oVJvQyQZUcz1GHWbE7Bpe+W0vkDeFXxlCP39UWmzfCChhzCFXTTgvl2EHdx
-// QMPnn3dhjf+uBtZZXUpjo6lFNIrWlqnSgE7krJUfzD5TVgG1q8AfF5BdYmAtEoIa
-// nWQrn51++seJQcCh0g4bRWbjk79Qp2uanoeVZpZdRQSaguUnJbOLXkSJAiqAczoD
-// MWqXSWkCgYEA2SpJMMdN30yjTZeHPLb72wum7l+ZZ6Wrv8zt6AWPA3T7i6dBRSKi
-// D6ycCLk6V6SWOEgm2MruYxwvd3lY3XHjJwmfpWqQjYrKp1u6w+B8NKhcdAx2lcn3
-// Uv7rv1A/Et7Q1abzL9e/vteP0o5sYDvfDGxMFt9jgu43S/rpHOWTe10CgYEAx9ha
-// iHHcPfpysZR5kX0eOT8pXpOG6UfrEw38yZqIyCGz4fyZKQ+bttg0WqHMpWe1mw9P
-// pYBq2PtzUuyJVGft9xTP7Ov1oa741cYFpBbYtW4e81CWPBj4cZNBzq1Y0dg4Hw0s
-// aGYBQ9L87koM0elfjNo2/HJfTVc8OWU60EWLsG8CgYBg2BSvphG6JQkmTw7GKqwC
-// MR4Oa5+TszP2YsMtl10Bo6eRzdKzrBAtgUJMOZ4k+4bqLnL0dvr8Q9N/KiRRDLrJ
-// 6+a/89fm5yAcpjGRrIh3SyV/sxcnEVw0LO6g8H5QQgFLZhpJGaOuzZ6bvVvjRo/f
-// kGQWRySvfOA4B/rxIgg1GQKBgCM/VqBwLJ9F2ArYHCT8A2Onbz1+GbJ1e9GtiuNn
-// /S4HO7nlGoJyfU1fjsRZe0XFJ/PEXJDdOHsyxmFe1M3tUrxckFvCNl2hBcR2m7IY
-// UXqWhKD3mrfY06D8jwPL8Tl5wFRBt45mR1zWDsRcjSxM1Ax8xGv8JDD47OdWomvv
-// iDbDAoGBAK/WpzoN59vsaw+PcxS93WgBAnGe9q+mPxNPLj0xaakiNFn7FCdiGV8z
-// 4Wn0sua48v7QcJbKcL0ZbXky61EZV3HqAyzhWZ6jSSQoM37S/nPpq5EPTfs4Dvnl
-// BA7dWeyLFnN7ePVSkL1SES58MpMMiunrUo/Ci6OjiOyN7ynnU6tE
-// -----END RSA PRIVATE KEY-----`)
-//
-// 	block, _ := pem.Decode(rawPrivateKeyData)
-// 	// parse the pem block into a private key
-// 	privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes)
-// 	if err != nil {
-// 		assert.Fail(err.Error())
-// 	}
-//
-// 	spec.APIKeyDecryptionKey = privateKey
-//
-// 	testEventCh <- &event{added, testAPIKey}
-// 	time.Sleep(100 * time.Millisecond)
-//
-// 	result, _ := spec.KeyStore.Get("CByPKc5XL8U77Ag8c6RbVgKJkJ9B2J5b")
-//
-// 	assert.Equal(spec.APIKey{
-// 		TypeMeta: unversioned.TypeMeta{},
-// 		ObjectMeta: api.ObjectMeta{
-// 			Name:      "abc123",
-// 			Namespace: "foo",
-// 		},
-// 		Spec: spec.APIKeySpec{
-// 			APIKeyData: "CByPKc5XL8U77Ag8c6RbVgKJkJ9B2J5b",
-// 		},
-// 	}, result)
-//
-// 	testAPIKey.Spec.APIKeyData = "3e4d0a287d22be64eb43cca350a27204c51a9da07a37d9d5ec6e5689fc4d659ea7931d814935c24b5a4fb3a724cf40e76c597933b47c1c9b86e04a7a895ec305c4c0b1c4e2066e2fc319ec077adfb12dc267cb1443c0cd2bc4195b3655e418ac45137f475ff6fa5f6b1394b7a63b5f31dca7c3fc6846ca3794c2b7da66e517c4540c5dd51299b6dc65a65e947545601d146b6af7b45aa51f869f13635eb3a8b2a9d7bbce4f7895ade3509333a0b707e49427bfd2da168258095ba585910c99da70c7106e57e9e52ae6556d6626253856afb0a8fcb8b18a3f9b83adeab19fcfe8baf2898d143824aac5450d8b5bb9f772f44614ea5958ff5774c328432a2ec38d"
-// 	testEventCh <- &event{modified, testAPIKey}
-// 	time.Sleep(100 * time.Millisecond)
-// 	result, _ = spec.KeyStore.Get("WS6JlCUtOYBB3cxokdcJAnkIciZaOwyS")
-// 	assert.Equal(spec.APIKey{
-// 		TypeMeta: unversioned.TypeMeta{},
-// 		ObjectMeta: api.ObjectMeta{
-// 			Name:      "abc123",
-// 			Namespace: "foo",
-// 		},
-// 		Spec: spec.APIKeySpec{
-// 			APIKeyData: "WS6JlCUtOYBB3cxokdcJAnkIciZaOwyS",
-// 		},
-// 	}, result)
-//
-// 	testEventCh <- &event{deleted, testAPIKey}
-// 	time.Sleep(100 * time.Millisecond)
-// 	assert.Nil(spec.KeyStore.Get("WS6JlCUtOYBB3cxokdcJAnkIciZaOwyS"))
-//
-// 	testEventCh <- &event{added, testService}
-// 	time.Sleep(100 * time.Millisecond)
-// 	untypedResult, _ := spec.ServiceStore.Get(spec.CreateService(testService), http.Header{})
-// 	resultSvc := untypedResult.(spec.Service)
-// 	assert.Equal("1.2.3.4", resultSvc.ClusterIP)
-//
-// 	testService.Spec.ClusterIP = "2.3.4.5"
-// 	testEventCh <- &event{modified, testService}
-// 	time.Sleep(100 * time.Millisecond)
-// 	untypedResult, _ = spec.ServiceStore.Get(spec.CreateService(testService), http.Header{})
-// 	resultSvc = untypedResult.(spec.Service)
-// 	assert.Equal("2.3.4.5", resultSvc.ClusterIP)
-//
-// 	testEventCh <- &event{deleted, testService}
-// 	time.Sleep(100 * time.Millisecond)
-// 	untypedResult, _ = spec.ServiceStore.Get(spec.CreateService(testService), http.Header{})
-// 	assert.Nil(untypedResult)
-//
-// 	testEventCh <- &event{added, testSecret}
-// 	time.Sleep(100 * time.Millisecond)
-// 	result, _ = spec.SecretStore.Get(testSecret.ObjectMeta.Name, testSecret.ObjectMeta.Namespace)
-// 	assert.Equal(testSecret, result)
-//
-// 	testSecret.Data["tls.ca"] = []byte("YWJjMTIz")
-// 	testEventCh <- &event{modified, testSecret}
-// 	time.Sleep(100 * time.Millisecond)
-// 	result, _ = spec.SecretStore.Get(testSecret.ObjectMeta.Name, testSecret.ObjectMeta.Namespace)
-// 	assert.Equal(testSecret, result)
-//
-// 	testEventCh <- &event{deleted, testSecret}
-// 	time.Sleep(100 * time.Millisecond)
-// 	assert.Nil(spec.SecretStore.Get(testSecret.ObjectMeta.Name, testSecret.ObjectMeta.Namespace))
-//
-// }
-
 func getTestSecret() api.Secret {
 	return api.Secret{
 		TypeMeta: unversioned.TypeMeta{},

diff --git a/grafana.json b/grafana.json
@@ -2,11 +2,18 @@
   "__inputs": [
     {
       "name": "DS_KANALI",
-      "label": "kanali",
-      "description": "",
+      "label": "Datasource",
+      "description": "name of datasource",
       "type": "datasource",
       "pluginId": "influxdb",
       "pluginName": "InfluxDB"
+    },
+    {
+      "name": "VAR_ENV",
+      "label": "Environment",
+      "description": "name of environment",
+      "type": "constant",
+      "value": "LOCAL"
     }
   ],
   "__requires": [
@@ -173,7 +180,7 @@
               "measurement": "request_details",
               "orderByTime": "ASC",
               "policy": "default",
-              "query": "SELECT count(\"clientIP\") FROM \"request_details\" WHERE \"proxyName\" =~ /^$proxyName$/ AND \"responseCode\" = '401' AND \"keyName\" =~ /^$keyName$/ AND \"method\" =~ /^$httpMethod$/ AND $timeFilter GROUP BY time($__interval) fill(0)",
+              "query": "SELECT count(\"clientIP\") FROM \"request_details\" WHERE \"proxyName\" =~ /^$proxyName$/ AND \"responseCode\" = '401' AND \"keyName\" =~ /^$keyName$/ AND $timeFilter GROUP BY time($__interval) fill(0)",
               "rawQuery": false,
               "refId": "B",
               "resultFormat": "time_series",
@@ -2009,6 +2016,6 @@
     ]
   },
   "timezone": "browser",
-  "title": "Kanali",
+  "title": "Kanali ${VAR_ENV}",
   "version": 1
 }
diff --git a/handlers/handler.go b/handlers/handler.go
@@ -52,7 +52,7 @@ func (h Handler) serveHTTP(w http.ResponseWriter, r *http.Request) {
 	defer func() {
 		m.Add(
 			metrics.Metric{Name: "total_time", Value: int(time.Now().Sub(t0) / time.Millisecond), Index: false},
-			metrics.Metric{Name: "http_method", Value: r.Method, Index: true},
+			metrics.Metric{Name: "http_method", Value: r.Method, Index: false},
 			metrics.Metric{Name: "http_uri", Value: utils.ComputeURLPath(r.URL), Index: false},
 			metrics.Metric{Name: "client_ip", Value: strings.Split(r.RemoteAddr, ":")[0], Index: false},
 		)

diff --git a/helm/values.yaml b/helm/values.yaml
@@ -1,7 +1,7 @@
 
 imageRegistry: northwesternmutual
 
-dockerImageTag: v1.2.1
+dockerImageTag: v1.2.3
 
 pullPolicy: Always
 

diff --git a/kubernetes.yml b/kubernetes.yml
@@ -56,7 +56,7 @@ spec:
       containers:
       - name: kanali
         imagePullPolicy: IfNotPresent
-        image: northwesternmutual/kanali:v1.2.1
+        image: northwesternmutual/kanali:v1.2.3
         command:
         - /kanali
         - start

diff --git a/steps/writeresponse.go b/steps/writeresponse.go
@@ -26,6 +26,7 @@ import (
 	"net/http"
 	"strconv"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/northwesternmutual/kanali/metrics"
 	"github.com/northwesternmutual/kanali/spec"
 	"github.com/northwesternmutual/kanali/tracer"
@@ -57,7 +58,7 @@ func (step WriteResponseStep) Do(ctx context.Context, proxy *spec.APIProxy, m *m
 	w.WriteHeader(resp.StatusCode)
 
 	if _, err := io.Copy(w, resp.Body); err != nil {
-		return err
+		logrus.Warnf("error copying data to http response: %s", err.Error())
 	}
 
 	return nil