detect if k8s event handlers modify read-only objects

k8s libraries provide a mechanism to detect if object handlers are modifying objects that are read only. We will enable this functionality by default in our CI to avoid these accidental writes. Signed-off-by: André Martins <andre@cilium.io>
cilium · Apr 16, 2020 · 406eaba · 406eaba
1 parent 809dba5
commit 406eaba
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 15 deletions.
diff --git a/install/kubernetes/cilium/charts/agent/templates/daemonset.yaml b/install/kubernetes/cilium/charts/agent/templates/daemonset.yaml
@@ -86,6 +86,10 @@ spec:
             fieldRef:
               apiVersion: v1
               fieldPath: spec.nodeName
+{{- if .Values.global.ci.kubeCacheMutationDetector }}
+        - name: KUBE_CACHE_MUTATION_DETECTOR
+          value: "true"
+{{- end }}
         - name: CILIUM_K8S_NAMESPACE
           valueFrom:
             fieldRef:

diff --git a/install/kubernetes/cilium/charts/operator/templates/deployment.yaml b/install/kubernetes/cilium/charts/operator/templates/deployment.yaml
@@ -40,6 +40,10 @@ spec:
             fieldRef:
               apiVersion: v1
               fieldPath: spec.nodeName
+{{- if .Values.global.ci.kubeCacheMutationDetector }}
+        - name: KUBE_CACHE_MUTATION_DETECTOR
+          value: "true"
+{{- end }}
         - name: CILIUM_K8S_NAMESPACE
           valueFrom:
             fieldRef:

diff --git a/install/kubernetes/cilium/values.yaml b/install/kubernetes/cilium/values.yaml
@@ -435,3 +435,8 @@ global:
     # See https://github.com/cilium/hubble/blob/master/Documentation/metrics.md for more comprehensive
     # documentation about Hubble's metric collection.
     metrics: []
+
+  # CI specific options: DO NOT USE IN PRODUCTION.
+  ci:
+    # Make Cilium panic if objects received by k8s are modified.
+    kubeCacheMutationDetector: false
diff --git a/pkg/k8s/informer/informer.go b/pkg/k8s/informer/informer.go
@@ -15,6 +15,7 @@
 package informer
 
 import (
+	"fmt"
 	"time"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -55,6 +56,8 @@ func NewInformerWithStore(
 	// of update/delete deltas.
 	fifo := cache.NewDeltaFIFO(cache.MetaNamespaceKeyFunc, clientState)
 
+	cacheMutationDetector := cache.NewCacheMutationDetector(fmt.Sprintf("%T", objType))
+
 	cfg := &cache.Config{
 		Queue:            fifo,
 		ListerWatcher:    lw,
@@ -68,6 +71,10 @@ func NewInformerWithStore(
 
 				obj := convertFunc(d.Object)
 
+				// In CI we detect if the objects were modified and panic
+				// this is a no-op in production environments.
+				cacheMutationDetector.AddObject(obj)
+
 				switch d.Type {
 				case cache.Sync, cache.Added, cache.Updated:
 					if old, exists, err := clientState.Get(obj); err == nil && exists {

diff --git a/test/helpers/kubectl.go b/test/helpers/kubectl.go
@@ -85,21 +85,22 @@ var (
 	// below. These overrides represent a desire to set the default for all
 	// tests, instead of test-specific variations.
 	defaultHelmOptions = map[string]string{
-		"global.registry":               "k8s1:5000/cilium",
-		"agent.image":                   "cilium-dev",
-		"preflight.image":               "cilium-dev", // Set again in init to match agent.image!
-		"global.tag":                    "latest",
-		"operator.image":                "operator",
-		"managed-etcd.registry":         "docker.io/cilium",
-		"global.debug.enabled":          "true",
-		"global.k8s.requireIPv4PodCIDR": "true",
-		"global.pprof.enabled":          "true",
-		"global.logSystemLoad":          "true",
-		"global.bpf.preallocateMaps":    "true",
-		"global.etcd.leaseTTL":          "30s",
-		"global.ipv4.enabled":           "true",
-		"global.ipv6.enabled":           "true",
-		"global.psp.enabled":            "true",
+		"global.registry":                     "k8s1:5000/cilium",
+		"agent.image":                         "cilium-dev",
+		"preflight.image":                     "cilium-dev", // Set again in init to match agent.image!
+		"global.tag":                          "latest",
+		"operator.image":                      "operator",
+		"managed-etcd.registry":               "docker.io/cilium",
+		"global.debug.enabled":                "true",
+		"global.k8s.requireIPv4PodCIDR":       "true",
+		"global.pprof.enabled":                "true",
+		"global.logSystemLoad":                "true",
+		"global.bpf.preallocateMaps":          "true",
+		"global.etcd.leaseTTL":                "30s",
+		"global.ipv4.enabled":                 "true",
+		"global.ipv6.enabled":                 "true",
+		"global.psp.enabled":                  "true",
+		"global.ci.kubeCacheMutationDetector": "true",
 	}
 
 	flannelHelmOverrides = map[string]string{