Skip to content

Commit

Permalink
Use created_at field to determine if entry is stale
Browse files Browse the repository at this point in the history
Signed-off-by: Sorin Dumitru <[email protected]>
  • Loading branch information
sorindumitru committed Jan 9, 2025
1 parent 4b9c1e2 commit 83b6633
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 20 deletions.
22 changes: 20 additions & 2 deletions pkg/agent/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var (
RevisionNumber: true,
StoreSvid: true,
Hint: true,
CreatedAt: true,
}
)

Expand Down Expand Up @@ -416,6 +417,23 @@ func (c *client) syncEntries(ctx context.Context, cachedEntries map[string]*comm
return stats, nil
}

func entryIsStale(entry *common.RegistrationEntry, revisionNumber, revisionCreatedAt int64) bool {
if entry.RevisionNumber != revisionNumber {
return true
}

// TOOD: remove in SPIRE 1.14
if revisionCreatedAt == 0 {
return false
}

if entry.CreatedAt != revisionCreatedAt {
return true
}

return false
}

func (c *client) streamAndSyncEntries(ctx context.Context, entryClient entryv1.EntryClient, cachedEntries map[string]*common.RegistrationEntry) (stats SyncEntriesStats, err error) {
// Build a set of all the entries to be removed. This set is initialized
// with all entries currently known. As entries are synced down from the
Expand Down Expand Up @@ -459,7 +477,7 @@ func (c *client) streamAndSyncEntries(ctx context.Context, entryClient entryv1.E
// If entry is either not cached or is stale, record the ID so
// the full entry can be requested after syncing down all
// entry revisions.
if cachedEntry, ok := cachedEntries[entryRevision.Id]; !ok || cachedEntry.RevisionNumber < entryRevision.RevisionNumber {
if cachedEntry, ok := cachedEntries[entryRevision.Id]; !ok || entryIsStale(cachedEntry, entryRevision.GetRevisionNumber(), entryRevision.GetCreatedAt()) {

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / unit-test (macos-latest)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / unit-test (ubuntu-22.04)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / lint (linux)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)) (typecheck)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / lint (linux)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)) (typecheck)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / lint (linux)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)) (typecheck)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / unit-test (linux with race detection)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)

Check failure on line 480 in pkg/agent/client/client.go

View workflow job for this annotation

GitHub Actions / unit-test (windows)

entryRevision.GetCreatedAt undefined (type *entryv1.EntryRevision has no field or method GetCreatedAt)
needFull = append(needFull, entryRevision.Id)
}
}
Expand Down Expand Up @@ -487,7 +505,7 @@ func (c *client) streamAndSyncEntries(ctx context.Context, entryClient entryv1.E
switch {
case !ok:
stats.Missing++
case cachedEntry.RevisionNumber < entry.RevisionNumber:
case entryIsStale(cachedEntry, entry.GetRevisionNumber(), entry.GetCreatedAt()):
stats.Stale++
}

Expand Down
39 changes: 21 additions & 18 deletions pkg/agent/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,19 @@ func TestSyncUpdatesEntries(t *testing.T) {
assert.Equal(t, expected, cachedEntries)
}

entryA1 := makeEntry("A", 1)
entryB1 := makeEntry("B", 1)
entryC1 := makeEntry("C", 1)
entryD1 := makeEntry("D", 1)
firstDate := time.Date(2024, time.December, 31, 0, 0, 0, 0, time.UTC)
secondDate := time.Date(2025, time.January, 1, 0, 0, 0, 0, time.UTC)

entryA2 := makeEntry("A", 2)
entryB2 := makeEntry("B", 2)
entryC2 := makeEntry("C", 2)
entryA1 := makeEntry("A", 1, firstDate)
entryB1 := makeEntry("B", 1, firstDate)
entryC1 := makeEntry("C", 1, firstDate)
entryD1 := makeEntry("D", 1, firstDate)

entryA2 := makeEntry("A", 2, firstDate)
entryB2 := makeEntry("B", 2, firstDate)
entryC2 := makeEntry("C", 2, firstDate)

entryB1prime := makeEntry("B", 1, secondDate)

// No entries yet
syncAndAssertEntries(t, 0, 0, 0, 0)
Expand All @@ -314,6 +319,12 @@ func TestSyncUpdatesEntries(t *testing.T) {

// Sync again but with no changes.
syncAndAssertEntries(t, 3, 0, 0, 0, entryA2, entryB2, entryC2)

// Sync again after recreating an entry with the same entry ID, which should be marked stale
syncAndAssertEntries(t, 3, 0, 1, 0, entryA2, entryB1prime, entryC2)

// Sync again after the database has been rolled back to a previous version
syncAndAssertEntries(t, 4, 1, 3, 0, entryA1, entryB1, entryC1, entryD1)
}

func TestRenewSVID(t *testing.T) {
Expand Down Expand Up @@ -1185,16 +1196,7 @@ type testServer struct {
}

func checkAuthorizedEntryOutputMask(outputMask *types.EntryMask) error {
if diff := cmp.Diff(outputMask, &types.EntryMask{
SpiffeId: true,
Selectors: true,
FederatesWith: true,
Admin: true,
Downstream: true,
RevisionNumber: true,
StoreSvid: true,
Hint: true,
}, protocmp.Transform()); diff != "" {
if diff := cmp.Diff(outputMask, entryOutputMask, protocmp.Transform()); diff != "" {
return status.Errorf(codes.InvalidArgument, "invalid output mask requested: %s", diff)
}
return nil
Expand All @@ -1214,12 +1216,13 @@ func makeCommonBundle(trustDomainName string) *common.Bundle {
}
}

func makeEntry(id string, revisionNumber int64) *types.Entry {
func makeEntry(id string, revisionNumber int64, createdAt time.Time) *types.Entry {
return &types.Entry{
Id: id,
SpiffeId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/workload"},
ParentId: &types.SPIFFEID{TrustDomain: "example.org", Path: "/agent"},
Selectors: []*types.Selector{{Type: "not", Value: "relevant"}},
RevisionNumber: revisionNumber,
CreatedAt: createdAt.Unix(),
}
}
1 change: 1 addition & 0 deletions pkg/agent/client/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,6 @@ func slicedEntryFromProto(e *types.Entry) (*common.RegistrationEntry, error) {
Admin: e.Admin,
Downstream: e.Downstream,
Hint: e.Hint,
CreatedAt: e.CreatedAt,
}, nil
}
1 change: 1 addition & 0 deletions pkg/server/api/entry/v1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ func SyncAuthorizedEntries(stream entryv1.Entry_SyncAuthorizedEntriesServer, ent
for j, entry := range entries[i : i+n] {
entryRevisions[j].Id = entry.Id
entryRevisions[j].RevisionNumber = entry.RevisionNumber
entryRevisions[j].CreatedAt = entry.CreatedAt

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / unit-test (macos-latest)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / unit-test (ubuntu-22.04)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / lint (linux)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt) (typecheck)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / lint (linux)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)) (typecheck)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / artifacts (windows)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / unit-test (linux with race detection)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)

Check failure on line 479 in pkg/server/api/entry/v1/service.go

View workflow job for this annotation

GitHub Actions / unit-test (windows)

entryRevisions[j].CreatedAt undefined (type *entryv1.EntryRevision has no field or method CreatedAt)
}

if err := stream.Send(&entryv1.SyncAuthorizedEntriesResponse{
Expand Down
1 change: 1 addition & 0 deletions pkg/server/api/entry/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3142,6 +3142,7 @@ func TestSyncAuthorizedEntries(t *testing.T) {
ParentId: entry1.ParentId,
Selectors: entry1.Selectors,
RevisionNumber: entry1.RevisionNumber,
CreatedAt: entry1.CreatedAt,
},
{
Id: entry2.Id,
Expand Down

0 comments on commit 83b6633

Please sign in to comment.