Fix racy spire upstreamauthority plugin tests (#5468)

The mock clock was not being threaded through everywhere causing sporadic NotAfter related assertions to fail. Signed-off-by: Andrew Harding <azdagron@gmail.com>
spiffe · Sep 5, 2024 · 39982e6 · 39982e6
1 parent 7118533
commit 39982e6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 18 deletions.
diff --git a/pkg/server/plugin/upstreamauthority/spire/fake_handlers_test.go b/pkg/server/plugin/upstreamauthority/spire/fake_handlers_test.go
@@ -66,10 +66,10 @@ type testHandler struct {
 	sAPIServer *handler
 }
 
-func (h *testHandler) startTestServers(t *testing.T, ca *testca.CA, serverCert []*x509.Certificate, serverKey crypto.Signer,
+func (h *testHandler) startTestServers(t *testing.T, clk clock.Clock, ca *testca.CA, serverCert []*x509.Certificate, serverKey crypto.Signer,
 	svidCert []byte, svidKey []byte) {
 	h.wAPIServer = &whandler{cert: serverCert, key: serverKey, ca: ca, svidCert: svidCert, svidKey: svidKey}
-	h.sAPIServer = &handler{cert: serverCert, key: serverKey, ca: ca}
+	h.sAPIServer = &handler{clock: clk, cert: serverCert, key: serverKey, ca: ca}
 	h.sAPIServer.startServerAPITestServer(t)
 	h.wAPIServer.startWAPITestServer(t)
 }

diff --git a/pkg/server/plugin/upstreamauthority/spire/spire.go b/pkg/server/plugin/upstreamauthority/spire/spire.go
@@ -30,8 +30,6 @@ const (
 	internalPollFreq = time.Second
 )
 
-var clk clock.Clock = clock.New()
-
 type Configuration struct {
 	ServerAddr        string             `hcl:"server_address" json:"server_address"`
 	ServerPort        string             `hcl:"server_port" json:"server_port"`
@@ -58,6 +56,7 @@ type Plugin struct {
 	upstreamauthorityv1.UnsafeUpstreamAuthorityServer
 	configv1.UnsafeConfigServer
 
+	clk clock.Clock
 	log hclog.Logger
 
 	mtx         sync.RWMutex
@@ -78,6 +77,7 @@ type Plugin struct {
 
 func New() *Plugin {
 	return &Plugin{
+		clk:           clock.New(),
 		currentBundle: &plugintypes.Bundle{},
 	}
 }
@@ -166,7 +166,7 @@ func (p *Plugin) MintX509CAAndSubscribe(request *upstreamauthorityv1.MintX509CAR
 		return status.Errorf(codes.Internal, "unable to form response X.509 CA chain: %v", err)
 	}
 
-	ticker := clk.Ticker(internalPollFreq)
+	ticker := p.clk.Ticker(internalPollFreq)
 	defer ticker.Stop()
 	for {
 		newRootCAs := p.getBundle().X509Authorities
@@ -224,7 +224,7 @@ func (p *Plugin) PublishJWTKeyAndSubscribe(req *upstreamauthorityv1.PublishJWTKe
 	p.setBundleJWTAuthorities(jwtKeys)
 
 	keys := []*plugintypes.JWTKey{}
-	ticker := clk.Ticker(internalPollFreq)
+	ticker := p.clk.Ticker(internalPollFreq)
 	defer ticker.Stop()
 	for {
 		newKeys := p.getBundle().JwtAuthorities
@@ -248,7 +248,7 @@ func (p *Plugin) PublishJWTKeyAndSubscribe(req *upstreamauthorityv1.PublishJWTKe
 }
 
 func (p *Plugin) pollBundleUpdates(ctx context.Context) {
-	ticker := clk.Ticker(upstreamPollFreq)
+	ticker := p.clk.Ticker(upstreamPollFreq)
 	defer ticker.Stop()
 	for {
 		preFetchCallVersion := p.getBundleVersion()

diff --git a/pkg/server/plugin/upstreamauthority/spire/spire_test.go b/pkg/server/plugin/upstreamauthority/spire/spire_test.go
@@ -263,9 +263,11 @@ func TestMintX509CA(t *testing.T) {
 	for _, c := range cases {
 		c := c
 		t.Run(c.name, func(t *testing.T) {
+			mockClock := clock.NewMock(t)
+
 			// Setup servers
 			server := testHandler{}
-			server.startTestServers(t, ca, serverCert, serverKey, svidCert, svidKey)
+			server.startTestServers(t, mockClock, ca, serverCert, serverKey, svidCert, svidKey)
 			server.sAPIServer.setError(c.sAPIError)
 			server.sAPIServer.setDownstreamResponse(c.downstreamResp)
 
@@ -278,7 +280,7 @@ func TestMintX509CA(t *testing.T) {
 				workloadAPIAddr = c.customWorkloadAPIAddr
 			}
 
-			ua, mockClock := newWithDefault(t, serverAddr, workloadAPIAddr)
+			ua := newWithDefault(t, mockClock, serverAddr, workloadAPIAddr)
 			server.sAPIServer.clock = mockClock
 
 			// Send initial request and get stream
@@ -357,9 +359,10 @@ func TestPublishJWTKey(t *testing.T) {
 	require.NoError(t, err)
 
 	// Setup servers
+	mockClock := clock.NewMock(t)
 	server := testHandler{}
-	server.startTestServers(t, ca, serverCert, serverKey, svidCert, svidKey)
-	ua, mockClock := newWithDefault(t, server.sAPIServer.addr, server.wAPIServer.workloadAPIAddr)
+	server.startTestServers(t, mockClock, ca, serverCert, serverKey, svidCert, svidKey)
+	ua := newWithDefault(t, mockClock, server.sAPIServer.addr, server.wAPIServer.workloadAPIAddr)
 
 	// Get first response
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
@@ -411,27 +414,26 @@ func TestPublishJWTKey(t *testing.T) {
 	spiretest.RequireGRPCStatusHasPrefix(t, err, codes.Internal, "upstreamauthority(spire): failed to push JWT authority: rpc error: code = Unknown desc = some erro")
 }
 
-func newWithDefault(t *testing.T, serverAddr string, workloadAPIAddr net.Addr) (*upstreamauthority.V1, *clock.Mock) {
+func newWithDefault(t *testing.T, mockClock *clock.Mock, serverAddr string, workloadAPIAddr net.Addr) *upstreamauthority.V1 {
 	host, port, _ := net.SplitHostPort(serverAddr)
 	config := Configuration{
 		ServerAddr: host,
 		ServerPort: port,
 	}
 	setWorkloadAPIAddr(&config, workloadAPIAddr)
 
+	p := New()
+	p.clk = mockClock
+
 	ua := new(upstreamauthority.V1)
-	plugintest.Load(t, BuiltIn(), ua,
+	plugintest.Load(t, builtin(p), ua,
 		plugintest.CoreConfig(catalog.CoreConfig{
 			TrustDomain: trustDomain,
 		}),
 		plugintest.ConfigureJSON(config),
 	)
 
-	mockClock := clock.NewMock(t)
-
-	clk = mockClock
-
-	return ua, mockClock
+	return ua
 }
 
 func certChainURIs(chain []*x509.Certificate) []string {