Clone AppConfig and ScheduledQuery list by hand to improve CPU usage (#8794)

* Close AppConfig and ScheduledQuery list by hand to improve CPU usage

* Address review comments

* Update remaining mocks

changes/issue-8710-by-hand-clone

@@ -0,0 +1 @@
+* Clone AppConfig and ScheduledQueries list by hand to improve CPU usage

server/datastore/cached_mysql/cached_mysql.go

@@ -208,10 +208,10 @@ func (ds *cachedMysql) ListPacksForHost(ctx context.Context, hid uint) ([]*fleet
 	return packs, nil
 }
 
-func (ds *cachedMysql) ListScheduledQueriesInPack(ctx context.Context, packID uint) ([]*fleet.ScheduledQuery, error) {
+func (ds *cachedMysql) ListScheduledQueriesInPack(ctx context.Context, packID uint) (fleet.ScheduledQueryList, error) {
 	key := fmt.Sprintf(scheduledQueriesKey, packID)
 	if x, found := ds.c.Get(key); found {
-		scheduledQueries, ok := x.([]*fleet.ScheduledQuery)
+		scheduledQueries, ok := x.(fleet.ScheduledQueryList)
 		if ok {
 			return scheduledQueries, nil
 		}

server/datastore/cached_mysql/cached_mysql_test.go

@@ -260,7 +260,7 @@ func TestCachedListScheduledQueriesInPack(t *testing.T) {
 	mockedDS := new(mock.Store)
 	ds := New(mockedDS, WithScheduledQueriesExpiration(100*time.Millisecond))
 
-	dbScheduledQueries := []*fleet.ScheduledQuery{
+	dbScheduledQueries := fleet.ScheduledQueryList{
 		{
 			ID:   1,
 			Name: "test-schedule-1",
@@ -271,7 +271,7 @@ func TestCachedListScheduledQueriesInPack(t *testing.T) {
 		},
 	}
 	called := 0
-	mockedDS.ListScheduledQueriesInPackFunc = func(ctx context.Context, packID uint) ([]*fleet.ScheduledQuery, error) {
+	mockedDS.ListScheduledQueriesInPackFunc = func(ctx context.Context, packID uint) (fleet.ScheduledQueryList, error) {
 		called++
 		return dbScheduledQueries, nil
 	}
@@ -281,7 +281,7 @@ func TestCachedListScheduledQueriesInPack(t *testing.T) {
 	require.Equal(t, dbScheduledQueries, scheduledQueries)
 
 	// change "stored" dbScheduledQueries.
-	dbScheduledQueries = []*fleet.ScheduledQuery{
+	dbScheduledQueries = fleet.ScheduledQueryList{
 		{
 			ID:   3,
 			Name: "test-schedule-3",
@@ -290,7 +290,7 @@ func TestCachedListScheduledQueriesInPack(t *testing.T) {
 
 	scheduledQueries2, err := ds.ListScheduledQueriesInPack(context.Background(), 1)
 	require.NoError(t, err)
-	require.Equal(t, scheduledQueries, scheduledQueries2) // returns the new db entry
+	require.Equal(t, scheduledQueries2, scheduledQueries) // returns the new db entry
 	require.Equal(t, 1, called)
 
 	time.Sleep(200 * time.Millisecond)

server/datastore/mysql/migrations/tables/20220818101352_ChangeSoftwareVendorWidth_test.go

@@ -14,6 +14,7 @@ func TestUp_20220818101352(t *testing.T) {
 	('zchunk-libs', '1.2.1', 'rpm_packages', '', 'Fedora Project', 'x86_64'), 
 	('zchunk-libs', '1.2.1', 'rpm_packages', '', 'Fedora Project II', 'x86_64'), 
 	('word', '1.2.1', 'rpm_packages', '', 'Fake MS', 'x86_64'),
+	('word', '1.2.2', 'rpm_packages', '', 'Fake MS', 'x86_64'),
 	('excel', '1.2.1', 'rpm_packages', '', '', 'x86_64')
 	`)
 	require.NoError(t, err)
@@ -25,7 +26,7 @@ func TestUp_20220818101352(t *testing.T) {
 	var vendors []string
 	err = db.Select(&vendors, `SELECT vendor FROM software`)
 	require.NoError(t, err)
-	require.ElementsMatch(t, []string{"Fedora Project", "Fedora Project II", "Fake MS", ""}, vendors)
+	require.ElementsMatch(t, []string{"Fedora Project", "Fedora Project II", "Fake MS", "Fake MS", ""}, vendors)
 
 	// Check we can store a longer vendors
 	randVendor := `

server/datastore/mysql/scheduled_queries.go

@@ -52,7 +52,7 @@ func (ds *Datastore) ListScheduledQueriesInPackWithStats(ctx context.Context, id
 }
 
 // ListScheduledQueriesInPack lists all the scheduled queries of a pack.
-func (ds *Datastore) ListScheduledQueriesInPack(ctx context.Context, id uint) ([]*fleet.ScheduledQuery, error) {
+func (ds *Datastore) ListScheduledQueriesInPack(ctx context.Context, id uint) (fleet.ScheduledQueryList, error) {
 	query := `
 		SELECT
 			sq.id,

server/fleet/app.go

@@ -131,6 +131,11 @@ type AppConfig struct {
 	// this field is set to the list of legacy settings keys during UnmarshalJSON
 	// if any legacy settings were set in the raw JSON.
 	didUnmarshalLegacySettings []string
+
+	/////////////////////////////////////////////////////////////////
+	// WARNING: If you add to this struct make sure it's taken into
+	// account in the AppConfig Clone implementation!
+	/////////////////////////////////////////////////////////////////
 }
 
 // legacyConfig holds settings that have been replaced, superceded or
@@ -139,6 +144,62 @@ type legacyConfig struct {
 	HostSettings *Features `json:"host_settings"`
 }
 
+func (c *AppConfig) Clone() (interface{}, error) {
+	if c == nil {
+		return nil, nil
+	}
+
+	var clone AppConfig
+	clone = *c
+
+	// OrgInfo: nothing needs cloning
+	// FleetDesktopSettings: nothing needs cloning
+
+	if c.ServerSettings.DebugHostIDs != nil {
+		clone.ServerSettings.DebugHostIDs = make([]uint, len(c.ServerSettings.DebugHostIDs))
+		copy(clone.ServerSettings.DebugHostIDs, c.ServerSettings.DebugHostIDs)
+	}
+
+	// SMTPSettings: nothing needs cloning
+	// HostExpirySettings: nothing needs cloning
+
+	if c.Features.AdditionalQueries != nil {
+		aq := make(json.RawMessage, len(*c.Features.AdditionalQueries))
+		copy(aq, *c.Features.AdditionalQueries)
+		c.Features.AdditionalQueries = &aq
+	}
+	if c.AgentOptions != nil {
+		ao := make(json.RawMessage, len(*c.AgentOptions))
+		copy(ao, *c.AgentOptions)
+		clone.AgentOptions = &ao
+	}
+
+	// SSOSettings: nothing needs cloning
+	// FleetDesktop: nothing needs cloning
+	// VulnerabilitySettings: nothing needs cloning
+
+	if c.WebhookSettings.FailingPoliciesWebhook.PolicyIDs != nil {
+		clone.WebhookSettings.FailingPoliciesWebhook.PolicyIDs = make([]uint, len(c.WebhookSettings.FailingPoliciesWebhook.PolicyIDs))
+		copy(clone.WebhookSettings.FailingPoliciesWebhook.PolicyIDs, c.WebhookSettings.FailingPoliciesWebhook.PolicyIDs)
+	}
+	if c.Integrations.Jira != nil {
+		clone.Integrations.Jira = make([]*JiraIntegration, len(c.Integrations.Jira))
+		for i, j := range c.Integrations.Jira {
+			jira := *j
+			clone.Integrations.Jira[i] = &jira
+		}
+	}
+	if c.Integrations.Zendesk != nil {
+		clone.Integrations.Zendesk = make([]*ZendeskIntegration, len(c.Integrations.Zendesk))
+		for i, z := range c.Integrations.Zendesk {
+			zd := *z
+			clone.Integrations.Zendesk[i] = &zd
+		}
+	}
+
+	return &clone, nil
+}
+
 // EnrichedAppConfig contains the AppConfig along with additional fleet
 // instance configuration settings as returned by the
 // "GET /api/latest/fleet/config" API endpoint (and fleetctl get config).

server/fleet/datastore.go

@@ -588,7 +588,7 @@ type Datastore interface {
 	UpdateHost(ctx context.Context, host *Host) error
 
 	// ListScheduledQueriesInPack lists all the scheduled queries of a pack.
-	ListScheduledQueriesInPack(ctx context.Context, packID uint) ([]*ScheduledQuery, error)
+	ListScheduledQueriesInPack(ctx context.Context, packID uint) (ScheduledQueryList, error)
 
 	// UpdateHostRefetchRequested updates a host's refetch requested field.
 	UpdateHostRefetchRequested(ctx context.Context, hostID uint, value bool) error

server/fleet/scheduled_queries.go

@@ -3,6 +3,7 @@ package fleet
 import (
 	"time"
 
+	"github.com/fleetdm/fleet/v4/server/ptr"
 	"gopkg.in/guregu/null.v3"
 )
 
@@ -48,6 +49,40 @@ type ScheduledQuery struct {
 	Denylist *bool `json:"denylist"`
 
 	AggregatedStats `json:"stats,omitempty"`
+
+	/////////////////////////////////////////////////////////////////
+	// WARNING: If you add to this struct make sure it's taken into
+	// account in the ScheduledQueryList Clone implementation!
+	/////////////////////////////////////////////////////////////////
+}
+
+type ScheduledQueryList []*ScheduledQuery
+
+func (sql ScheduledQueryList) Clone() (interface{}, error) {
+	var cloned ScheduledQueryList
+	for _, sq := range sql {
+		newSq := *sq
+		if sq.Snapshot != nil {
+			newSq.Snapshot = ptr.Bool(*sq.Snapshot)
+		}
+		if sq.Removed != nil {
+			newSq.Removed = ptr.Bool(*sq.Removed)
+		}
+		if sq.Platform != nil {
+			newSq.Platform = ptr.String(*sq.Platform)
+		}
+		if sq.Version != nil {
+			newSq.Version = ptr.String(*sq.Version)
+		}
+		if sq.Shard != nil {
+			newSq.Shard = ptr.Uint(*sq.Shard)
+		}
+		if sq.Denylist != nil {
+			newSq.Denylist = ptr.Bool(*sq.Denylist)
+		}
+		cloned = append(cloned, &newSq)
+	}
+	return cloned, nil
 }
 
 type AggregatedStats struct {

server/mock/datastore_mock.go

@@ -429,7 +429,7 @@ type UpdateHostSoftwareFunc func(ctx context.Context, hostID uint, software []fl
 
 type UpdateHostFunc func(ctx context.Context, host *fleet.Host) error
 
-type ListScheduledQueriesInPackFunc func(ctx context.Context, packID uint) ([]*fleet.ScheduledQuery, error)
+type ListScheduledQueriesInPackFunc func(ctx context.Context, packID uint) (fleet.ScheduledQueryList, error)
 
 type UpdateHostRefetchRequestedFunc func(ctx context.Context, hostID uint, value bool) error
 
@@ -2290,7 +2290,7 @@ func (s *DataStore) UpdateHost(ctx context.Context, host *fleet.Host) error {
 	return s.UpdateHostFunc(ctx, host)
 }
 
-func (s *DataStore) ListScheduledQueriesInPack(ctx context.Context, packID uint) ([]*fleet.ScheduledQuery, error) {
+func (s *DataStore) ListScheduledQueriesInPack(ctx context.Context, packID uint) (fleet.ScheduledQueryList, error) {
 	s.ListScheduledQueriesInPackFuncInvoked = true
 	return s.ListScheduledQueriesInPackFunc(ctx, packID)
 }

server/service/osquery_test.go

@@ -44,7 +44,7 @@ func TestGetClientConfig(t *testing.T) {
 	ds.ListPacksForHostFunc = func(ctx context.Context, hid uint) ([]*fleet.Pack, error) {
 		return []*fleet.Pack{}, nil
 	}
-	ds.ListScheduledQueriesInPackFunc = func(ctx context.Context, pid uint) ([]*fleet.ScheduledQuery, error) {
+	ds.ListScheduledQueriesInPackFunc = func(ctx context.Context, pid uint) (fleet.ScheduledQueryList, error) {
 		tru := true
 		fals := false
 		fortytwo := uint(42)