From 72d1dc584c5df86bff4b315752e8579e522840dd Mon Sep 17 00:00:00 2001 From: jagathprakash <31057312+jagathprakash@users.noreply.github.com> Date: Fri, 5 May 2023 10:46:16 -0400 Subject: [PATCH] This PR injects the spireControllerAPIClient into the pipelines controller and the taskrun reconciler. It makes it available in these objects to be used for signing and verification of the taskrunResults and the taskrun object itself. Before this change the spireAPIController object was not injected into the taskRun and as such SPIRE was not available to be used. After this change, - spireApiController will be available to be used by the pipeline controller and the taskrun object. - The spireApiController will be update with the spire config whenever the config changes. This commit is part of a series of PRs to implement TEP-0089. The implementation of TEP-0089 is tracked in the issue https://github.com/tektoncd/pipeline/issues/6597.[TEP-0089] SPIRE for non-falsifiable provenance. Inject SpireControllerAPIClient into the controller and the taskrun reconciler. This commit is part of a series of PRs to implement TEP-0089. The implementation of TEP-0089 is tracked in the issue [#6597](https://github.com/tektoncd/pipeline/issues/6597). --- pkg/reconciler/taskrun/controller.go | 5 ++++- pkg/reconciler/taskrun/taskrun.go | 2 ++ pkg/spire/controller.go | 26 +++++++++++++++++++++++++- pkg/spire/spire_mock.go | 3 +++ pkg/spire/spire_test.go | 27 +++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/taskrun/controller.go b/pkg/reconciler/taskrun/controller.go index 040974f53e9..8ce8db753df 100644 --- a/pkg/reconciler/taskrun/controller.go +++ b/pkg/reconciler/taskrun/controller.go @@ -32,6 +32,7 @@ import ( cloudeventclient "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" resolution "github.com/tektoncd/pipeline/pkg/resolution/resource" + "github.com/tektoncd/pipeline/pkg/spire" "github.com/tektoncd/pipeline/pkg/taskrunmetrics" "go.opentelemetry.io/otel/trace" "k8s.io/client-go/tools/cache" @@ -55,7 +56,8 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock, tracerProvi limitrangeInformer := limitrangeinformer.Get(ctx) verificationpolicyInformer := verificationpolicyinformer.Get(ctx) resolutionInformer := resolutioninformer.Get(ctx) - configStore := config.NewStore(logger.Named("config-store"), taskrunmetrics.MetricsOnStore(logger)) + spireClient := spire.GetControllerAPIClient(ctx) + configStore := config.NewStore(logger.Named("config-store"), taskrunmetrics.MetricsOnStore(logger), spire.OnStore(ctx, logger)) configStore.WatchConfigs(cmw) entrypointCache, err := pod.NewEntrypointCache(kubeclientset) @@ -68,6 +70,7 @@ func NewController(opts *pipeline.Options, clock clock.PassiveClock, tracerProvi PipelineClientSet: pipelineclientset, Images: opts.Images, Clock: clock, + spireClient: spireClient, taskRunLister: taskRunInformer.Lister(), limitrangeLister: limitrangeInformer.Lister(), verificationPolicyLister: verificationpolicyInformer.Lister(), diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index bbcf5436553..7d20b39878f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -45,6 +45,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" "github.com/tektoncd/pipeline/pkg/remote" resolution "github.com/tektoncd/pipeline/pkg/resolution/resource" + "github.com/tektoncd/pipeline/pkg/spire" "github.com/tektoncd/pipeline/pkg/taskrunmetrics" _ "github.com/tektoncd/pipeline/pkg/taskrunmetrics/fake" // Make sure the taskrunmetrics are setup "github.com/tektoncd/pipeline/pkg/trustedresources" @@ -78,6 +79,7 @@ type Reconciler struct { Clock clock.PassiveClock // listers index properties about resources + spireClient spire.ControllerAPIClient taskRunLister listers.TaskRunLister limitrangeLister corev1Listers.LimitRangeLister podLister corev1Listers.PodLister diff --git a/pkg/spire/controller.go b/pkg/spire/controller.go index 410c9c2ad63..e26275491ff 100644 --- a/pkg/spire/controller.go +++ b/pkg/spire/controller.go @@ -27,8 +27,10 @@ import ( "github.com/spiffe/go-spiffe/v2/workloadapi" entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1" spiffetypes "github.com/spiffe/spire-api-sdk/proto/spire/api/types" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" spireconfig "github.com/tektoncd/pipeline/pkg/spire/config" + "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" @@ -45,6 +47,22 @@ func init() { // controllerKey is a way to associate the ControllerAPIClient from inside the context.Context type controllerKey struct{} +// OnStore stores the changed spire config into the SpireClientApi +func OnStore(ctx context.Context, logger *zap.SugaredLogger) func(name string, value interface{}) { + return func(name string, value interface{}) { + if name == config.GetSpireConfigName() { + cfg, ok := value.(*spireconfig.SpireConfig) + if !ok { + logger.Error("Failed to do type assertion for extracting SPIRE config") + return + } + controllerAPIClient := GetControllerAPIClient(ctx) + controllerAPIClient.Close() + controllerAPIClient.SetConfig(*cfg) + } + } +} + // GetControllerAPIClient extracts the ControllerAPIClient from the context. func GetControllerAPIClient(ctx context.Context) ControllerAPIClient { untyped := ctx.Value(controllerKey{}) @@ -52,7 +70,7 @@ func GetControllerAPIClient(ctx context.Context) ControllerAPIClient { logging.FromContext(ctx).Errorf("Unable to fetch client from context.") return nil } - return untyped.(*spireControllerAPIClient) + return untyped.(ControllerAPIClient) } func withControllerClient(ctx context.Context, cfg *rest.Config) context.Context { @@ -67,6 +85,8 @@ type spireControllerAPIClient struct { workloadAPI *workloadapi.Client } +var _ ControllerAPIClient = (*spireControllerAPIClient)(nil) + func (sc *spireControllerAPIClient) setupClient(ctx context.Context) error { if sc.config == nil { return errors.New("config has not been set yet") @@ -297,18 +317,22 @@ func (sc *spireControllerAPIClient) Close() error { if err != nil { return err } + sc.serverConn = nil } if sc.workloadAPI != nil { err = sc.workloadAPI.Close() if err != nil { return err } + sc.workloadAPI = nil } if sc.workloadConn != nil { err = sc.workloadConn.Close() if err != nil { return err } + sc.workloadConn = nil } + sc.entryClient = nil return nil } diff --git a/pkg/spire/spire_mock.go b/pkg/spire/spire_mock.go index 49e0571fca2..ad96e9db657 100644 --- a/pkg/spire/spire_mock.go +++ b/pkg/spire/spire_mock.go @@ -77,6 +77,9 @@ type MockClient struct { SignOverride func(ctx context.Context, results []result.RunResult) ([]result.RunResult, error) } +var _ ControllerAPIClient = (*MockClient)(nil) +var _ EntrypointerAPIClient = (*MockClient)(nil) + const controllerSvid = "CONTROLLER_SVID_DATA" func (*MockClient) mockSign(content, signedBy string) string { diff --git a/pkg/spire/spire_test.go b/pkg/spire/spire_test.go index 42675d6d9f1..aa1d94bb97b 100644 --- a/pkg/spire/spire_test.go +++ b/pkg/spire/spire_test.go @@ -23,14 +23,17 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/spiffe/go-spiffe/v2/spiffeid" "github.com/spiffe/go-spiffe/v2/svid/x509svid" + pconf "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/pkg/result" "github.com/tektoncd/pipeline/pkg/spire/config" "github.com/tektoncd/pipeline/pkg/spire/test" "github.com/tektoncd/pipeline/pkg/spire/test/fakeworkloadapi" + "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/logging" ) @@ -666,6 +669,30 @@ func TestTaskRunResultsSignTamper(t *testing.T) { } } +func TestOnStore(t *testing.T) { + ctx, _ := ttesting.SetupDefaultContext(t) + logger := logging.FromContext(ctx) + ctx = context.WithValue(ctx, controllerKey{}, &spireControllerAPIClient{ + config: &config.SpireConfig{ + TrustDomain: "before_test_domain", + SocketPath: "before_test_socket_path", + ServerAddr: "before_test_server_path", + NodeAliasPrefix: "before_test_node_alias_prefix", + }, + }) + want := config.SpireConfig{ + TrustDomain: "after_test_domain", + SocketPath: "after_test_socket_path", + ServerAddr: "after_test_server_path", + NodeAliasPrefix: "after_test_node_alias_prefix", + } + OnStore(ctx, logger)(pconf.GetSpireConfigName(), &want) + got := *GetControllerAPIClient(ctx).(*spireControllerAPIClient).config + if d := cmp.Diff(want, got); d != "" { + t.Fatalf("Diff in TestOnStore, diff: %s", diff.PrintWantGot(d)) + } +} + func x509svids(ca *test.CA, ids ...spiffeid.ID) []*x509svid.SVID { svids := []*x509svid.SVID{} for _, id := range ids {