Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 27 Packages Projects Releases Wiki Activity

Implement custom cloning of Team MDM config for the cached mysql layer. (#14965)

Browse Source
This commit is contained in:
Martin Angers
2023-11-07 09:51:55 -05:00
committed by GitHub
parent f42f4d48af
commit 3bfafcd23c
8 changed files with 295 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/issue-14708-fix-cached-team-mdm
+1
View File
@@ -0,0 +1 @@
* Fixed caching of a team's MDM configuration so that it implements a custom cloning, avoiding performance issues at scale.
server/datastore/cached_mysql/bench_cached_mysql_test.go
+72
View File
@@ -0,0 +1,72 @@
package cached_mysql
import (
"testing"
"time"
"github.com/fleetdm/fleet/v4/pkg/optjson"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/patrickmn/go-cache"
"github.com/stretchr/testify/require"
)
type unclonableTeamMDMConfig fleet.TeamMDM
// This exported variable is to make sure that the compiler doesn't optimize
// away the benchmarked function call.
var Result interface{}
// On my laptop, results are as follows. Under load, the reflection-based
// approach really adds up and is CPU intensive, resulting in drastic
// performance drops.
//
// goos: linux
// goarch: amd64
// pkg: github.com/fleetdm/fleet/v4/server/datastore/cached_mysql
// cpu: Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
// BenchmarkCacheGetFallbackClone-8 53228 22706 ns/op 11976 B/op 217 allocs/op
// BenchmarkCacheGetCustomClone-8 5741186 196.3 ns/op 177 B/op 3 allocs/op
func BenchmarkCacheGetFallbackClone(b *testing.B) {
v := unclonableTeamMDMConfig(cachedValue())
benchmarkCacheGet(b, &v)
}
func BenchmarkCacheGetCustomClone(b *testing.B) {
v := cachedValue()
benchmarkCacheGet(b, &v)
}
func benchmarkCacheGet(b *testing.B, v any) {
c := &cloneCache{cache.New(time.Minute, time.Minute)}
c.Set("k", v, cache.DefaultExpiration)
b.ResetTimer()
var ok bool
for i := 0; i < b.N; i++ {
Result, ok = c.Get("k")
if !ok {
b.Fatal("expected ok")
}
}
require.Equal(b, v, Result)
}
func cachedValue() fleet.TeamMDM {
return fleet.TeamMDM{
EnableDiskEncryption: true,
MacOSUpdates: fleet.MacOSUpdates{
MinimumVersion: optjson.SetString("10.10.10"),
Deadline: optjson.SetString("1992-03-01"),
},
MacOSSettings: fleet.MacOSSettings{
CustomSettings: []string{"a", "b"},
DeprecatedEnableDiskEncryption: ptr.Bool(false),
},
MacOSSetup: fleet.MacOSSetup{
BootstrapPackage: optjson.SetString("bootstrap"),
},
}
}
server/datastore/cached_mysql/cached_mysql.go
+9
View File
@@ -41,6 +41,11 @@ func clone(v interface{}) (interface{}, error) {
return cloner.Clone()
}
// TODO(mna): consider making implementation of the cloner interface
// mandatory, and panic/fail loudly if not implemented. Reflection-based deep
// cloning has significant performance issues at scale (better yet - make the
// cache accept/return cloner types instead of interface{}).
if v == nil {
return nil, nil
}
@@ -157,6 +162,7 @@ func New(ds fleet.Datastore, opts ...Option) fleet.Datastore {
scheduledQueriesExp: defaultScheduledQueriesExpiration,
teamAgentOptionsExp: defaultTeamAgentOptionsExpiration,
teamFeaturesExp: defaultTeamFeaturesExpiration,
teamMDMConfigExp: defaultTeamMDMConfigExpiration,
queryByNameExp: defaultQueryByNameExpiration,
queryResultsCountExp: defaultQueryResultsCountExpiration,
}
@@ -292,6 +298,9 @@ func (ds *cachedMysql) TeamMDMConfig(ctx context.Context, teamID uint) (*fleet.T
if err != nil {
return nil, err
}
ds.c.Set(key, cfg, ds.teamMDMConfigExp)
return cfg, nil
}
server/datastore/cached_mysql/cached_mysql_test.go
+39 -1
View File
@@ -400,6 +400,7 @@ func TestCachedTeamFeatures(t *testing.T) {
EnableHostUsers: false,
EnableSoftwareInventory: true,
AdditionalQueries: &aq,
DetailQueryOverrides: map[string]*string{"a": ptr.String("A"), "b": ptr.String("B")},
}
testTeam := fleet.Team{
@@ -427,9 +428,18 @@ func TestCachedTeamFeatures(t *testing.T) {
return nil
}
// get it the first time, it will populate the cache
features, err := ds.TeamFeatures(context.Background(), 1)
require.NoError(t, err)
require.Equal(t, testFeatures, *features)
require.True(t, mockedDS.TeamFeaturesFuncInvoked)
mockedDS.TeamFeaturesFuncInvoked = false
// get it again, will retrieve it from the cache
features, err = ds.TeamFeatures(context.Background(), 1)
require.NoError(t, err)
require.Equal(t, testFeatures, *features)
require.False(t, mockedDS.TeamFeaturesFuncInvoked)
// saving a team updates features in cache
aq = json.RawMessage(`{"bar": "baz"}`)
@@ -437,6 +447,7 @@ func TestCachedTeamFeatures(t *testing.T) {
EnableHostUsers: true,
EnableSoftwareInventory: false,
AdditionalQueries: &aq,
DetailQueryOverrides: map[string]*string{"c": ptr.String("C")},
}
updateTeam := &fleet.Team{
ID: testTeam.ID,
@@ -450,10 +461,12 @@ func TestCachedTeamFeatures(t *testing.T) {
_, err = ds.SaveTeam(context.Background(), updateTeam)
require.NoError(t, err)
require.True(t, mockedDS.SaveTeamFuncInvoked)
features, err = ds.TeamFeatures(context.Background(), testTeam.ID)
require.NoError(t, err)
require.Equal(t, updateFeatures, *features)
require.False(t, mockedDS.TeamFeaturesFuncInvoked)
// deleting a team removes the features from the cache
err = ds.DeleteTeam(context.Background(), testTeam.ID)
@@ -461,20 +474,29 @@ func TestCachedTeamFeatures(t *testing.T) {
_, err = ds.TeamFeatures(context.Background(), testTeam.ID)
require.Error(t, err)
require.True(t, mockedDS.TeamFeaturesFuncInvoked)
}
func TestCachedTeamMDMConfig(t *testing.T) {
t.Parallel()
mockedDS := new(mock.Store)
ds := New(mockedDS, WithTeamFeaturesExpiration(100*time.Millisecond))
ds := New(mockedDS, WithTeamMDMConfigExpiration(100*time.Millisecond))
ao := json.RawMessage(`{}`)
testMDMConfig := fleet.TeamMDM{
EnableDiskEncryption: true,
MacOSUpdates: fleet.MacOSUpdates{
MinimumVersion: optjson.SetString("10.10.10"),
Deadline: optjson.SetString("1992-03-01"),
},
MacOSSettings: fleet.MacOSSettings{
CustomSettings: []string{"a", "b"},
DeprecatedEnableDiskEncryption: ptr.Bool(false),
},
MacOSSetup: fleet.MacOSSetup{
BootstrapPackage: optjson.SetString("bootstrap"),
},
}
testTeam := fleet.Team{
@@ -502,9 +524,18 @@ func TestCachedTeamMDMConfig(t *testing.T) {
return nil
}
// get the team's config, will load it into cache
mdmConfig, err := ds.TeamMDMConfig(context.Background(), 1)
require.NoError(t, err)
require.Equal(t, testMDMConfig, *mdmConfig)
require.True(t, mockedDS.TeamMDMConfigFuncInvoked)
mockedDS.TeamMDMConfigFuncInvoked = false
// get it again, will get it from cache
mdmConfig, err = ds.TeamMDMConfig(context.Background(), 1)
require.NoError(t, err)
require.Equal(t, testMDMConfig, *mdmConfig)
require.False(t, mockedDS.TeamMDMConfigFuncInvoked)
// saving a team updates config in cache
updateMDMConfig := fleet.TeamMDM{
@@ -512,6 +543,10 @@ func TestCachedTeamMDMConfig(t *testing.T) {
MinimumVersion: optjson.SetString("13.13.13"),
Deadline: optjson.SetString("2022-03-01"),
},
MacOSSettings: fleet.MacOSSettings{
CustomSettings: nil,
DeprecatedEnableDiskEncryption: ptr.Bool(true),
},
}
updateTeam := &fleet.Team{
ID: testTeam.ID,
@@ -525,10 +560,12 @@ func TestCachedTeamMDMConfig(t *testing.T) {
_, err = ds.SaveTeam(context.Background(), updateTeam)
require.NoError(t, err)
require.True(t, mockedDS.SaveTeamFuncInvoked)
mdmConfig, err = ds.TeamMDMConfig(context.Background(), testTeam.ID)
require.NoError(t, err)
require.Equal(t, updateMDMConfig, *mdmConfig)
require.False(t, mockedDS.TeamMDMConfigFuncInvoked)
// deleting a team removes the config from the cache
err = ds.DeleteTeam(context.Background(), testTeam.ID)
@@ -536,4 +573,5 @@ func TestCachedTeamMDMConfig(t *testing.T) {
_, err = ds.TeamMDMConfig(context.Background(), testTeam.ID)
require.Error(t, err)
require.True(t, mockedDS.TeamMDMConfigFuncInvoked)
}
server/fleet/app.go
+52 -1
View File
@@ -443,7 +443,17 @@ func (c *AppConfig) Copy() *AppConfig {
if c.Features.AdditionalQueries != nil {
aq := make(json.RawMessage, len(*c.Features.AdditionalQueries))
copy(aq, *c.Features.AdditionalQueries)
c.Features.AdditionalQueries = &aq
clone.Features.AdditionalQueries = &aq
}
if c.Features.DetailQueryOverrides != nil {
clone.Features.DetailQueryOverrides = make(map[string]*string, len(c.Features.DetailQueryOverrides))
for k, v := range c.Features.DetailQueryOverrides {
var s *string
if v != nil {
s = ptr.String(*v)
}
clone.Features.DetailQueryOverrides[k] = s
}
}
if c.AgentOptions != nil {
ao := make(json.RawMessage, len(*c.AgentOptions))
@@ -774,6 +784,11 @@ type Features struct {
EnableSoftwareInventory bool `json:"enable_software_inventory"`
AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"`
DetailQueryOverrides map[string]*string `json:"detail_query_overrides,omitempty"`
/////////////////////////////////////////////////////////////////
// WARNING: If you add to this struct make sure it's taken into
// account in the Features Clone implementation!
/////////////////////////////////////////////////////////////////
}
func (f *Features) ApplyDefaultsForNewInstalls() {
@@ -788,6 +803,42 @@ func (f *Features) ApplyDefaults() {
f.EnableHostUsers = true
}
// Clone implements cloner for Features.
func (f *Features) Clone() (interface{}, error) {
return f.Copy(), nil
}
// Copy returns a deep copy of the Features.
func (f *Features) Copy() *Features {
if f == nil {
return nil
}
// EnableHostUsers and EnableSoftwareInventory don't have fields that require
// cloning (all fields are basic value types, no pointers/slices/maps).
var clone Features
clone = *f
if f.AdditionalQueries != nil {
aq := make(json.RawMessage, len(*f.AdditionalQueries))
copy(aq, *f.AdditionalQueries)
clone.AdditionalQueries = &aq
}
if f.DetailQueryOverrides != nil {
clone.DetailQueryOverrides = make(map[string]*string, len(f.DetailQueryOverrides))
for k, v := range f.DetailQueryOverrides {
var s *string
if v != nil {
s = ptr.String(*v)
}
clone.DetailQueryOverrides[k] = s
}
}
return &clone
}
// FleetDesktopSettings contains settings used to configure Fleet Desktop.
type FleetDesktopSettings struct {
// TransparencyURL is the URL used for the “Transparency” link in the Fleet Desktop menu.
server/fleet/app_test.go
+47
View File
@@ -311,3 +311,50 @@ func TestAtLeastOnePlatformEnabledAndConfigured(t *testing.T) {
})
}
}
func TestFeaturesCopy(t *testing.T) {
t.Run("nil receiver", func(t *testing.T) {
var f *Features
require.Nil(t, f.Copy())
})
t.Run("copy value fields", func(t *testing.T) {
f := &Features{
EnableHostUsers: true,
EnableSoftwareInventory: false,
}
clone := f.Copy()
require.NotNil(t, clone)
require.Equal(t, f.EnableHostUsers, clone.EnableHostUsers)
require.Equal(t, f.EnableSoftwareInventory, clone.EnableSoftwareInventory)
require.Nil(t, clone.AdditionalQueries)
require.Nil(t, clone.DetailQueryOverrides)
})
t.Run("copy AdditionalQueries", func(t *testing.T) {
rawMessage := json.RawMessage(`{"test": "data"}`)
f := &Features{
AdditionalQueries: &rawMessage,
}
clone := f.Copy()
require.NotNil(t, clone.AdditionalQueries)
require.NotSame(t, f.AdditionalQueries, clone.AdditionalQueries)
require.Equal(t, *f.AdditionalQueries, *clone.AdditionalQueries)
})
t.Run("copy DetailQueryOverrides", func(t *testing.T) {
f := &Features{
DetailQueryOverrides: map[string]*string{
"foo": ptr.String("bar"),
"baz": nil,
},
}
clone := f.Copy()
require.NotNil(t, clone.DetailQueryOverrides)
require.NotSame(t, f.DetailQueryOverrides, clone.DetailQueryOverrides)
// map values are pointers, check that they have been cloned
require.NotSame(t, f.DetailQueryOverrides["foo"], clone.DetailQueryOverrides["foo"])
// the map content itself is equal
require.Equal(t, f.DetailQueryOverrides, clone.DetailQueryOverrides)
})
}
server/fleet/teams.go
+34
View File
@@ -7,6 +7,7 @@ import (
"time"
"github.com/fleetdm/fleet/v4/pkg/optjson"
"github.com/fleetdm/fleet/v4/server/ptr"
)
const (
@@ -152,6 +153,39 @@ type TeamMDM struct {
MacOSSettings MacOSSettings `json:"macos_settings"`
MacOSSetup MacOSSetup `json:"macos_setup"`
// NOTE: TeamSpecMDM must be kept in sync with TeamMDM.
/////////////////////////////////////////////////////////////////
// WARNING: If you add to this struct make sure it's taken into
// account in the TeamMDM Clone implementation!
/////////////////////////////////////////////////////////////////
}
// Clone implements cloner for TeamMDM.
func (t *TeamMDM) Clone() (interface{}, error) {
return t.Copy(), nil
}
// Copy returns a deep copy of the TeamMDM.
func (t *TeamMDM) Copy() *TeamMDM {
if t == nil {
return nil
}
var clone TeamMDM
clone = *t
// EnableDiskEncryption, MacOSUpdates and MacOSSetup don't have fields that
// require cloning (all fields are basic value types, no
// pointers/slices/maps).
if t.MacOSSettings.CustomSettings != nil {
clone.MacOSSettings.CustomSettings = make([]string, len(t.MacOSSettings.CustomSettings))
copy(clone.MacOSSettings.CustomSettings, t.MacOSSettings.CustomSettings)
}
if t.MacOSSettings.DeprecatedEnableDiskEncryption != nil {
clone.MacOSSettings.DeprecatedEnableDiskEncryption = ptr.Bool(*t.MacOSSettings.DeprecatedEnableDiskEncryption)
}
return &clone
}
type TeamSpecMDM struct {
server/fleet/teams_test.go
+41
View File
@@ -3,6 +3,7 @@ package fleet
import (
"testing"
"github.com/fleetdm/fleet/v4/pkg/optjson"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/stretchr/testify/require"
)
@@ -258,3 +259,43 @@ func TestValidateUserRoles(t *testing.T) {
})
}
}
func TestTeamMDMCopy(t *testing.T) {
t.Run("nil receiver", func(t *testing.T) {
var tm *TeamMDM
require.Nil(t, tm.Copy())
})
t.Run("copy value fields", func(t *testing.T) {
tm := &TeamMDM{
EnableDiskEncryption: true,
MacOSUpdates: MacOSUpdates{
MinimumVersion: optjson.SetString("10.15.4"),
Deadline: optjson.SetString("2020-01-01"),
},
MacOSSetup: MacOSSetup{
BootstrapPackage: optjson.SetString("bootstrap"),
EnableEndUserAuthentication: true,
MacOSSetupAssistant: optjson.SetString("assistant"),
},
}
clone := tm.Copy()
require.NotNil(t, clone)
require.NotSame(t, tm, clone)
require.Equal(t, tm, clone)
})
t.Run("copy MacOSSettings", func(t *testing.T) {
tm := &TeamMDM{
MacOSSettings: MacOSSettings{
CustomSettings: []string{"a", "b"},
DeprecatedEnableDiskEncryption: ptr.Bool(false),
},
}
clone := tm.Copy()
require.NotSame(t, tm, clone)
require.Equal(t, tm, clone)
require.NotSame(t, tm.MacOSSettings.CustomSettings, clone.MacOSSettings.CustomSettings)
require.NotSame(t, tm.MacOSSettings.DeprecatedEnableDiskEncryption, clone.MacOSSettings.DeprecatedEnableDiskEncryption)
})
}