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 1 commit
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
Next Next commit
add x509svid, jwtsvid, svdistorex509svid count
Signed-off-by: FedeNQ <fedenahuel07@gmail.com>
  • Loading branch information
FedeNQ committed Aug 2, 2024
commit a4fe2fb2b29739d35e281373798051d0912aa55d
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,13 +16,17 @@
svids map[string]*client.JWTSVID
}

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

func NewJWTSVIDCache() *JWTSVIDCache {
return &JWTSVIDCache{
svids: make(map[string]*client.JWTSVID),
}
}

func (c *JWTSVIDCache) GetJWTSVID(spiffeID spiffeid.ID, audience []string) (*client.JWTSVID, bool) {

Check failure on line 29 in pkg/agent/manager/cache/jwt_cache.go

View workflow job for this annotation

GitHub Actions / lint (linux)

receiver-naming: receiver name c should be consistent with previous receiver name j for JWTSVIDCache (revive)

Check failure on line 29 in pkg/agent/manager/cache/jwt_cache.go

View workflow job for this annotation

GitHub Actions / lint (windows)

receiver-naming: receiver name c should be consistent with previous receiver name j for JWTSVIDCache (revive)
key := jwtSVIDKey(spiffeID, audience)

c.mu.Lock()
Expand All @@ -31,7 +35,7 @@
return svid, ok
}

func (c *JWTSVIDCache) SetJWTSVID(spiffeID spiffeid.ID, audience []string, svid *client.JWTSVID) {

Check failure on line 38 in pkg/agent/manager/cache/jwt_cache.go

View workflow job for this annotation

GitHub Actions / lint (linux)

receiver-naming: receiver name c should be consistent with previous receiver name j for JWTSVIDCache (revive)

Check failure on line 38 in pkg/agent/manager/cache/jwt_cache.go

View workflow job for this annotation

GitHub Actions / lint (windows)

receiver-naming: receiver name c should be consistent with previous receiver name j for JWTSVIDCache (revive)
key := jwtSVIDKey(spiffeID, audience)

c.mu.Lock()
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
30 changes: 23 additions & 7 deletions pkg/agent/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ type Manager interface {
// is no JWT cached, the manager will get one signed upstream.
FetchJWTSVID(ctx context.Context, entry *common.RegistrationEntry, audience []string) (*client.JWTSVID, error)

// CountSVIDs returns the amount of X509 SVIDs on memory
CountSVIDs() int
// CountX509SVIDs returns the amount of X509 SVIDs on memory
CountX509SVIDs() int

// CountJWTSVIDs returns the amount of JWT SVIDs on memory
CountJWTSVIDs() int

CountSVIDStoreX509SVIDs() int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add a descriptive comment here as we have for the other counts.


// GetLastSync returns the last successful rotation timestamp
GetLastSync() time.Time
Expand Down Expand Up @@ -107,8 +112,11 @@ type Cache interface {
// MatchingRegistrationEntries with given selectors
MatchingRegistrationEntries(selectors []*common.Selector) []*common.RegistrationEntry

// CountSVIDs in cache stored
CountSVIDs() int
// CountX509SVIDs in cache stored
CountX509SVIDs() int

// CountJWTSVIDs in cache stored
CountJWTSVIDs() int

// FetchWorkloadUpdate for given selectors
FetchWorkloadUpdate(selectors []*common.Selector) *cache.WorkloadUpdate
Expand Down Expand Up @@ -251,8 +259,16 @@ func (m *manager) MatchingRegistrationEntries(selectors []*common.Selector) []*c
return m.cache.MatchingRegistrationEntries(selectors)
}

func (m *manager) CountSVIDs() int {
return m.cache.CountSVIDs()
func (m *manager) CountX509SVIDs() int {
return m.cache.CountX509SVIDs()
}

func (m *manager) CountJWTSVIDs() int {
return m.cache.CountJWTSVIDs()
}

func (m *manager) CountSVIDStoreX509SVIDs() int {
return m.svidStoreCache.CountX509SVIDs()
}

// FetchWorkloadUpdates gets the latest workload update for the selectors
Expand Down Expand Up @@ -317,7 +333,7 @@ func (m *manager) runSynchronizer(ctx context.Context) error {

// Clamp the sync interval to the default value when the agent doesn't have any SVIDs cached
// AND the previous sync request succeeded
if m.cache.CountSVIDs() == 0 {
if m.cache.CountX509SVIDs() == 0 {
syncInterval = min(syncInterval, defaultSyncInterval)
}
}
Expand Down
Loading
Loading