From 4f00e5169baa47561267db2dc93c4f043f34e3af 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] [TEP-0089] Inject SpireControllerAPIClient into the Taskrun
 controller and reconciler

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 {