Skip to content

Commit

Permalink
remove interceptor cross namespace secret references; adjust controll…
Browse files Browse the repository at this point in the history
…er permission accordingly
  • Loading branch information
gabemontero authored and tekton-robot committed Oct 8, 2020
1 parent 09539a8 commit 5bae96d
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 45 deletions.
2 changes: 1 addition & 1 deletion config/200-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
32 changes: 32 additions & 0 deletions config/200-role.yaml
Original file line number Diff line number Diff line change
@@ -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"]
30 changes: 30 additions & 0 deletions config/201-rolebinding.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion pkg/apis/triggers/v1alpha1/event_listener_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 1 addition & 5 deletions pkg/interceptors/bitbucket/bitbucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/interceptors/cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand Down
14 changes: 2 additions & 12 deletions pkg/interceptors/cel/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 1 addition & 5 deletions pkg/interceptors/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/interceptors/gitlab/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
8 changes: 2 additions & 6 deletions pkg/interceptors/interceptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,15 @@ 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 {
return secretValue.([]byte), nil
}
}

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
}
Expand Down
1 change: 0 additions & 1 deletion pkg/interceptors/interceptors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func makeSecretRef() triggersv1.SecretRef {
return triggersv1.SecretRef{
SecretKey: "token",
SecretName: "test-secret",
Namespace: testNS,
}
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/sink/sink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ func TestHandleEventWithInterceptors(t *testing.T) {
SecretRef: &triggersv1.SecretRef{
SecretKey: "secretKey",
SecretName: "secret",
Namespace: namespace,
},
EventTypes: []string{"pull_request"},
},
Expand Down Expand Up @@ -997,7 +996,6 @@ func TestHandleEventWithInterceptorsAndTriggerAuth(t *testing.T) {
SecretRef: &triggersv1.SecretRef{
SecretKey: "secretKey",
SecretName: "secret",
Namespace: namespace,
},
EventTypes: []string{"pull_request"},
},
Expand Down Expand Up @@ -1085,7 +1083,6 @@ func TestHandleEventWithBitbucketInterceptors(t *testing.T) {
SecretRef: &triggersv1.SecretRef{
SecretKey: "secretKey",
SecretName: "secret",
Namespace: namespace,
},
EventTypes: []string{"repo:refs_changed"},
},
Expand Down

0 comments on commit 5bae96d

Please sign in to comment.