Fix event being dropped on error (spiffe#5030)

* Fix event being dropped on error Signed-off-by: Faisal Memon <fymemon@yahoo.com> * Fix missing event id bump Signed-off-by: Faisal Memon <fymemon@yahoo.com> --------- Signed-off-by: Faisal Memon <fymemon@yahoo.com> Co-authored-by: Andrew Harding <azdagron@gmail.com> Co-authored-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
rushi47 · Apr 11, 2024 · 611c591 · 611c591
1 parent 98cb26d
commit 611c591
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 1 deletion.
diff --git a/pkg/server/endpoints/authorized_entryfetcher.go b/pkg/server/endpoints/authorized_entryfetcher.go
@@ -160,10 +160,10 @@ func (a *AuthorizedEntryFetcherWithEventsBasedCache) updateAttestedNodesCache(ct
 		if err != nil {
 			return err
 		}
-		a.lastAttestedNodeEventID = event.EventID
 
 		if node == nil {
 			a.cache.RemoveAgent(event.SpiffeID)
+			a.lastAttestedNodeEventID = event.EventID
 			continue
 		}
 
@@ -176,10 +176,12 @@ func (a *AuthorizedEntryFetcherWithEventsBasedCache) updateAttestedNodesCache(ct
 		agentExpiresAt := time.Unix(node.CertNotAfter, 0)
 		if agentExpiresAt.Before(a.clk.Now()) {
 			a.cache.RemoveAgent(event.SpiffeID)
+			a.lastAttestedNodeEventID = event.EventID
 			continue
 		}
 
 		a.cache.UpdateAgent(node.SpiffeId, agentExpiresAt, api.ProtoFromSelectors(node.Selectors))
+		a.lastAttestedNodeEventID = event.EventID
 	}
 
 	return nil

diff --git a/pkg/server/endpoints/authorized_entryfetcher_test.go b/pkg/server/endpoints/authorized_entryfetcher_test.go
@@ -187,3 +187,65 @@ func TestBuildRegistrationEntriesCache(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateAttestedNodesCache(t *testing.T) {
+	ctx := context.Background()
+	log, _ := test.NewNullLogger()
+	clk := clock.NewMock(t)
+	ds := fakedatastore.New(t)
+
+	ef, err := NewAuthorizedEntryFetcherWithEventsBasedCache(ctx, log, clk, ds, defaultCacheReloadInterval, defaultPruneEventsOlderThan)
+	require.NoError(t, err)
+	require.NotNil(t, ef)
+
+	agentID, err := spiffeid.FromString("spiffe://example.org/myagent")
+	require.NoError(t, err)
+
+	_, err = ds.CreateAttestedNode(ctx, &common.AttestedNode{
+		SpiffeId:     agentID.String(),
+		CertNotAfter: time.Now().Add(5 * time.Hour).Unix(),
+	})
+	require.NoError(t, err)
+
+	for _, tt := range []struct {
+		name                            string
+		errs                            []error
+		expectedLastAttestedNodeEventID uint
+	}{
+		{
+			name:                            "Error Listing Attested Node Events",
+			errs:                            []error{errors.New("listing attested node events")},
+			expectedLastAttestedNodeEventID: uint(0),
+		},
+		{
+			name:                            "Error Fetching Attested Node",
+			errs:                            []error{nil, errors.New("fetching attested node")},
+			expectedLastAttestedNodeEventID: uint(0),
+		},
+		{
+			name:                            "Error Getting Node Selectors",
+			errs:                            []error{nil, nil, errors.New("getting node selectors")},
+			expectedLastAttestedNodeEventID: uint(0),
+		},
+		{
+			name:                            "No Errors",
+			expectedLastAttestedNodeEventID: uint(1),
+		},
+	} {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			for _, err = range tt.errs {
+				ds.AppendNextError(err)
+			}
+
+			err = ef.updateAttestedNodesCache(ctx)
+			if len(tt.errs) > 0 {
+				assert.EqualError(t, err, tt.errs[len(tt.errs)-1].Error())
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, tt.expectedLastAttestedNodeEventID, ef.lastAttestedNodeEventID)
+		})
+	}
+}