From 5bae96d773cd79d30b94eae1a3e186b037d6c27f Mon Sep 17 00:00:00 2001 From: gabemontero Date: Tue, 8 Sep 2020 12:55:07 -0400 Subject: [PATCH] remove interceptor cross namespace secret references; adjust controller permission accordingly --- config/200-clusterrole.yaml | 2 +- config/200-role.yaml | 32 +++++++++++++++++++ config/201-rolebinding.yaml | 30 +++++++++++++++++ .../triggers/v1alpha1/event_listener_types.go | 1 - pkg/interceptors/bitbucket/bitbucket_test.go | 6 +--- pkg/interceptors/cel/cel_test.go | 6 ---- pkg/interceptors/cel/triggers.go | 14 ++------ pkg/interceptors/github/github_test.go | 6 +--- pkg/interceptors/gitlab/gitlab_test.go | 6 +--- pkg/interceptors/interceptors.go | 8 ++--- pkg/interceptors/interceptors_test.go | 1 - pkg/sink/sink_test.go | 3 -- 12 files changed, 70 insertions(+), 45 deletions(-) create mode 100644 config/200-role.yaml create mode 100644 config/201-rolebinding.yaml diff --git a/config/200-clusterrole.yaml b/config/200-clusterrole.yaml index 0cf588100..9f919a8e6 100644 --- a/config/200-clusterrole.yaml +++ b/config/200-clusterrole.yaml @@ -21,7 +21,7 @@ metadata: app.kubernetes.io/part-of: tekton-triggers rules: - apiGroups: [""] - resources: ["configmaps", "secrets", "services", "events"] + resources: ["configmaps", "services", "events"] verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] - apiGroups: ["apps"] resources: ["deployments", "deployments/finalizers"] diff --git a/config/200-role.yaml b/config/200-role.yaml new file mode 100644 index 000000000..6d05aa36b --- /dev/null +++ b/config/200-role.yaml @@ -0,0 +1,32 @@ +# Copyright 2020 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. + +# NOTE: when multi-tenant EventListener progresses, moving this Role +# to a ClusterRole is not the advisable path. Additional Roles that +# adds access to Secrets to the Namespaces managed by the multi-tenant +# EventListener is what should be done. While not as simple, it avoids +# giving access to K8S system level, cluster admin privileged level Secrets + +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: tekton-triggers-admin + namespace: tekton-pipelines + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-triggers +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] diff --git a/config/201-rolebinding.yaml b/config/201-rolebinding.yaml new file mode 100644 index 000000000..00d3faebb --- /dev/null +++ b/config/201-rolebinding.yaml @@ -0,0 +1,30 @@ +# Copyright 2020 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. + +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: tekton-triggers-controller-admin + namespace: tekton-pipelines + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-triggers +subjects: + - kind: ServiceAccount + name: tekton-triggers-controller + namespace: tekton-pipelines +roleRef: + kind: Role + name: tekton-triggers-admin + apiGroup: rbac.authorization.k8s.io diff --git a/pkg/apis/triggers/v1alpha1/event_listener_types.go b/pkg/apis/triggers/v1alpha1/event_listener_types.go index e819bacb9..daa7f3f7b 100644 --- a/pkg/apis/triggers/v1alpha1/event_listener_types.go +++ b/pkg/apis/triggers/v1alpha1/event_listener_types.go @@ -112,7 +112,6 @@ type EventInterceptor = TriggerInterceptor type SecretRef struct { SecretKey string `json:"secretKey,omitempty"` SecretName string `json:"secretName,omitempty"` - Namespace string `json:"namespace,omitempty"` } // EventListenerBinding refers to a particular TriggerBinding or ClusterTriggerBindingresource. diff --git a/pkg/interceptors/bitbucket/bitbucket_test.go b/pkg/interceptors/bitbucket/bitbucket_test.go index d4ba0fae3..08866903e 100644 --- a/pkg/interceptors/bitbucket/bitbucket_test.go +++ b/pkg/interceptors/bitbucket/bitbucket_test.go @@ -231,11 +231,7 @@ func TestInterceptor_ExecuteTrigger_Signature(t *testing.T) { request.Header.Add("X-Hub-Signature", tt.args.signature) } if tt.args.secret != nil { - ns := tt.Bitbucket.SecretRef.Namespace - if ns == "" { - ns = metav1.NamespaceDefault - } - if _, err := kubeClient.CoreV1().Secrets(ns).Create(ctx, tt.args.secret, metav1.CreateOptions{}); err != nil { + if _, err := kubeClient.CoreV1().Secrets(metav1.NamespaceDefault).Create(ctx, tt.args.secret, metav1.CreateOptions{}); err != nil { t.Error(err) } } diff --git a/pkg/interceptors/cel/cel_test.go b/pkg/interceptors/cel/cel_test.go index b6e9c928d..00209cda6 100644 --- a/pkg/interceptors/cel/cel_test.go +++ b/pkg/interceptors/cel/cel_test.go @@ -578,12 +578,6 @@ func TestExpressionEvaluation_Error(t *testing.T) { expr: "'testing'.compareSecret('testing', 'testSecret', 'mytoken')", want: "failed to find secret.*testing.*", }, - { - name: "secret not in default ns", - expr: "'testing'.compareSecret('testSecret', 'mytoken')", - secretNS: "another-ns", - want: "failed to find secret.*another-ns.*", - }, { name: "invalid parseJSON body", expr: "body.value.parseJSON().test == 'test'", diff --git a/pkg/interceptors/cel/triggers.go b/pkg/interceptors/cel/triggers.go index 6c83aebb9..ef3a5e3aa 100644 --- a/pkg/interceptors/cel/triggers.go +++ b/pkg/interceptors/cel/triggers.go @@ -291,17 +291,8 @@ func makeCompareSecret(request *http.Request, defaultNS string, k kubernetes.Int if !ok { return types.ValOrErr(compareString, "unexpected type '%v' passed to compareSecret", vals[0].Type()) } - paramCount := len(vals) - - var secretNS types.String - if paramCount == 4 { - secretNS, ok = vals[3].(types.String) - if !ok { - return types.ValOrErr(secretNS, "unexpected type '%v' passed to compareSecret", vals[1].Type()) - } - } else { - secretNS = types.String(defaultNS) - } + + secretNS := types.String(defaultNS) secretName, ok := vals[2].(types.String) if !ok { @@ -316,7 +307,6 @@ func makeCompareSecret(request *http.Request, defaultNS string, k kubernetes.Int secretRef := &triggersv1.SecretRef{ SecretKey: string(secretKey), SecretName: string(secretName), - Namespace: string(secretNS), } secretToken, err := interceptors.GetSecretToken(request, k, secretRef, string(secretNS)) if err != nil { diff --git a/pkg/interceptors/github/github_test.go b/pkg/interceptors/github/github_test.go index 3e9c2d862..ef5b79d39 100644 --- a/pkg/interceptors/github/github_test.go +++ b/pkg/interceptors/github/github_test.go @@ -231,11 +231,7 @@ func TestInterceptor_ExecuteTrigger_Signature(t *testing.T) { request.Header.Add("X-Hub-Signature", tt.args.signature) } if tt.args.secret != nil { - ns := tt.GitHub.SecretRef.Namespace - if ns == "" { - ns = metav1.NamespaceDefault - } - if _, err := kubeClient.CoreV1().Secrets(ns).Create(ctx, tt.args.secret, metav1.CreateOptions{}); err != nil { + if _, err := kubeClient.CoreV1().Secrets(metav1.NamespaceDefault).Create(ctx, tt.args.secret, metav1.CreateOptions{}); err != nil { t.Error(err) } } diff --git a/pkg/interceptors/gitlab/gitlab_test.go b/pkg/interceptors/gitlab/gitlab_test.go index 199766d9c..ac7765a4b 100644 --- a/pkg/interceptors/gitlab/gitlab_test.go +++ b/pkg/interceptors/gitlab/gitlab_test.go @@ -215,11 +215,7 @@ func TestInterceptor_ExecuteTrigger(t *testing.T) { request.Header.Add("X-GitLab-Event", tt.args.eventType) } if tt.args.secret != nil { - ns := tt.GitLab.SecretRef.Namespace - if ns == "" { - ns = metav1.NamespaceDefault - } - if _, err := kubeClient.CoreV1().Secrets(ns).Create(context.Background(), tt.args.secret, metav1.CreateOptions{}); err != nil { + if _, err := kubeClient.CoreV1().Secrets(metav1.NamespaceDefault).Create(context.Background(), tt.args.secret, metav1.CreateOptions{}); err != nil { t.Error(err) } } diff --git a/pkg/interceptors/interceptors.go b/pkg/interceptors/interceptors.go index 0db9bc001..1b80f9439 100644 --- a/pkg/interceptors/interceptors.go +++ b/pkg/interceptors/interceptors.go @@ -63,7 +63,7 @@ func getCache(req *http.Request) map[string]interface{} { func GetSecretToken(req *http.Request, cs kubernetes.Interface, sr *triggersv1.SecretRef, eventListenerNamespace string) ([]byte, error) { var cache map[string]interface{} - cacheKey := path.Join("secret", sr.Namespace, sr.SecretName, sr.SecretKey) + cacheKey := path.Join("secret", eventListenerNamespace, sr.SecretName, sr.SecretKey) if req != nil { cache = getCache(req) if secretValue, ok := cache[cacheKey]; ok { @@ -71,11 +71,7 @@ func GetSecretToken(req *http.Request, cs kubernetes.Interface, sr *triggersv1.S } } - ns := sr.Namespace - if ns == "" { - ns = eventListenerNamespace - } - secret, err := cs.CoreV1().Secrets(ns).Get(context.Background(), sr.SecretName, metav1.GetOptions{}) + secret, err := cs.CoreV1().Secrets(eventListenerNamespace).Get(context.Background(), sr.SecretName, metav1.GetOptions{}) if err != nil { return nil, err } diff --git a/pkg/interceptors/interceptors_test.go b/pkg/interceptors/interceptors_test.go index 967849b02..def0558ee 100644 --- a/pkg/interceptors/interceptors_test.go +++ b/pkg/interceptors/interceptors_test.go @@ -93,7 +93,6 @@ func makeSecretRef() triggersv1.SecretRef { return triggersv1.SecretRef{ SecretKey: "token", SecretName: "test-secret", - Namespace: testNS, } } diff --git a/pkg/sink/sink_test.go b/pkg/sink/sink_test.go index 7e20ac9c7..9e234cdef 100644 --- a/pkg/sink/sink_test.go +++ b/pkg/sink/sink_test.go @@ -413,7 +413,6 @@ func TestHandleEventWithInterceptors(t *testing.T) { SecretRef: &triggersv1.SecretRef{ SecretKey: "secretKey", SecretName: "secret", - Namespace: namespace, }, EventTypes: []string{"pull_request"}, }, @@ -997,7 +996,6 @@ func TestHandleEventWithInterceptorsAndTriggerAuth(t *testing.T) { SecretRef: &triggersv1.SecretRef{ SecretKey: "secretKey", SecretName: "secret", - Namespace: namespace, }, EventTypes: []string{"pull_request"}, }, @@ -1085,7 +1083,6 @@ func TestHandleEventWithBitbucketInterceptors(t *testing.T) { SecretRef: &triggersv1.SecretRef{ SecretKey: "secretKey", SecretName: "secret", - Namespace: namespace, }, EventTypes: []string{"repo:refs_changed"}, },