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

Fetch agent selectors when refreshing event based cache #4803

Merged
merged 7 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion pkg/server/api/agent/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3189,7 +3189,7 @@ func setupServiceTest(t *testing.T, agentSVIDTTL time.Duration) *serviceTest {
ca := fakeserverca.New(t, td, &fakeserverca.Options{
AgentSVIDTTL: agentSVIDTTL,
})
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
cat := fakeservercatalog.New()
clk := clock.NewMock(t)

Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/bundle/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2894,7 +2894,7 @@ func (c *serviceTest) Cleanup() {
}

func setupServiceTest(t *testing.T) *serviceTest {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
up := new(fakeUpstreamPublisher)
rateLimiter := new(fakeRateLimiter)
service := bundle.New(bundle.Config{
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/debug/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ func (s *serviceTest) Cleanup() {

func setupServiceTest(t *testing.T) *serviceTest {
clk := clock.NewMock()
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
log, logHook := test.NewNullLogger()
log.Level = logrus.DebugLevel
fakeUptime := &fakeUptime{
Expand Down
18 changes: 9 additions & 9 deletions pkg/server/api/entry/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestCountEntries(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()

Expand Down Expand Up @@ -221,7 +221,7 @@ func TestListEntries(t *testing.T) {
}

// setup
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()

Expand Down Expand Up @@ -1219,7 +1219,7 @@ func TestListEntries(t *testing.T) {

func TestGetEntry(t *testing.T) {
now := time.Now().Unix()
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()

Expand Down Expand Up @@ -2652,7 +2652,7 @@ func TestBatchDeleteEntry(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()

Expand Down Expand Up @@ -2869,7 +2869,7 @@ func TestGetAuthorizedEntries(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
test := setupServiceTest(t, fakedatastore.New(t))
test := setupServiceTest(t, fakedatastore.New(t, false))
defer test.Cleanup()

test.omitCallerID = tt.failCallerID
Expand Down Expand Up @@ -3266,7 +3266,7 @@ func TestSyncAuthorizedEntries(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
test := setupServiceTest(t, fakedatastore.New(t))
test := setupServiceTest(t, fakedatastore.New(t, false))
defer func() {
test.Cleanup()
spiretest.AssertLogs(t, test.logHook.AllEntries(), tt.expectLogs)
Expand Down Expand Up @@ -3346,7 +3346,7 @@ func FuzzSyncAuthorizedStreams(f *testing.F) {

entries := entries[:totalEntries]

test := setupServiceTest(t, fakedatastore.New(t), withEntryPageSize(entryPageSize))
test := setupServiceTest(t, fakedatastore.New(t, false), withEntryPageSize(entryPageSize))
defer test.Cleanup()
test.ef.entries = entries

Expand Down Expand Up @@ -4598,7 +4598,7 @@ func TestBatchUpdateEntry(t *testing.T) {
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()
// Create federated bundles, that we use on "FederatesWith"
Expand Down Expand Up @@ -4798,7 +4798,7 @@ type fakeDS struct {

func newFakeDS(t *testing.T) *fakeDS {
return &fakeDS{
DataStore: fakedatastore.New(t),
DataStore: fakedatastore.New(t, false),
expectEntries: make(map[string]*common.RegistrationEntry),
results: make(map[string]*common.RegistrationEntry),
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/health/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestServiceCheck(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
log, logHook := test.NewNullLogger()

ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
if tt.dsErr != nil {
ds.SetNextError(tt.dsErr)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/localauthority/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1785,7 +1785,7 @@ func TestRevokeX509Authority(t *testing.T) {
}

func setupServiceTest(t *testing.T) *serviceTest {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
m := &fakeCAManager{}

service := localauthority.New(localauthority.Config{
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/api/svid/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2077,7 +2077,7 @@ func setupServiceTest(t *testing.T) *serviceTest {
ca := fakeserverca.New(t, trustDomain, &fakeserverca.Options{})
ef := &entryFetcher{}
downstream := &entryFetcher{}
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)

rateLimiter := &fakeRateLimiter{}
service := svid.New(svid.Config{
Expand Down
6 changes: 3 additions & 3 deletions pkg/server/api/trustdomain/v1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,7 @@ func TestBatchDeleteFederationRelationship(t *testing.T) {
},
} {
t.Run(tt.name, func(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
test := setupServiceTest(t, ds)
defer test.Cleanup()

Expand Down Expand Up @@ -2133,7 +2133,7 @@ func TestRefreshBundle(t *testing.T) {
},
} {
t.Run(tt.name, func(t *testing.T) {
test := setupServiceTest(t, fakedatastore.New(t))
test := setupServiceTest(t, fakedatastore.New(t, false))
defer test.Cleanup()

_, err := test.client.RefreshBundle(ctx, &trustdomainv1.RefreshBundleRequest{
Expand Down Expand Up @@ -2237,7 +2237,7 @@ type fakeDS struct {

func newFakeDS(t *testing.T) *fakeDS {
return &fakeDS{
DataStore: fakedatastore.New(t),
DataStore: fakedatastore.New(t, false),
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/server/bundle/client/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func newManagerTest(t *testing.T, source TrustDomainConfigSource, localBundles,
test.manager = NewManager(ManagerConfig{
Log: log,
Metrics: telemetry.Blackhole{},
DataStore: fakedatastore.New(t),
DataStore: fakedatastore.New(t, false),
Clock: test.clock,
Source: source,
newBundleUpdater: test.newBundleUpdater,
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/bundle/client/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestBundleUpdaterUpdateBundle(t *testing.T) {
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)

if testCase.localBundle != nil {
localBundleProto, err := bundleutil.SPIFFEBundleToProto(testCase.localBundle)
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/bundle/datastore/wrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestWithBundlePublisher(t *testing.T) {
tt := tt
ctx := context.Background()
t.Run(tt.name, func(t *testing.T) {
var ds datastore.DataStore = fakedatastore.New(t)
var ds datastore.DataStore = fakedatastore.New(t, false)

// We want to have at least two JWT signing keys so one can be
// pruned.
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/bundle/pubmanager/pubmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (p *fakeBundlePublisher) Type() string {
func setupTest(t *testing.T, bundlePublishers []bundlepublisher.BundlePublisher) *managerTest {
log, logHook := test.NewNullLogger()
log.Level = logrus.DebugLevel
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)

clock := clock.NewMock(t)
m, err := newManager(&ManagerConfig{
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/ca/manager/journal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func setupJournalTest(t *testing.T) *journalTest {
})
require.NoError(t, err)

ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
cat := fakeservercatalog.New()
cat.SetDataStore(ds)

Expand Down Expand Up @@ -137,7 +137,7 @@ func TestJournalPersistence(t *testing.T) {
// Test for the case when SPIRE starts with a CA journal on disk and does
// not yet have a CA journal stored in the datastore. Reset the datastore so
// we only have the CA journal on disk.
test.ds = fakedatastore.New(t)
test.ds = fakedatastore.New(t, false)
test.jc.cat.(*fakeservercatalog.Catalog).SetDataStore(test.ds)

// Load the journal again. It should still get the CA journal stored on
Expand Down
6 changes: 3 additions & 3 deletions pkg/server/ca/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ func TestPruneCAJournals(t *testing.T) {
expectedCAJournals = []*datastore.CAJournal{}
t.Run(testCase.name, func(t *testing.T) {
// Have a fresh data store in each test case
test.ds = fakedatastore.New(t)
test.ds = fakedatastore.New(t, false)
test.m.c.Catalog.(*fakeservercatalog.Catalog).SetDataStore(test.ds)

for _, j := range testCase.testJournals {
Expand Down Expand Up @@ -982,7 +982,7 @@ func setupTest(t *testing.T) *managerTest {
log, logHook := test.NewNullLogger()
metrics := fakemetrics.New()
km := fakeserverkeymanager.New(t)
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)

cat := fakeservercatalog.New()
cat.SetKeyManager(km)
Expand Down Expand Up @@ -1221,7 +1221,7 @@ func (m *managerTest) wipeJournal(t *testing.T) {
require.NoError(t, os.Remove(filepath.Join(m.m.c.Dir, "journal.pem")))

// Have a clean datastore.
m.ds = fakedatastore.New(t)
m.ds = fakedatastore.New(t, false)
m.cat.SetDataStore(m.ds)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/server/ca/manager/slot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestJournalLoad(t *testing.T) {
require.NoError(t, err)

km := fakeserverkeymanager.New(t)
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
td := spiffeid.RequireTrustDomainFromString("example.org")

cat := fakeservercatalog.New()
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/cache/dscache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestFetchBundleCache(t *testing.T) {
td := "spiffe://domain.test"
bundle1 := &common.Bundle{TrustDomainId: "spiffe://domain.test", RefreshHint: 1}
bundle2 := &common.Bundle{TrustDomainId: "spiffe://domain.test", RefreshHint: 2}
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
clock := clock.NewMock(t)
cache := New(ds, clock)
ctxWithCache := WithCache(context.Background())
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestBundleInvalidations(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
// Create datastore and cache
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
cache := New(ds, clock.NewMock(t))
ctxWithCache := WithCache(context.Background())

Expand Down
4 changes: 2 additions & 2 deletions pkg/server/cache/entrycache/fullcache_ds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func TestEntryIteratorDS(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
ctx := context.Background()

t.Run("no entries", func(t *testing.T) {
Expand Down Expand Up @@ -79,7 +79,7 @@ func TestEntryIteratorDS(t *testing.T) {
}

func TestAgentIteratorDS(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
ctx := context.Background()

t.Run("no entries", func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/server/cache/entrycache/fullcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var (
)

func TestCache(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
ctx := context.Background()

rootID := spiffeid.RequireFromString("spiffe://example.org/root")
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestCache(t *testing.T) {
}

func TestCacheReturnsClonedEntries(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)

expected, err := api.RegistrationEntryToProto(createRegistrationEntry(context.Background(), t, ds, &common.RegistrationEntry{
ParentId: "spiffe://domain.test/node",
Expand All @@ -156,7 +156,7 @@ func TestCacheReturnsClonedEntries(t *testing.T) {
}

func TestFullCacheNodeAliasing(t *testing.T) {
ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
ctx := context.Background()

const serverID = "spiffe://example.org/spire/server"
Expand Down Expand Up @@ -270,7 +270,7 @@ func TestFullCacheExcludesNodeSelectorMappedEntriesForExpiredAgents(t *testing.T
// This is a mitigation for performance problems that arise when hydrating the cache today
// due to stale expired Agent data remaining in the datastore: https://github.com/spiffe/spire/issues/1836

ds := fakedatastore.New(t)
ds := fakedatastore.New(t, false)
ctx := context.Background()
serverURI := &url.URL{
Scheme: spiffeScheme,
Expand Down
6 changes: 6 additions & 0 deletions pkg/server/endpoints/authorized_entryfetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ func (a *AuthorizedEntryFetcherWithEventsBasedCache) updateAttestedNodesCache(ct
continue
}

selectors, err := a.ds.GetNodeSelectors(ctx, event.SpiffeID, datastore.RequireCurrent)
if err != nil {
return err
}
node.Selectors = selectors

agentExpiresAt := time.Unix(node.CertNotAfter, 0)
if agentExpiresAt.Before(a.clk.Now()) {
a.cache.RemoveAgent(event.SpiffeID)
Expand Down
Loading
Loading