-
Notifications
You must be signed in to change notification settings - Fork 416
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Cache trigger secrets for the duration of request
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
1 parent
60d1129
commit fc41889
Showing
9 changed files
with
169 additions
and
15 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/* | ||
Copyright 2019 The Tekton Authors | ||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
http://www.apache.org/licenses/LICENSE-2.0 | ||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package interceptors | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
corev1 "k8s.io/api/core/v1" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" | ||
rtesting "knative.dev/pkg/reconciler/testing" | ||
|
||
triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" | ||
) | ||
|
||
const testNS = "testing-ns" | ||
|
||
func Test_GetSecretToken(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
cache map[string]interface{} | ||
wanted []byte | ||
}{ | ||
{ | ||
name: "no matching cache entry exists", | ||
cache: make(map[string]interface{}), | ||
wanted: []byte("secret from API"), | ||
}, | ||
{ | ||
name: "a matching cache entry exists", | ||
cache: map[string]interface{}{ | ||
fmt.Sprintf("secret/%s/test-secret/token", testNS): []byte("secret from cache"), | ||
}, | ||
wanted: []byte("secret from cache"), | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(rt *testing.T) { | ||
req := setCache(&http.Request{}, tt.cache) | ||
|
||
ctx, _ := rtesting.SetupFakeContext(t) | ||
kubeClient := fakekubeclient.Get(ctx) | ||
secretRef := makeSecretRef() | ||
|
||
if _, err := kubeClient.CoreV1().Secrets(testNS).Create(makeSecret("secret from API")); err != nil { | ||
rt.Error(err) | ||
} | ||
|
||
secret, err := GetSecretToken(req, kubeClient, &secretRef, testNS) | ||
if err != nil { | ||
rt.Error(err) | ||
} | ||
|
||
if diff := cmp.Diff(tt.wanted, secret); diff != "" { | ||
rt.Errorf("secret value (-want, +got) = %s", diff) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func makeSecret(secretText string) *corev1.Secret { | ||
return &corev1.Secret{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Namespace: testNS, | ||
Name: "test-secret", | ||
}, | ||
Data: map[string][]byte{ | ||
"token": []byte(secretText), | ||
}, | ||
} | ||
} | ||
|
||
func makeSecretRef() triggersv1.SecretRef { | ||
return triggersv1.SecretRef{ | ||
SecretKey: "token", | ||
SecretName: "test-secret", | ||
Namespace: testNS, | ||
} | ||
} | ||
|
||
func setCache(req *http.Request, vals map[string]interface{}) *http.Request { | ||
return req.WithContext(context.WithValue(req.Context(), requestCacheKey, vals)) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters