diff --git a/policies/ingress/require_ingress_exemption.go b/policies/ingress/require_ingress_exemption.go index 657753c..539758d 100644 --- a/policies/ingress/require_ingress_exemption.go +++ b/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 } diff --git a/policies/pod/bind_mounts.go b/policies/pod/bind_mounts.go index c887024..df07276 100644 --- a/policies/pod/bind_mounts.go +++ b/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 } diff --git a/policies/pod/docker_sock.go b/policies/pod/docker_sock.go index b3cc445..00c364a 100644 --- a/policies/pod/docker_sock.go +++ b/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 } diff --git a/policies/pod/immutable_image_digest.go b/policies/pod/immutable_image_digest.go index a1ed0f8..bffc4dc 100644 --- a/policies/pod/immutable_image_digest.go +++ b/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 } diff --git a/policies/pod/mutate_default_seccomp_policy.go b/policies/pod/mutate_default_seccomp_policy.go index 41c6055..99232ad 100644 --- a/policies/pod/mutate_default_seccomp_policy.go +++ b/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 } diff --git a/policies/pod/mutate_safe_to_evict.go b/policies/pod/mutate_safe_to_evict.go index 0574b31..79bae6d 100644 --- a/policies/pod/mutate_safe_to_evict.go +++ b/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 } diff --git a/policies/pod/no_exec.go b/policies/pod/no_exec.go index 5af185f..7ae5734 100644 --- a/policies/pod/no_exec.go +++ b/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 } diff --git a/policies/pod/no_host_network.go b/policies/pod/no_host_network.go index a41ab4c..c287bd2 100644 --- a/policies/pod/no_host_network.go +++ b/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 } diff --git a/policies/pod/no_host_pid.go b/policies/pod/no_host_pid.go index 794b3ee..d04586d 100644 --- a/policies/pod/no_host_pid.go +++ b/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 } diff --git a/policies/pod/no_new_capabilities.go b/policies/pod/no_new_capabilities.go index ff5e7ab..0bbfa95 100644 --- a/policies/pod/no_new_capabilities.go +++ b/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 } diff --git a/policies/pod/no_privileged_container.go b/policies/pod/no_privileged_container.go index 50847f6..3abd8b9 100644 --- a/policies/pod/no_privileged_container.go +++ b/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 } diff --git a/policies/pod/no_shareprocessnamespace.go b/policies/pod/no_shareprocessnamespace.go index a7c2aa0..6d48128 100644 --- a/policies/pod/no_shareprocessnamespace.go +++ b/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 } diff --git a/policies/pod/no_tiller.go b/policies/pod/no_tiller.go index 453344d..a65190d 100644 --- a/policies/pod/no_tiller.go +++ b/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 } diff --git a/policies/pod/safe_to_evict.go b/policies/pod/safe_to_evict.go index 1b56536..d272fdc 100644 --- a/policies/pod/safe_to_evict.go +++ b/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 } diff --git a/policies/pod/trusted_repository.go b/policies/pod/trusted_repository.go index 7616053..682fc5a 100644 --- a/policies/pod/trusted_repository.go +++ b/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 } diff --git a/resource/context.go b/resource/context.go new file mode 100644 index 0000000..26df74c --- /dev/null +++ b/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) +} diff --git a/resource/context_test.go b/resource/context_test.go new file mode 100644 index 0000000..63208ef --- /dev/null +++ b/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 +} diff --git a/resource/ingress.go b/resource/ingress.go index 5eab49f..079d47e 100644 --- a/resource/ingress.go +++ b/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{} diff --git a/resource/pod.go b/resource/pod.go index 911149a..44d9e14 100644 --- a/resource/pod.go +++ b/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{} diff --git a/resource/pod_exec.go b/resource/pod_exec.go index ead949c..4e60fc7 100644 --- a/resource/pod_exec.go +++ b/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{} diff --git a/resource/pod_test.go b/resource/pod_test.go new file mode 100644 index 0000000..5692674 --- /dev/null +++ b/resource/pod_test.go @@ -0,0 +1,56 @@ +package resource + +import ( + "context" + "testing" + + admissionv1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func BenchmarkDecodePodWithoutCaching(b *testing.B) { + req := fakeReq([]byte(podExample)) + ctx := context.TODO() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = GetPodResource(req, ctx) + } +} + +func BenchmarkDecodePodCaching(b *testing.B) { + req := fakeReq([]byte(podExample)) + ctx := WithResourceCache(context.TODO()) + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = GetPodResource(req, ctx) + } +} + +func fakeReq(b []byte) *admissionv1beta1.AdmissionRequest { + return &admissionv1beta1.AdmissionRequest{ + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Name: "any", + Namespace: "test", + Object: runtime.RawExtension{Raw: b}, + } +} + +const podExample = ` +kind: Pod +apiVersion: apps/v1 +metadata: + name: foobar + namespace: testing + annotations: + created-by: alpe + labels: + app: foo +spec: + containers: + - name: foo + image: v0.1.0 + ports: + - name: http + containerPort: 8080 +` diff --git a/server/webhook.go b/server/webhook.go index e0f6705..50bef07 100644 --- a/server/webhook.go +++ b/server/webhook.go @@ -107,7 +107,8 @@ func (s *Server) ValidatingWebhook(w http.ResponseWriter, r *http.Request) { // validateResources accepts K8s resources to process func (s *Server) validateResources(ar v1beta1.AdmissionReview) v1beta1.AdmissionReview { - ctx, cancelfn := context.WithTimeout(context.Background(), 5*time.Second) + ctx := resource.WithResourceCache(context.Background()) + ctx, cancelfn := context.WithTimeout(ctx, 5*time.Second) defer cancelfn() if ar.Request == nil { @@ -142,7 +143,7 @@ func (s *Server) validateResources(ar v1beta1.AdmissionReview) v1beta1.Admission // TODO: This could use a bit of refactoring so there is less repetition and we could // have the relevant resource name available for any resource being checked for exemptions. // The AdmissionReview Name is often empty and populated by an downstream controller. - podResource := resource.GetPodResource(ar.Request) + podResource := resource.GetPodResource(ar.Request, ctx) if len(violations) == 0 && patches != nil && !policies.IsExempt( podResource.ResourceName, ar.Request.Namespace,