From 78c2ae18bb17d10c7f7a4913d4f8592b89b4bd01 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Mon, 31 Oct 2022 12:40:15 -0700 Subject: [PATCH 1/5] Add active filter for enrollment key queries. Add an active: true filter to enrollment key queries. This allows fleet-server to handle cases where there may be 10+ inactive keys associated with a policy. --- CHANGELOG.next.asciidoc | 3 +- internal/pkg/dl/enrollment_api_key.go | 16 ++++--- .../dl/enrollment_api_key_integration_test.go | 45 +++++++++++++++---- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index c74d1b9ee..efc6c5687 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -16,6 +16,7 @@ - Use seperate rate limiters for internal and external API listeners. {issue}1859[1859] {pull}1904[1904] - Fix fleet.migration.total log key overlap {pull}1951[1951] - Remove POLICY_CHANGE actions from list retrieved from actions index before sending actions to agent on Checkin. {issue}1773[1773] {pull}1963[1963] +- Add active: true filter to enrollemnent key queries. {issue}2029[2029] {pull}2044[2044] ==== New Features @@ -27,4 +28,4 @@ - Fleet Server now allows setting transaction sample rate on APM instrumentation {pull}1681[1681] - Log redacted config when config updates. {issue}1626[1626] {pull}1668[1668] - Storing checkin message in last_checkin_message {pull}1932[1932] -- Allow upgrade actions to signal that they will be retried. {pull}1887[1887] \ No newline at end of file +- Allow upgrade actions to signal that they will be retried. {pull}1887[1887] diff --git a/internal/pkg/dl/enrollment_api_key.go b/internal/pkg/dl/enrollment_api_key.go index 0252beff0..e9b62e21a 100644 --- a/internal/pkg/dl/enrollment_api_key.go +++ b/internal/pkg/dl/enrollment_api_key.go @@ -19,25 +19,29 @@ const ( ) var ( - QueryEnrollmentAPIKeyByID = prepareFindEnrollmentAPIKeyByID() - QueryEnrollmentAPIKeyByPolicyID = prepareFindEnrollmentAPIKeyByPolicyID() + QueryEnrollmentAPIKeyByID = prepareFindActiveEnrollmentAPIKeyByID() + QueryEnrollmentAPIKeyByPolicyID = prepareFindActiveEnrollmentAPIKeyByPolicyID() ) -func prepareFindEnrollmentAPIKeyByID() *dsl.Tmpl { +func prepareFindActiveEnrollmentAPIKeyByID() *dsl.Tmpl { tmpl := dsl.NewTmpl() root := dsl.NewRoot() - root.Query().Bool().Filter().Term(FieldAPIKeyID, tmpl.Bind(FieldAPIKeyID), nil) + filter := root.Query().Bool().Filter() + filter.Term(FieldAPIKeyID, tmpl.Bind(FieldAPIKeyID), nil) + filter.Term(FieldActive, true, nil) tmpl.MustResolve(root) return tmpl } -func prepareFindEnrollmentAPIKeyByPolicyID() *dsl.Tmpl { +func prepareFindActiveEnrollmentAPIKeyByPolicyID() *dsl.Tmpl { tmpl := dsl.NewTmpl() root := dsl.NewRoot() - root.Query().Bool().Filter().Term(FieldPolicyID, tmpl.Bind(FieldPolicyID), nil) + filter := root.Query().Bool().Filter() + filter.Term(FieldPolicyID, tmpl.Bind(FieldPolicyID), nil) + filter.Term(FieldActive, true, nil) tmpl.MustResolve(root) return tmpl diff --git a/internal/pkg/dl/enrollment_api_key_integration_test.go b/internal/pkg/dl/enrollment_api_key_integration_test.go index 02883ee3c..a913e5966 100644 --- a/internal/pkg/dl/enrollment_api_key_integration_test.go +++ b/internal/pkg/dl/enrollment_api_key_integration_test.go @@ -22,13 +22,13 @@ import ( ftesting "github.com/elastic/fleet-server/v7/internal/pkg/testing" ) -func createRandomEnrollmentAPIKey(policyID string) model.EnrollmentAPIKey { +func createRandomEnrollmentAPIKey(policyID string, active bool) model.EnrollmentAPIKey { now := time.Now().UTC() return model.EnrollmentAPIKey{ ESDocument: model.ESDocument{ Id: xid.New().String(), }, - Active: true, + Active: active, APIKey: "d2JndlFIWUJJUVVxWDVia2NJTV86X0d6ZmljZGNTc1d4R1otbklrZFFRZw==", APIKeyID: xid.New().String(), CreatedAt: now.Format(time.RFC3339), @@ -38,8 +38,8 @@ func createRandomEnrollmentAPIKey(policyID string) model.EnrollmentAPIKey { } -func storeRandomEnrollmentAPIKey(ctx context.Context, bulker bulk.Bulk, index string, policyID string) (rec model.EnrollmentAPIKey, err error) { - rec = createRandomEnrollmentAPIKey(policyID) +func storeRandomEnrollmentAPIKey(ctx context.Context, bulker bulk.Bulk, index string, policyID string, active bool) (rec model.EnrollmentAPIKey, err error) { + rec = createRandomEnrollmentAPIKey(policyID, active) body, err := json.Marshal(rec) if err != nil { @@ -58,7 +58,7 @@ func TestSearchEnrollmentAPIKeyByID(t *testing.T) { index, bulker := ftesting.SetupCleanIndex(ctx, t, FleetEnrollmentAPIKeys) - rec, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String()) + rec, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String(), true) if err != nil { t.Fatal(err) } @@ -91,15 +91,15 @@ func TestSearchEnrollmentAPIKeyByPolicyID(t *testing.T) { index, bulker := ftesting.SetupCleanIndex(ctx, t, FleetEnrollmentAPIKeys) policyID := uuid.Must(uuid.NewV4()).String() - rec1, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID) + rec1, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID, true) if err != nil { t.Fatal(err) } - rec2, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID) + rec2, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID, true) if err != nil { t.Fatal(err) } - _, err = storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String()) + _, err = storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String(), true) if err != nil { t.Fatal(err) } @@ -114,3 +114,32 @@ func TestSearchEnrollmentAPIKeyByPolicyID(t *testing.T) { t.Fatal(diff) } } + +func TestSearchEnrollmentAPIKeyByPolicyIDWithInactiveIDs(t *testing.T) { + ctx, cn := context.WithCancel(context.Background()) + defer cn() + + index, bulker := ftesting.SetupCleanIndex(ctx, t, FleetEnrollmentAPIKeys) + + policyID := uuid.Must(uuid.NewV4()).String() + rec, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID, true) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 10; i++ { + _, err = storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String(), false) + if err != nil { + t.Fatal(err) + } + } + + foundRecs, err := findEnrollmentAPIKeys(ctx, bulker, index, QueryEnrollmentAPIKeyByPolicyID, FieldPolicyID, policyID) + if err != nil { + t.Fatal(err) + } + + diff := cmp.Diff([]model.EnrollmentAPIKey{rec}, foundRecs) + if diff != "" { + t.Fatal(diff) + } +} From af60b04ef1144e104461cd45c7fd628d960db974 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 1 Nov 2022 09:00:24 -0700 Subject: [PATCH 2/5] review feedback --- CHANGELOG.next.asciidoc | 2 +- .../pkg/dl/enrollment_api_key_integration_test.go | 8 ++++---- internal/pkg/policy/self.go | 11 ----------- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index efc6c5687..67546b1da 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -16,7 +16,7 @@ - Use seperate rate limiters for internal and external API listeners. {issue}1859[1859] {pull}1904[1904] - Fix fleet.migration.total log key overlap {pull}1951[1951] - Remove POLICY_CHANGE actions from list retrieved from actions index before sending actions to agent on Checkin. {issue}1773[1773] {pull}1963[1963] -- Add active: true filter to enrollemnent key queries. {issue}2029[2029] {pull}2044[2044] +- Add "active: true" filter to enrollemnent key queries. {issue}2029[2029] {pull}2044[2044] ==== New Features diff --git a/internal/pkg/dl/enrollment_api_key_integration_test.go b/internal/pkg/dl/enrollment_api_key_integration_test.go index a913e5966..d4c538c97 100644 --- a/internal/pkg/dl/enrollment_api_key_integration_test.go +++ b/internal/pkg/dl/enrollment_api_key_integration_test.go @@ -124,22 +124,22 @@ func TestSearchEnrollmentAPIKeyByPolicyIDWithInactiveIDs(t *testing.T) { policyID := uuid.Must(uuid.NewV4()).String() rec, err := storeRandomEnrollmentAPIKey(ctx, bulker, index, policyID, true) if err != nil { - t.Fatal(err) + t.Fatalf("unable to store enrollment key: %v", err) } for i := 0; i < 10; i++ { _, err = storeRandomEnrollmentAPIKey(ctx, bulker, index, uuid.Must(uuid.NewV4()).String(), false) if err != nil { - t.Fatal(err) + t.Fatalf("unable to store enrollment key: %v", err) } } foundRecs, err := findEnrollmentAPIKeys(ctx, bulker, index, QueryEnrollmentAPIKeyByPolicyID, FieldPolicyID, policyID) if err != nil { - t.Fatal(err) + t.Fatalf("unable to find enrollment key: %v", err) } diff := cmp.Diff([]model.EnrollmentAPIKey{rec}, foundRecs) if diff != "" { - t.Fatal(diff) + t.Fatalf("expected content does not match", diff) } } diff --git a/internal/pkg/policy/self.go b/internal/pkg/policy/self.go index 468f0fef8..d0ef942f9 100644 --- a/internal/pkg/policy/self.go +++ b/internal/pkg/policy/self.go @@ -228,7 +228,6 @@ func (m *selfMonitorT) updateStatus(ctx context.Context) (proto.StateObserved_St if err != nil { return proto.StateObserved_FAILED, err } - tokens = filterActiveTokens(tokens) if len(tokens) == 0 { // no tokens created for the policy, still starting if m.policyID == "" { @@ -271,13 +270,3 @@ func (d *policyData) HasType(val string) bool { func findEnrollmentAPIKeys(ctx context.Context, bulker bulk.Bulk, policyID string) ([]model.EnrollmentAPIKey, error) { return dl.FindEnrollmentAPIKeys(ctx, bulker, dl.QueryEnrollmentAPIKeyByPolicyID, dl.FieldPolicyID, policyID) } - -func filterActiveTokens(tokens []model.EnrollmentAPIKey) []model.EnrollmentAPIKey { - active := make([]model.EnrollmentAPIKey, 0, len(tokens)) - for _, t := range tokens { - if t.Active { - active = append(active, t) - } - } - return active -} From 80aaad741b668545fb112e159b1086778d6eb2b7 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 1 Nov 2022 09:54:29 -0700 Subject: [PATCH 3/5] fix linter --- internal/pkg/dl/enrollment_api_key_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/dl/enrollment_api_key_integration_test.go b/internal/pkg/dl/enrollment_api_key_integration_test.go index d4c538c97..4cd2de509 100644 --- a/internal/pkg/dl/enrollment_api_key_integration_test.go +++ b/internal/pkg/dl/enrollment_api_key_integration_test.go @@ -140,6 +140,6 @@ func TestSearchEnrollmentAPIKeyByPolicyIDWithInactiveIDs(t *testing.T) { diff := cmp.Diff([]model.EnrollmentAPIKey{rec}, foundRecs) if diff != "" { - t.Fatalf("expected content does not match", diff) + t.Fatalf("expected content does not match: %v", diff) } } From 67253f850734b523a58c40962542ae3924592f04 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 1 Nov 2022 10:33:05 -0700 Subject: [PATCH 4/5] fix tests --- internal/pkg/policy/self.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/pkg/policy/self.go b/internal/pkg/policy/self.go index d0ef942f9..6ef3c49bf 100644 --- a/internal/pkg/policy/self.go +++ b/internal/pkg/policy/self.go @@ -270,3 +270,14 @@ func (d *policyData) HasType(val string) bool { func findEnrollmentAPIKeys(ctx context.Context, bulker bulk.Bulk, policyID string) ([]model.EnrollmentAPIKey, error) { return dl.FindEnrollmentAPIKeys(ctx, bulker, dl.QueryEnrollmentAPIKeyByPolicyID, dl.FieldPolicyID, policyID) } + +// FIXME filterActiveTokens may be used in self_test.go +func filterActiveTokens(tokens []model.EnrollmentAPIKey) []model.EnrollmentAPIKey { + active := make([]model.EnrollmentAPIKey, 0, len(tokens)) + for _, t := range tokens { + if t.Active { + active = append(active, t) + } + } + return active +} From 5976fbf5f86a3056c70a169bf017124c84d998bb Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 1 Nov 2022 10:58:06 -0700 Subject: [PATCH 5/5] Fix test cases --- internal/pkg/policy/self.go | 11 ----------- internal/pkg/policy/self_test.go | 30 ------------------------------ 2 files changed, 41 deletions(-) diff --git a/internal/pkg/policy/self.go b/internal/pkg/policy/self.go index 6ef3c49bf..d0ef942f9 100644 --- a/internal/pkg/policy/self.go +++ b/internal/pkg/policy/self.go @@ -270,14 +270,3 @@ func (d *policyData) HasType(val string) bool { func findEnrollmentAPIKeys(ctx context.Context, bulker bulk.Bulk, policyID string) ([]model.EnrollmentAPIKey, error) { return dl.FindEnrollmentAPIKeys(ctx, bulker, dl.QueryEnrollmentAPIKeyByPolicyID, dl.FieldPolicyID, policyID) } - -// FIXME filterActiveTokens may be used in self_test.go -func filterActiveTokens(tokens []model.EnrollmentAPIKey) []model.EnrollmentAPIKey { - active := make([]model.EnrollmentAPIKey, 0, len(tokens)) - for _, t := range tokens { - if t.Active { - active = append(active, t) - } - } - return active -} diff --git a/internal/pkg/policy/self_test.go b/internal/pkg/policy/self_test.go index ef2df4813..2d81c8274 100644 --- a/internal/pkg/policy/self_test.go +++ b/internal/pkg/policy/self_test.go @@ -262,21 +262,6 @@ func TestSelfMonitor_DefaultPolicy_Degraded(t *testing.T) { t.Fatal(err) } - // add inactive token that should be filtered out - inactiveToken := model.EnrollmentAPIKey{ - ESDocument: model.ESDocument{ - Id: xid.New().String(), - }, - Active: false, - APIKey: "d2JndlFIWUJJUVVxWDVia2NJTV86X0d6ZmljZGNTc1d4R1otbklrZFFRZw==", - APIKeyID: xid.New().String(), - Name: "Inactive", - PolicyID: policyID, - } - tokenLock.Lock() - tokenResult = append(tokenResult, inactiveToken) - tokenLock.Unlock() - go func() { chHitT <- []es.HitT{{ ID: rId, @@ -578,21 +563,6 @@ func TestSelfMonitor_SpecificPolicy_Degraded(t *testing.T) { t.Fatal(err) } - // add inactive token that should be filtered out - inactiveToken := model.EnrollmentAPIKey{ - ESDocument: model.ESDocument{ - Id: xid.New().String(), - }, - Active: false, - APIKey: "d2JndlFIWUJJUVVxWDVia2NJTV86X0d6ZmljZGNTc1d4R1otbklrZFFRZw==", - APIKeyID: xid.New().String(), - Name: "Inactive", - PolicyID: policyID, - } - tokenLock.Lock() - tokenResult = append(tokenResult, inactiveToken) - tokenLock.Unlock() - go func() { chHitT <- []es.HitT{{ ID: rId,