Skip to content

Commit

Permalink
Cache trigger secrets for the duration of request
Browse files Browse the repository at this point in the history
This commit adds a request-local cache for interceptors to leverage
during the processing of triggers. It allows interceptors to avoid doing
expensive work more than once for each request, such as fetching a
Kubernetes secret for validating webhooks.

The implementation uses the request context to provide the cache. This
was the least disruptive method of providing a cache for use with
interceptors, and is appropriate if you consider the cache should live
only for the duration of each request.

Alternative implementations might have used the client-go informers to
extend the Kubernetes client to watch for secrets in the cluster. This
would cause the work required to fetch secrets to scale with the number
of secrets in the cluster, as opposed to making a fresh request per
webhook we process. That said, building caching clients seems like more
work than is necessary for fixing this simple problem, which is why I
went with a simple cache object.

The background for this change was finding Github webhooks timing out
once we exceeded ~40 triggers on our EventListener. While the CEL
filtering was super fast, the validation of Github webhook signatures
was being computed for every trigger, even though each trigger used the
same Github secret. Pulling the secret from Kubernetes was taking about
250ms, which meant 40 triggers exceeded the 10s Github timeout.
  • Loading branch information
lawrencejones committed May 26, 2020
1 parent 61a2e7b commit dbce2b6
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
42 changes: 40 additions & 2 deletions pkg/interceptors/interceptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
package interceptors

import (
"context"
"net/http"
"path"

triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -29,7 +31,40 @@ type Interceptor interface {
ExecuteTrigger(req *http.Request) (*http.Response, error)
}

func GetSecretToken(cs kubernetes.Interface, sr *triggersv1.SecretRef, eventListenerNamespace string) ([]byte, error) {
const requestCacheKey = "interceptors.RequestCache"

// WithCache clones the given request and sets the request context to include a cache.
// This allows us to cache results from expensive operations and perform them just once
// per each trigger.
//
// Each request should have its own cache, and those caches should expire once the request
// is processed. For this reason, it's appropriate to store the cache on the request
// context.
func WithCache(req *http.Request) *http.Request {
return req.WithContext(context.WithValue(req.Context(), requestCacheKey, make(map[string]interface{})))
}

func getCache(req *http.Request) map[string]interface{} {
if cache, ok := req.Context().Value(requestCacheKey).(map[string]interface{}); ok {
return cache
}

return make(map[string]interface{})
}

// GetSecretToken queries Kubernetes for the given secret reference. We use this function
// to resolve secret material like Github webhook secrets, and call it once for every
// trigger that references it.
//
// As we may have many triggers that all use the same secret, we cache the secret values
// in the request cache.
func GetSecretToken(req *http.Request, cs kubernetes.Interface, sr *triggersv1.SecretRef, eventListenerNamespace string) ([]byte, error) {
cacheKey := path.Join("secret", sr.Namespace, sr.SecretName, sr.SecretKey)
cache := getCache(req)
if secretValue, ok := cache[cacheKey]; ok {
return secretValue.([]byte), nil
}

ns := sr.Namespace
if ns == "" {
ns = eventListenerNamespace
Expand All @@ -39,5 +74,8 @@ func GetSecretToken(cs kubernetes.Interface, sr *triggersv1.SecretRef, eventList
return nil, err
}

return secret.Data[sr.SecretKey], nil
secretValue := secret.Data[sr.SecretKey]
cache[cacheKey] = secret.Data[sr.SecretKey]

return secretValue, nil
}
5 changes: 5 additions & 0 deletions pkg/sink/sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ func (r Sink) executeInterceptors(t *triggersv1.EventListenerTrigger, in *http.R
Header: in.Header,
Body: ioutil.NopCloser(bytes.NewBuffer(event)),
}

// We create a cache against each request, so whenever we make network calls like
// fetching kubernetes secrets, we can do so only once per request.
request = interceptors.WithCache(request)

var resp *http.Response
for _, i := range t.Interceptors {
var interceptor interceptors.Interceptor
Expand Down

0 comments on commit dbce2b6

Please sign in to comment.