Merge pull request #38 from alpe/alex/resource_caching_proposal

Proposal for caching admission resources decoding

policies/ingress/require_ingress_exemption.go

@@ -31,7 +31,7 @@ func (p PolicyRequireIngressExemption) Validate(ctx context.Context, config poli
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	ingressResource := resource.GetIngressResource(ar)
+	ingressResource := resource.GetIngressResource(ctx, ar)
 	if ingressResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/bind_mounts.go

@@ -30,7 +30,7 @@ func (p PolicyBindMounts) Validate(ctx context.Context, config policies.Config,
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/docker_sock.go

@@ -31,7 +31,7 @@ func (p PolicyDockerSock) Validate(ctx context.Context, config policies.Config,
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/immutable_image_digest.go

@@ -35,7 +35,7 @@ func (p PolicyImageImmutableReference) Validate(ctx context.Context, config poli
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/mutate_default_seccomp_policy.go

@@ -28,7 +28,7 @@ func (p PolicyDefaultSeccompPolicy) Name() string {
 
 func (p PolicyDefaultSeccompPolicy) Validate(ctx context.Context, config policies.Config, ar *admissionv1beta1.AdmissionRequest) ([]policies.ResourceViolation, []policies.PatchOperation) {
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return nil, nil
 	}

policies/pod/mutate_safe_to_evict.go

@@ -28,7 +28,7 @@ func (p PolicyMutateSafeToEvict) Name() string {
 
 func (p PolicyMutateSafeToEvict) Validate(ctx context.Context, config policies.Config, ar *admissionv1beta1.AdmissionRequest) ([]policies.ResourceViolation, []policies.PatchOperation) {
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return nil, nil
 	}

policies/pod/no_exec.go

@@ -31,7 +31,7 @@ func (p PolicyNoExec) Validate(ctx context.Context, config policies.Config, ar *
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podExecResource := resource.GetPodExecResource(ar)
+	podExecResource := resource.GetPodExecResource(ctx, ar)
 	if podExecResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_host_network.go

@@ -31,7 +31,7 @@ func (p PolicyNoHostNetwork) Validate(ctx context.Context, config policies.Confi
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_host_pid.go

@@ -31,7 +31,7 @@ func (p PolicyNoHostPID) Validate(ctx context.Context, config policies.Config, a
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_new_capabilities.go

@@ -32,7 +32,7 @@ func (p PolicyNoNewCapabilities) Validate(ctx context.Context, config policies.C
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_privileged_container.go

@@ -32,7 +32,7 @@ func (p PolicyNoPrivilegedContainer) Validate(ctx context.Context, config polici
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_shareprocessnamespace.go

@@ -31,7 +31,7 @@ func (p PolicyNoShareProcessNamespace) Validate(ctx context.Context, config poli
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/no_tiller.go

@@ -33,7 +33,7 @@ func (p PolicyNoTiller) Validate(ctx context.Context, config policies.Config, ar
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/safe_to_evict.go

@@ -30,7 +30,7 @@ func (p PolicySafeToEvict) Validate(ctx context.Context, config policies.Config,
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

policies/pod/trusted_repository.go

@@ -33,7 +33,7 @@ func (p PolicyTrustedRepository) Validate(ctx context.Context, config policies.C
 
 	resourceViolations := []policies.ResourceViolation{}
 
-	podResource := resource.GetPodResource(ar)
+	podResource := resource.GetPodResource(ar, ctx)
 	if podResource == nil {
 		return resourceViolations, nil
 	}

resource/context.go

@@ -0,0 +1,54 @@
+package resource
+
+import (
+	"context"
+	"sync"
+)
+
+type contextKey int // local to the resource module
+
+const (
+	ctxKeyCache contextKey = iota
+)
+const (
+	cacheKeyPod = iota
+	cacheKeyPodExec
+	cacheKeyIngress
+)
+
+type cache struct {
+	l sync.Mutex
+	m map[int]interface{}
+}
+
+// getOrSet returns the cached value for the given key. When none exists the passed function is called once to create
+// the initial value. When cache is nil no caching happens and the create function is always called.
+// Calls are executed thread safe.
+func (c *cache) getOrSet(cacheKey int, f func() interface{}) interface{} {
+	if c == nil {
+		return f()
+	}
+	c.l.Lock()
+	defer c.l.Unlock()
+	if p, ok := c.m[cacheKey]; ok {
+		return p
+	}
+	v := f()
+	c.m[cacheKey] = v
+	return v
+}
+
+// WithResourceCache adds a resource cache to the context returned.
+func WithResourceCache(ctx context.Context) context.Context {
+	c := &cache{m: make(map[int]interface{}, 1)}
+	return context.WithValue(ctx, ctxKeyCache, c)
+}
+
+// GetResourceCache returns the cache from the context. Result will return nil when none exists.
+func GetResourceCache(ctx context.Context) *cache {
+	c := ctx.Value(ctxKeyCache)
+	if c == nil {
+		return nil
+	}
+	return c.(*cache)
+}

resource/context_test.go

@@ -0,0 +1,128 @@
+package resource
+
+import (
+	"context"
+	"reflect"
+	"sync"
+	"testing"
+	"time"
+)
+
+func TestResourceCacheContext(t *testing.T) {
+	const testRunCount = 2
+	const testCacheKey = 999
+	specs := map[string]struct {
+		srcCtx         func() context.Context
+		srcKey         int
+		srcFactoryFunc *creatorMock
+		expResp        interface{}
+		expCalls       int
+	}{
+		"with cache in ctx ": {
+			srcCtx: func() context.Context {
+				return WithResourceCache(context.TODO())
+			},
+			srcKey:         testCacheKey,
+			srcFactoryFunc: &creatorMock{respValue: "myValue"},
+			expCalls:       1,
+			expResp:        "myValue",
+		},
+		"with cache filled": {
+			srcCtx: func() context.Context {
+				ctx := WithResourceCache(context.TODO())
+				GetResourceCache(ctx).getOrSet(testCacheKey, func() interface{} {
+					return "myValue"
+				})
+				return ctx
+			},
+			srcKey:         testCacheKey,
+			srcFactoryFunc: &creatorMock{respValue: "otherValue"},
+			expCalls:       0,
+			expResp:        "myValue",
+		},
+		"with empty ctx": {
+			srcCtx:         context.TODO,
+			srcKey:         testCacheKey,
+			srcFactoryFunc: &creatorMock{respValue: "foo"},
+			expCalls:       testRunCount,
+			expResp:        "foo",
+		},
+	}
+	for msg, spec := range specs {
+		t.Run(msg, func(t *testing.T) {
+			ctx := spec.srcCtx()
+			mock := spec.srcFactoryFunc
+			for i := 0; i < testRunCount; i++ {
+				resp := GetResourceCache(ctx).getOrSet(spec.srcKey, mock.CountCall)
+				if exp, got := spec.expResp, resp; !reflect.DeepEqual(exp, got) {
+					t.Errorf("expected %v but got %v", exp, got)
+				}
+			}
+			if exp, got := spec.expCalls, mock.called; exp != got {
+				t.Errorf("expected %d but got %d", exp, got)
+			}
+		})
+	}
+}
+
+func TestCacheWithConcurrentAccess(t *testing.T) {
+	const testCacheKey = 999
+	const actorCount = 10
+
+	var awaitStart sync.WaitGroup
+	awaitStart.Add(actorCount)
+	var awaitCompleted sync.WaitGroup
+	awaitCompleted.Add(actorCount)
+
+	actors := make([]*creatorMock, actorCount)
+	rsp := make(chan interface{}, actorCount)
+
+	c := &cache{m: make(map[int]interface{}, 1)}
+	for i := 0; i < actorCount; i++ {
+		actors[i] = &creatorMock{respValue: i}
+		go func(i int) {
+			awaitStart.Done()
+			awaitStart.Wait() // wait for all actors to start sync
+			rsp <- c.getOrSet(testCacheKey, actors[i].CountCall)
+			awaitCompleted.Done()
+		}(i)
+	}
+	awaitCompleted.Wait()
+
+	// then only 1 create function should be called
+	var active *creatorMock
+	var expResult int
+	for i, a := range actors {
+		if a.called != 0 {
+			if active != nil {
+				t.Fatal("more than 1 create function called")
+			}
+			active = a
+			expResult = i
+		}
+	}
+	// and all should see the same result
+	for i := 0; i < actorCount; i++ {
+		select {
+		case r := <-rsp:
+			if exp, got := expResult, r; exp != got {
+				t.Errorf("expected %v but got %v", exp, got)
+			}
+		case <-time.After(time.Millisecond):
+			t.Fatal("test timeout")
+		}
+	}
+}
+
+type creatorMock struct {
+	l         sync.Mutex
+	called    int
+	respValue interface{}
+}
+
+func (m *creatorMock) CountCall() interface{} {
+	m.l.Lock()
+	m.called++
+	m.l.Unlock()
+	return m.respValue
+}

resource/ingress.go

@@ -13,6 +13,8 @@
 package resource
 
 import (
+	"context"
+
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	networkingv1beta1 "k8s.io/api/networking/v1beta1"
@@ -28,7 +30,14 @@ type IngressResource struct {
 }
 
 // GetIngressResource extracts and IngressResource from an AdmissionRequest
-func GetIngressResource(ar *admissionv1beta1.AdmissionRequest) *IngressResource {
+func GetIngressResource(ctx context.Context, ar *admissionv1beta1.AdmissionRequest) *IngressResource {
+	c := GetResourceCache(ctx)
+	return c.getOrSet(cacheKeyIngress, func() interface{} {
+		return decodeIngressResource(ar)
+	}).(*IngressResource)
+}
+
+func decodeIngressResource(ar *admissionv1beta1.AdmissionRequest) *IngressResource {
 	switch ar.Resource {
 	case metav1.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "ingresses"}:
 		ing := extensionsv1beta1.Ingress{}

resource/pod.go

@@ -13,6 +13,8 @@
 package resource
 
 import (
+	"context"
+
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
@@ -34,8 +36,14 @@ type PodResource struct {
 }
 
 // GetPodResource extracts a PodResource from an AdmissionRequest
-func GetPodResource(ar *admissionv1beta1.AdmissionRequest) *PodResource {
+func GetPodResource(ar *admissionv1beta1.AdmissionRequest, ctx context.Context) *PodResource {
+	c := GetResourceCache(ctx)
+	return c.getOrSet(cacheKeyPod, func() interface{} {
+		return decodePodResource(ar)
+	}).(*PodResource)
+}
 
+func decodePodResource(ar *admissionv1beta1.AdmissionRequest) *PodResource {
 	switch ar.Resource {
 	case metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}:
 		pod := corev1.Pod{}

resource/pod_exec.go

@@ -13,6 +13,7 @@
 package resource
 
 import (
+	"context"
 	"encoding/json"
 	"strings"
 
@@ -29,7 +30,14 @@ type PodExecResource struct {
 }
 
 // GetPodExecResource extracts and PodExecResource from an AdmissionRequest
-func GetPodExecResource(ar *admissionv1beta1.AdmissionRequest) *PodExecResource {
+func GetPodExecResource(ctx context.Context, ar *admissionv1beta1.AdmissionRequest) *PodExecResource {
+	c := GetResourceCache(ctx)
+	return c.getOrSet(cacheKeyPodExec, func() interface{} {
+		return decodePodExecResource(ar)
+	}).(*PodExecResource)
+}
+
+func decodePodExecResource(ar *admissionv1beta1.AdmissionRequest) *PodExecResource {
 	switch ar.Kind {
 	case metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "PodExecOptions"}:
 		podExecOptions := corev1.PodExecOptions{}