Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVID count update #5352

Merged
merged 4 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pkg/agent/api/debug/v1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,13 @@ func (s *Service) GetInfo(context.Context, *debugv1.GetInfoRequest) (*debugv1.Ge
// Reset clock and set current response
s.getInfoResp.ts = s.clock.Now()
s.getInfoResp.resp = &debugv1.GetInfoResponse{
SvidChain: svidChain,
Uptime: int32(s.uptime().Seconds()),
SvidsCount: int32(s.m.CountSVIDs()),
LastSyncSuccess: s.m.GetLastSync().UTC().Unix(),
SvidChain: svidChain,
Uptime: int32(s.uptime().Seconds()),
SvidsCount: int32(s.m.CountX509SVIDs()),
CachedX509SvidsCount: int32(s.m.CountX509SVIDs()),
CachedJwtSvidsCount: int32(s.m.CountJWTSVIDs()),
CachedSvidstoreX509SvidsCount: int32(s.m.CountSVIDStoreX509SVIDs()),
LastSyncSuccess: s.m.GetLastSync().UTC().Unix(),
}
}

Expand Down
96 changes: 66 additions & 30 deletions pkg/agent/api/debug/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,45 +99,62 @@ func TestGetInfo(t *testing.T) {
expectResp *debugv1.GetInfoResponse
expectedLogs []spiretest.LogEntry
// Time to add to clock.Mock
addToClk time.Duration
initCache bool
lastSync time.Time
svidCount int
svidState svid.State
addToClk time.Duration
initCache bool
lastSync time.Time
svidCount int
x509SvidCount int
jwtSvidCount int
svidstoreX509SvidCount int
svidState svid.State
}{
{
name: "svid without intermediate",
lastSync: lastSync,
svidState: x509SVIDState,
svidCount: 123,
name: "svid without intermediate",
lastSync: lastSync,
svidState: x509SVIDState,
svidCount: 123,
x509SvidCount: 123,
jwtSvidCount: 123,
svidstoreX509SvidCount: 123,
expectResp: &debugv1.GetInfoResponse{
LastSyncSuccess: lastSync.UTC().Unix(),
SvidsCount: 123,
SvidChain: x509SVIDChain,
LastSyncSuccess: lastSync.UTC().Unix(),
SvidChain: x509SVIDChain,
SvidsCount: 123,
CachedX509SvidsCount: 123,
CachedJwtSvidsCount: 123,
CachedSvidstoreX509SvidsCount: 123,
},
},
{
name: "svid with intermediate",
lastSync: lastSync,
svidState: stateWithIntermediate,
svidCount: 456,
name: "svid with intermediate",
lastSync: lastSync,
svidState: stateWithIntermediate,
svidCount: 456,
x509SvidCount: 456,
jwtSvidCount: 456,
svidstoreX509SvidCount: 456,
expectResp: &debugv1.GetInfoResponse{
LastSyncSuccess: lastSync.UTC().Unix(),
SvidsCount: 456,
SvidChain: svidWithIntermediateChain,
LastSyncSuccess: lastSync.UTC().Unix(),
SvidChain: svidWithIntermediateChain,
SvidsCount: 456,
CachedX509SvidsCount: 456,
CachedJwtSvidsCount: 456,
CachedSvidstoreX509SvidsCount: 456,
},
},
{
name: "get response from cache",
expectResp: &debugv1.GetInfoResponse{
LastSyncSuccess: cachedLastSync.Unix(),
SvidsCount: 99999,
SvidChain: x509SVIDChain,
LastSyncSuccess: cachedLastSync.Unix(),
SvidsCount: 99999,
CachedX509SvidsCount: 99999,
SvidChain: x509SVIDChain,
},
initCache: true,
lastSync: lastSync,
svidState: stateWithIntermediate,
svidCount: 456,
initCache: true,
lastSync: lastSync,
svidState: stateWithIntermediate,
svidCount: 253,
x509SvidCount: 253,
},
{
name: "expires cache",
Expand Down Expand Up @@ -182,6 +199,7 @@ func TestGetInfo(t *testing.T) {
// Set a success state before running actual test case and expire time
if tt.initCache {
test.m.svidCount = 99999
test.m.x509SvidCount = 99999
test.m.svidState = x509SVIDState
test.m.lastSync = cachedLastSync

Expand All @@ -192,6 +210,9 @@ func TestGetInfo(t *testing.T) {
test.clk.Add(tt.addToClk)

test.m.svidCount = tt.svidCount
test.m.x509SvidCount = tt.x509SvidCount
test.m.jwtSvidCount = tt.jwtSvidCount
test.m.svidstoreX509SvidCount = tt.svidstoreX509SvidCount
test.m.svidState = tt.svidState
test.m.lastSync = tt.lastSync

Expand Down Expand Up @@ -264,10 +285,13 @@ func setupServiceTest(t *testing.T) *serviceTest {
type fakeManager struct {
manager.Manager

bundle *cache.Bundle
svidState svid.State
svidCount int
lastSync time.Time
bundle *cache.Bundle
svidState svid.State
svidCount int
x509SvidCount int
jwtSvidCount int
svidstoreX509SvidCount int
lastSync time.Time
}

func (m *fakeManager) GetCurrentCredentials() svid.State {
Expand All @@ -278,6 +302,18 @@ func (m *fakeManager) CountSVIDs() int {
return m.svidCount
}

func (m *fakeManager) CountX509SVIDs() int {
return m.x509SvidCount
}

func (m *fakeManager) CountJWTSVIDs() int {
return m.jwtSvidCount
}

func (m *fakeManager) CountSVIDStoreX509SVIDs() int {
return m.svidstoreX509SvidCount
}

func (m *fakeManager) GetLastSync() time.Time {
return m.lastSync
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/agent/manager/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (c *Cache) Identities() []Identity {
return out
}

func (c *Cache) CountSVIDs() int {
func (c *Cache) CountX509SVIDs() int {
c.mu.RLock()
defer c.mu.RUnlock()

Expand All @@ -183,6 +183,10 @@ func (c *Cache) CountSVIDs() int {
return records
}

func (c *Cache) CountJWTSVIDs() int {
return c.JWTSVIDCache.CountJWTSVIDs()
}

func (c *Cache) MatchingIdentities(selectors []*common.Selector) []Identity {
set, setDone := allocSelectorSet(selectors...)
defer setDone()
Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/manager/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ func TestCountSVIDs(t *testing.T) {
cache.UpdateEntries(updateEntries, nil)

// No SVIDs expected
require.Equal(t, 0, cache.CountSVIDs())
require.Equal(t, 0, cache.CountX509SVIDs())

updateSVIDs := &UpdateSVIDs{
X509SVIDs: makeX509SVIDs(foo),
}
cache.UpdateSVIDs(updateSVIDs)

// Only one SVID expected
require.Equal(t, 1, cache.CountSVIDs())
require.Equal(t, 1, cache.CountX509SVIDs())
}

func TestBundleChanges(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/agent/manager/cache/jwt_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ type JWTSVIDCache struct {
svids map[string]*client.JWTSVID
}

func (c *JWTSVIDCache) CountJWTSVIDs() int {
return len(c.svids)
}

func NewJWTSVIDCache() *JWTSVIDCache {
return &JWTSVIDCache{
svids: make(map[string]*client.JWTSVID),
Expand Down
11 changes: 9 additions & 2 deletions pkg/agent/manager/cache/lru_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,20 @@ func (c *LRUCache) Entries() []*common.RegistrationEntry {
return out
}

func (c *LRUCache) CountSVIDs() int {
func (c *LRUCache) CountX509SVIDs() int {
c.mu.RLock()
defer c.mu.RUnlock()

return len(c.svids)
}

func (c *LRUCache) CountJWTSVIDs() int {
c.mu.RLock()
defer c.mu.RUnlock()

return len(c.JWTSVIDCache.svids)
}

func (c *LRUCache) CountRecords() int {
c.mu.RLock()
defer c.mu.RUnlock()
Expand Down Expand Up @@ -446,7 +453,7 @@ func (c *LRUCache) UpdateEntries(update *UpdateEntries, checkSVID func(*common.R

func (c *LRUCache) UpdateSVIDs(update *UpdateSVIDs) {
c.mu.Lock()
defer func() { agentmetrics.SetSVIDMapSize(c.metrics, c.CountSVIDs()) }()
defer func() { agentmetrics.SetSVIDMapSize(c.metrics, c.CountX509SVIDs()) }()
defer c.mu.Unlock()

// Allocate a set of selectors that
Expand Down
24 changes: 12 additions & 12 deletions pkg/agent/manager/cache/lru_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@ func TestLRUCacheCountSVIDs(t *testing.T) {
cache.UpdateEntries(updateEntries, nil)

// No SVIDs expected
require.Equal(t, 0, cache.CountSVIDs())
require.Equal(t, 0, cache.CountX509SVIDs())

updateSVIDs := &UpdateSVIDs{
X509SVIDs: makeX509SVIDs(foo),
}
cache.UpdateSVIDs(updateSVIDs)

// Only one SVID expected
require.Equal(t, 1, cache.CountSVIDs())
require.Equal(t, 1, cache.CountX509SVIDs())
}

func TestLRUCacheCountRecords(t *testing.T) {
Expand Down Expand Up @@ -705,10 +705,10 @@ func TestLRUCacheSVIDCacheExpiry(t *testing.T) {
sub.Finish()
}
}
assert.Equal(t, 12, cache.CountSVIDs())
assert.Equal(t, 12, cache.CountX509SVIDs())

cache.UpdateEntries(updateEntries, nil)
assert.Equal(t, 10, cache.CountSVIDs())
assert.Equal(t, 10, cache.CountX509SVIDs())

// foo SVID should be removed from cache as it does not have active subscriber
assert.False(t, cache.notifySubscriberIfSVIDAvailable(makeSelectors("A"), subA.(*lruCacheSubscriber)))
Expand All @@ -724,7 +724,7 @@ func TestLRUCacheSVIDCacheExpiry(t *testing.T) {
require.Len(t, cache.GetStaleEntries(), 1)
assert.Equal(t, foo, cache.GetStaleEntries()[0].Entry)

assert.Equal(t, 10, cache.CountSVIDs())
assert.Equal(t, 10, cache.CountX509SVIDs())
}

func TestLRUCacheMaxSVIDCacheSize(t *testing.T) {
Expand All @@ -741,7 +741,7 @@ func TestLRUCacheMaxSVIDCacheSize(t *testing.T) {
X509SVIDs: makeX509SVIDsFromStaleEntries(cache.GetStaleEntries()),
})
require.Len(t, cache.GetStaleEntries(), 0)
assert.Equal(t, 10, cache.CountSVIDs())
assert.Equal(t, 10, cache.CountX509SVIDs())

// Validate that active subscriber will still get SVID even if SVID count is at maxSvidCacheSize
foo := makeRegistrationEntry("FOO", "A")
Expand All @@ -752,12 +752,12 @@ func TestLRUCacheMaxSVIDCacheSize(t *testing.T) {

cache.UpdateEntries(updateEntries, nil)
require.Len(t, cache.GetStaleEntries(), 1)
assert.Equal(t, 10, cache.CountSVIDs())
assert.Equal(t, 10, cache.CountX509SVIDs())

cache.UpdateSVIDs(&UpdateSVIDs{
X509SVIDs: makeX509SVIDs(foo),
})
assert.Equal(t, 11, cache.CountSVIDs())
assert.Equal(t, 11, cache.CountX509SVIDs())
require.Len(t, cache.GetStaleEntries(), 0)
}

Expand All @@ -770,7 +770,7 @@ func TestSyncSVIDsWithSubscribers(t *testing.T) {
cache.UpdateSVIDs(&UpdateSVIDs{
X509SVIDs: makeX509SVIDsFromStaleEntries(cache.GetStaleEntries()),
})
assert.Equal(t, 5, cache.CountSVIDs())
assert.Equal(t, 5, cache.CountX509SVIDs())

// Update foo but its SVID is not yet cached
foo := makeRegistrationEntry("FOO", "A")
Expand All @@ -788,7 +788,7 @@ func TestSyncSVIDsWithSubscribers(t *testing.T) {
require.Len(t, cache.GetStaleEntries(), 1)
assert.Equal(t, []*StaleEntry{{Entry: cache.records[foo.EntryId].entry}}, cache.GetStaleEntries())

assert.Equal(t, 5, cache.CountSVIDs())
assert.Equal(t, 5, cache.CountX509SVIDs())
}

func TestNotifySubscriberWhenSVIDIsAvailable(t *testing.T) {
Expand Down Expand Up @@ -863,7 +863,7 @@ func TestSubscribeToWorkloadUpdatesLRUNoSelectors(t *testing.T) {
cache.UpdateSVIDs(&UpdateSVIDs{
X509SVIDs: makeX509SVIDs(foo, bar),
})
assert.Equal(t, 2, cache.CountSVIDs())
assert.Equal(t, 2, cache.CountX509SVIDs())

select {
case err := <-subErrCh:
Expand Down Expand Up @@ -932,7 +932,7 @@ func TestSubscribeToLRUCacheChanges(t *testing.T) {
cache.UpdateSVIDs(&UpdateSVIDs{
X509SVIDs: makeX509SVIDs(foo, bar),
})
assert.Equal(t, 2, cache.CountSVIDs())
assert.Equal(t, 2, cache.CountX509SVIDs())

clk.WaitForAfter(time.Second, "waiting for after to get called")
clk.Add(SVIDSyncInterval * 4)
Expand Down
Loading
Loading