Skip to content

Commit

Permalink
Add Secret Lister to taskrun and pipelinerun reconciler
Browse files Browse the repository at this point in the history
This commit adds Secret Lister to taskrun and pipelinerun reconciler, so
they can be used to fetch secret from caches instead of making calls to
api server.
  • Loading branch information
Yongxuanzhang committed Dec 16, 2022
1 parent 7e7d271 commit 78e0c43
Show file tree
Hide file tree
Showing 67 changed files with 4,309 additions and 612 deletions.
10 changes: 5 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ require (
k8s.io/code-generator v0.25.4
k8s.io/klog v1.0.0
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280
knative.dev/pkg v0.0.0-20221011175852-714b7630a836
knative.dev/pkg v0.0.0-20221209013515-911b435f02a1
sigs.k8s.io/yaml v1.3.0
)

Expand All @@ -45,7 +45,7 @@ require (
github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20221030203717-1711cefd7eec
github.com/letsencrypt/boulder v0.0.0-20221109233200-85aa52084eaf
github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399
k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85
k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2
)

require (
Expand Down Expand Up @@ -180,9 +180,9 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1
k8s.io/apiextensions-apiserver v0.25.2 // indirect
k8s.io/gengo v0.0.0-20220613173612-397b4ae3bce7 // indirect
k8s.io/klog/v2 v2.80.1 // indirect
k8s.io/apiextensions-apiserver v0.25.4 // indirect
k8s.io/gengo v0.0.0-20221011193443-fad74ee6edd9 // indirect
k8s.io/klog/v2 v2.80.2-0.20221028030830-9ae4992afb54 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
)
Expand Down
10 changes: 10 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/internal/computeresources/limitrange/limitrange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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
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,
Expand All @@ -31,6 +31,7 @@ import (
fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake"
fakelimitrangeinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/limitrange/fake"
fakeserviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake"
_ "knative.dev/pkg/system/testing"
)

func setupTestData(t *testing.T, serviceaccounts []corev1.ServiceAccount, limitranges []corev1.LimitRange) (context.Context, func()) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/pipelinerun/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
kubeclient "knative.dev/pkg/client/injection/kube/client"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret"
"knative.dev/pkg/logging"
)

Expand All @@ -56,6 +57,7 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
resourceInformer := resourceinformer.Get(ctx)
resolutionInformer := resolutioninformer.Get(ctx)
runInformer := runinformer.Get(ctx)
secretinformer := secretinformer.Get(ctx)
verificationpolicyInformer := verificationpolicyinformer.Get(ctx)
configStore := config.NewStore(logger.Named("config-store"), pipelinerunmetrics.MetricsOnStore(logger))
configStore.WatchConfigs(cmw)
Expand All @@ -70,6 +72,7 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
customRunLister: customRunInformer.Lister(),
runLister: runInformer.Lister(),
resourceLister: resourceInformer.Lister(),
secretLister: secretinformer.Lister(),
verificationPolicyLister: verificationpolicyInformer.Lister(),
cloudEventClient: cloudeventclient.Get(ctx),
metrics: pipelinerunmetrics.Get(ctx),
Expand Down
31 changes: 28 additions & 3 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes"
corev1Listers "k8s.io/client-go/listers/core/v1"
"k8s.io/utils/clock"
"knative.dev/pkg/apis"
"knative.dev/pkg/controller"
Expand Down Expand Up @@ -145,6 +146,7 @@ type Reconciler struct {
customRunLister listers.CustomRunLister
runLister alpha1listers.RunLister
resourceLister resourcelisters.PipelineResourceLister
secretLister corev1Listers.SecretLister
verificationPolicyLister alpha1listers.VerificationPolicyLister
cloudEventClient cloudevent.CEClient
metrics *pipelinerunmetrics.Recorder
Expand Down Expand Up @@ -191,7 +193,19 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
if err != nil {
return fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err)
}
getPipelineFunc := resources.GetVerifiedPipelineFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, vp)
getSecretFunc := func(namespace, name string) (*corev1.Secret, error) {
secrets, err := c.secretLister.Secrets(namespace).List(labels.Everything())
if err != nil {
return nil, fmt.Errorf("failed to list secrets:%v", err)
}
for _, s := range secrets {
if s.Name == name {
return s, nil
}
}
return nil, fmt.Errorf("failed to find secret:%v in namespace: %v", name, namespace)
}
getPipelineFunc := resources.GetVerifiedPipelineFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, vp, getSecretFunc)

if pr.IsDone() {
pr.SetDefaults(ctx)
Expand Down Expand Up @@ -322,8 +336,19 @@ func (c *Reconciler) resolvePipelineState(
if err != nil {
return nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %v", pr.Namespace, err)
}

fn := tresources.GetVerifiedTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp)
getSecretFunc := func(namespace, name string) (*corev1.Secret, error) {
secrets, err := c.secretLister.Secrets(namespace).List(labels.Everything())
if err != nil {
return nil, fmt.Errorf("failed to list secrets:%v", err)
}
for _, s := range secrets {
if s.Name == name {
return s, nil
}
}
return nil, fmt.Errorf("failed to find secret:%v in namespace: %v", name, namespace)
}
fn := tresources.GetVerifiedTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp, getSecretFunc)

getRunObjectFunc := func(name string) (v1beta1.RunObject, error) {
r, err := c.customRunLister.CustomRuns(pr.Namespace).Get(name)
Expand Down
5 changes: 3 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelineref.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/tektoncd/pipeline/pkg/remote/resolution"
remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
"github.com/tektoncd/pipeline/pkg/trustedresources"
"github.com/tektoncd/pipeline/pkg/trustedresources/verifier"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -101,7 +102,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien

// GetVerifiedPipelineFunc is a wrapper of GetPipelineFunc and return the function to
// verify the pipeline if resource-verification-mode is not "skip"
func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun, verificationpolicies []*v1alpha1.VerificationPolicy) rprp.GetPipeline {
func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun, verificationpolicies []*v1alpha1.VerificationPolicy, getSecretFunc verifier.GetSecret) rprp.GetPipeline {
get := GetPipelineFunc(ctx, k8s, tekton, requester, pipelineRun)
return func(context.Context, string) (v1beta1.PipelineObject, *v1beta1.ConfigSource, error) {
p, s, err := get(ctx, pipelineRun.Spec.PipelineRef.Name)
Expand All @@ -118,7 +119,7 @@ func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekt
}
logger := logging.FromContext(ctx)
if config.CheckEnforceResourceVerificationMode(ctx) || config.CheckWarnResourceVerificationMode(ctx) {
if err := trustedresources.VerifyPipeline(ctx, p, k8s, source, verificationpolicies); err != nil {
if err := trustedresources.VerifyPipeline(ctx, p, getSecretFunc, source, verificationpolicies); err != nil {
if config.CheckEnforceResourceVerificationMode(ctx) {
logger.Errorf("GetVerifiedPipelineFunc failed: %v", err)
return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrorResourceVerificationFailed, err)
Expand Down
22 changes: 15 additions & 7 deletions pkg/reconciler/pipelinerun/resources/pipelineref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,10 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) {
func TestGetVerifiedPipelineFunc_Success(t *testing.T) {
ctx := context.Background()
tektonclient := fake.NewSimpleClientset()
signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")

signer, secret, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")
getSecretFunc := func(namespace, name string) (*corev1.Secret, error) {
return secret, nil
}
unsignedPipeline := test.GetUnsignedPipeline("test-pipeline")
unsignedPipelineBytes, err := json.Marshal(unsignedPipeline)
if err != nil {
Expand Down Expand Up @@ -577,7 +579,7 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) {
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
ctx = test.SetupTrustedResourceConfig(ctx, tc.resourceVerificationMode)
fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps)
fn := resources.GetVerifiedPipelineFunc(ctx, fakek8s.NewSimpleClientset(), tektonclient, tc.requester, &tc.pipelinerun, vps, getSecretFunc)

resolvedPipeline, source, err := fn(ctx, pipelineRef.Name)
if err != nil {
Expand All @@ -596,7 +598,10 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) {
func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) {
ctx := context.Background()
tektonclient := fake.NewSimpleClientset()
signer, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")
signer, secret, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")
getSecretFunc := func(namespace, name string) (*corev1.Secret, error) {
return secret, nil
}

unsignedPipeline := test.GetUnsignedPipeline("test-pipeline")
unsignedPipelineBytes, err := json.Marshal(unsignedPipeline)
Expand Down Expand Up @@ -654,7 +659,7 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) {
ServiceAccountName: "default",
},
}
fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, pr, vps)
fn := resources.GetVerifiedPipelineFunc(ctx, fakek8s.NewSimpleClientset(), tektonclient, tc.requester, pr, vps, getSecretFunc)

resolvedPipeline, source, err := fn(ctx, pipelineRef.Name)
if !errors.Is(err, tc.expectedErr) {
Expand All @@ -673,7 +678,10 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) {
func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) {
ctx := context.Background()
tektonclient := fake.NewSimpleClientset()
_, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")
_, secret, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources")
getSecretFunc := func(namespace, name string) (*corev1.Secret, error) {
return secret, nil
}

unsignedPipeline := test.GetUnsignedPipeline("test-pipeline")
unsignedPipelineBytes, err := json.Marshal(unsignedPipeline)
Expand Down Expand Up @@ -747,7 +755,7 @@ func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) {
store.OnConfigChanged(featureflags)
ctx = store.ToContext(ctx)

fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps)
fn := resources.GetVerifiedPipelineFunc(ctx, fakek8s.NewSimpleClientset(), tektonclient, tc.requester, &tc.pipelinerun, vps, getSecretFunc)

_, _, err = fn(ctx, tc.pipelinerun.Spec.PipelineRef.Name)
if d := cmp.Diff(err.Error(), tc.expectedErr.Error()); d != "" {
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/taskrun/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
filteredpodinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/pod/filtered"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret"
"knative.dev/pkg/logging"
)

Expand All @@ -54,6 +55,7 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
podInformer := filteredpodinformer.Get(ctx, v1beta1.ManagedByLabelKey)
resourceInformer := resourceinformer.Get(ctx)
limitrangeInformer := limitrangeinformer.Get(ctx)
secretinformer := secretinformer.Get(ctx)
verificationpolicyInformer := verificationpolicyinformer.Get(ctx)
resolutionInformer := resolutioninformer.Get(ctx)
configStore := config.NewStore(logger.Named("config-store"), taskrunmetrics.MetricsOnStore(logger))
Expand All @@ -72,6 +74,7 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock) func(contex
taskRunLister: taskRunInformer.Lister(),
resourceLister: resourceInformer.Lister(),
limitrangeLister: limitrangeInformer.Lister(),
secretLister: secretinformer.Lister(),
verificationPolicyLister: verificationpolicyInformer.Lister(),
cloudEventClient: cloudeventclient.Get(ctx),
metrics: taskrunmetrics.Get(ctx),
Expand Down
9 changes: 5 additions & 4 deletions pkg/reconciler/taskrun/resources/taskref.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/tektoncd/pipeline/pkg/remote/resolution"
remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
"github.com/tektoncd/pipeline/pkg/trustedresources"
"github.com/tektoncd/pipeline/pkg/trustedresources/verifier"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
Expand All @@ -58,7 +59,7 @@ func GetTaskKind(taskrun *v1beta1.TaskRun) v1beta1.TaskKind {
// also requires a kubeclient, tektonclient, namespace, and service account in case it needs to find that task in
// cluster or authorize against an external repositroy. It will figure out whether it needs to look in the cluster or in
// a remote image to fetch the reference. It will also return the "kind" of the task being referenced.
func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, taskrun *v1beta1.TaskRun, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask {
func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, taskrun *v1beta1.TaskRun, verificationpolicies []*v1alpha1.VerificationPolicy, getSecretFunc verifier.GetSecret) GetTask {
// if the spec is already in the status, do not try to fetch it again, just use it as source of truth.
// Same for the Source field in the Status.Provenance.
if taskrun.Status.TaskSpec != nil {
Expand All @@ -76,13 +77,13 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto
}, configsource, nil
}
}
return GetVerifiedTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationpolicies)
return GetVerifiedTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationpolicies, getSecretFunc)
}

// GetVerifiedTaskFunc is a wrapper of GetTaskFunc and return the function to
// verify the task if resource-verification-mode is not "skip"
func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester,
owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask {
owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy, getSecretFunc verifier.GetSecret) GetTask {
get := GetTaskFunc(ctx, k8s, tekton, requester, owner, taskref, trName, namespace, saName)

return func(context.Context, string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
Expand All @@ -96,7 +97,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
}
logger := logging.FromContext(ctx)
if config.CheckEnforceResourceVerificationMode(ctx) || config.CheckWarnResourceVerificationMode(ctx) {
if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil {
if err := trustedresources.VerifyTask(ctx, t, getSecretFunc, source, verificationpolicies); err != nil {
if config.CheckEnforceResourceVerificationMode(ctx) {
logger.Errorf("GetVerifiedTaskFunc failed: %v", err)
return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrorResourceVerificationFailed, err)
Expand Down
Loading

0 comments on commit 78e0c43

Please sign in to comment.