Skip to content

Commit b2f9fb3

Browse files
slokslok
authored
Merge pull request slok#703 from slok/slok/log
Add ability to load plugins in strict mode on sloth lib
2 parents 3accaae + 39b835a commit b2f9fb3

File tree

6 files changed

+87
-40
lines changed

6 files changed

+87
-40
lines changed

‎cmd/sloth/commands/helpers.go‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,40 +5,19 @@ import (
55
"encoding/json"
66
"fmt"
77
"io/fs"
8-
"os"
98
"path/filepath"
109
"regexp"
1110
"strings"
1211

1312
"github.com/slok/sloth/internal/app/generate"
1413
"github.com/slok/sloth/internal/log"
15-
"github.com/slok/sloth/internal/plugin"
1614
plugincorealertrulesv1 "github.com/slok/sloth/internal/plugin/slo/core/alert_rules_v1"
1715
plugincoremetadatarulesv1 "github.com/slok/sloth/internal/plugin/slo/core/metadata_rules_v1"
1816
plugincoreslirulesv1 "github.com/slok/sloth/internal/plugin/slo/core/sli_rules_v1"
1917
plugincorevalidatev1 "github.com/slok/sloth/internal/plugin/slo/core/validate_v1"
20-
pluginenginesli "github.com/slok/sloth/internal/pluginengine/sli"
21-
pluginengineslo "github.com/slok/sloth/internal/pluginengine/slo"
22-
storagefs "github.com/slok/sloth/internal/storage/fs"
2318
"github.com/slok/sloth/pkg/common/model"
2419
)
2520

26-
func createPluginLoader(ctx context.Context, logger log.Logger, paths []string) (*storagefs.FilePluginRepo, error) {
27-
fss := []fs.FS{
28-
plugin.EmbeddedDefaultSLOPlugins,
29-
}
30-
for _, p := range paths {
31-
fss = append(fss, os.DirFS(p))
32-
}
33-
34-
pluginsRepo, err := storagefs.NewFilePluginRepo(logger, pluginenginesli.PluginLoader, pluginengineslo.PluginLoader, fss...)
35-
if err != nil {
36-
return nil, fmt.Errorf("could not create file SLO and SLI plugins repository: %w", err)
37-
}
38-
39-
return pluginsRepo, nil
40-
}
41-
4221
func discoverSLOManifests(logger log.Logger, exclude, include *regexp.Regexp, path string) ([]string, error) {
4322
logger = logger.WithValues(log.Kv{"svc": "SLODiscovery"})
4423

‎cmd/sloth/commands/k8scontroller.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ import (
3333
"github.com/slok/sloth/internal/app/generate"
3434
"github.com/slok/sloth/internal/app/kubecontroller"
3535
"github.com/slok/sloth/internal/log"
36+
"github.com/slok/sloth/internal/plugin"
37+
pluginenginesli "github.com/slok/sloth/internal/pluginengine/sli"
38+
pluginengineslo "github.com/slok/sloth/internal/pluginengine/slo"
3639
"github.com/slok/sloth/internal/storage"
40+
storagefs "github.com/slok/sloth/internal/storage/fs"
3741
storageio "github.com/slok/sloth/internal/storage/io"
3842
storagek8s "github.com/slok/sloth/internal/storage/k8s"
3943
slothv1 "github.com/slok/sloth/pkg/kubernetes/api/sloth/v1"
@@ -483,3 +487,19 @@ func (g generatorLogger) WithValues(values map[string]interface{}) log.Logger {
483487
func (g generatorLogger) WithCtxValues(ctx context.Context) log.Logger {
484488
return generatorLogger{Logger: g.Logger.WithCtxValues(ctx)}
485489
}
490+
491+
func createPluginLoader(ctx context.Context, logger log.Logger, paths []string) (*storagefs.FilePluginRepo, error) {
492+
fss := []fs.FS{
493+
plugin.EmbeddedDefaultSLOPlugins,
494+
}
495+
for _, p := range paths {
496+
fss = append(fss, os.DirFS(p))
497+
}
498+
499+
pluginsRepo, err := storagefs.NewFilePluginRepo(logger, false, pluginenginesli.PluginLoader, pluginengineslo.PluginLoader, fss...)
500+
if err != nil {
501+
return nil, fmt.Errorf("could not create file SLO and SLI plugins repository: %w", err)
502+
}
503+
504+
return pluginsRepo, nil
505+
}

‎internal/storage/fs/plugin.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,19 @@ type FilePluginRepo struct {
2929
sliPluginCache map[string]pluginenginesli.SLIPlugin
3030
logger log.Logger
3131
mu sync.RWMutex
32+
failOnError bool
3233
}
3334

3435
// NewFilePluginRepo returns a new FilePluginRepo that loads SLI and SLO plugins from the given file system.
35-
func NewFilePluginRepo(logger log.Logger, sliPluginLoader SLIPluginLoader, sloPluginLoader SLOPluginLoader, fss ...fs.FS) (*FilePluginRepo, error) {
36+
func NewFilePluginRepo(logger log.Logger, failOnError bool, sliPluginLoader SLIPluginLoader, sloPluginLoader SLOPluginLoader, fss ...fs.FS) (*FilePluginRepo, error) {
3637
r := &FilePluginRepo{
3738
fss: fss,
3839
sliPluginLoader: sliPluginLoader,
3940
sloPluginLoader: sloPluginLoader,
4041
sloPluginCache: map[string]pluginengineslo.Plugin{},
4142
sliPluginCache: map[string]pluginenginesli.SLIPlugin{},
4243
logger: logger,
44+
failOnError: failOnError,
4345
}
4446

4547
err := r.Reload(context.Background())
@@ -153,7 +155,11 @@ func (r *FilePluginRepo) loadPlugins(ctx context.Context, fss ...fs.FS) (map[str
153155
return nil
154156
}
155157

156-
r.logger.Errorf("could not load %q as SLI or SLO plugin: (SLI plugin error: %s | SLO plugin error: %s)", path, sliErr, sloErr)
158+
err = fmt.Errorf("could not load %q as SLI or SLO plugin: (SLI plugin error: %w | SLO plugin error: %w)", path, sliErr, sloErr)
159+
if r.failOnError {
160+
return err
161+
}
162+
r.logger.Errorf(err.Error())
157163

158164
return nil
159165
})

‎internal/storage/fs/plugin_test.go‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ import (
1919

2020
func TestFilePluginRepoListSLOPlugins(t *testing.T) {
2121
tests := map[string]struct {
22-
fss func() []fs.FS
23-
mock func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader)
24-
expPlugins map[string]pluginengineslo.Plugin
25-
expLoadErr bool
26-
expErr bool
22+
failOnError bool
23+
fss func() []fs.FS
24+
mock func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader)
25+
expPlugins map[string]pluginengineslo.Plugin
26+
expLoadErr bool
27+
expErr bool
2728
}{
2829
"Having no files, should return empty list of plugins.": {
2930
fss: func() []fs.FS { return nil },
@@ -113,6 +114,26 @@ func TestFilePluginRepoListSLOPlugins(t *testing.T) {
113114
"p1": {ID: "p1"},
114115
},
115116
},
117+
118+
"Having an error while loading a plugin on strict mode, should fail.": {
119+
failOnError: true,
120+
fss: func() []fs.FS {
121+
m1 := make(fstest.MapFS)
122+
m1["m1/pl1/plugin.go"] = &fstest.MapFile{Data: []byte("p1")}
123+
m1["m1/pl2/plugin.go"] = &fstest.MapFile{Data: []byte("p2")}
124+
125+
return []fs.FS{m1}
126+
},
127+
mock: func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader) {
128+
mslipl.On("LoadRawSLIPlugin", mock.Anything, "p1").Once().Return(nil, fmt.Errorf("something"))
129+
mslipl.On("LoadRawSLIPlugin", mock.Anything, "p2").Once().Return(nil, fmt.Errorf("something"))
130+
131+
mslopl.On("LoadRawPlugin", mock.Anything, "p1").Once().Return(&pluginengineslo.Plugin{ID: "p1"}, nil)
132+
mslopl.On("LoadRawPlugin", mock.Anything, "p2").Once().Return(nil, fmt.Errorf("something"))
133+
134+
},
135+
expLoadErr: true,
136+
},
116137
}
117138

118139
for name, test := range tests {
@@ -124,7 +145,7 @@ func TestFilePluginRepoListSLOPlugins(t *testing.T) {
124145
test.mock(mslopl, mslipl)
125146

126147
// Create repository and load plugins.
127-
repo, err := storagefs.NewFilePluginRepo(log.Noop, mslipl, mslopl, test.fss()...)
148+
repo, err := storagefs.NewFilePluginRepo(log.Noop, test.failOnError, mslipl, mslopl, test.fss()...)
128149
if test.expLoadErr {
129150
assert.Error(err)
130151
return
@@ -185,7 +206,7 @@ func TestFilePluginRepoGetSLOPlugin(t *testing.T) {
185206
test.mock(mslopl, mslipl)
186207

187208
// Create repository and load plugins.
188-
repo, err := storagefs.NewFilePluginRepo(log.Noop, mslipl, mslopl, test.fss()...)
209+
repo, err := storagefs.NewFilePluginRepo(log.Noop, false, mslipl, mslopl, test.fss()...)
189210
require.NoError(err)
190211

191212
plugin, err := repo.GetSLOPlugin(t.Context(), test.pluginID)
@@ -200,11 +221,12 @@ func TestFilePluginRepoGetSLOPlugin(t *testing.T) {
200221

201222
func TestFilePluginRepoListSLIPlugins(t *testing.T) {
202223
tests := map[string]struct {
203-
fss func() []fs.FS
204-
mock func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader)
205-
expPlugins map[string]pluginenginesli.SLIPlugin
206-
expLoadErr bool
207-
expErr bool
224+
failOnError bool
225+
fss func() []fs.FS
226+
mock func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader)
227+
expPlugins map[string]pluginenginesli.SLIPlugin
228+
expLoadErr bool
229+
expErr bool
208230
}{
209231
"Having no files, should return empty list of plugins.": {
210232
fss: func() []fs.FS { return nil },
@@ -281,6 +303,24 @@ func TestFilePluginRepoListSLIPlugins(t *testing.T) {
281303
"p1": {ID: "p1"},
282304
},
283305
},
306+
307+
"Having an error while loading a plugin on strict mode, should fail.": {
308+
failOnError: true,
309+
fss: func() []fs.FS {
310+
m1 := make(fstest.MapFS)
311+
m1["m1/pl1/plugin.go"] = &fstest.MapFile{Data: []byte("p1")}
312+
m1["m1/pl2/plugin.go"] = &fstest.MapFile{Data: []byte("p2")}
313+
314+
return []fs.FS{m1}
315+
},
316+
mock: func(mslopl *fsmock.SLOPluginLoader, mslipl *fsmock.SLIPluginLoader) {
317+
mslipl.On("LoadRawSLIPlugin", mock.Anything, "p1").Once().Return(&pluginenginesli.SLIPlugin{ID: "p1"}, nil)
318+
mslipl.On("LoadRawSLIPlugin", mock.Anything, "p2").Once().Return(nil, fmt.Errorf("something"))
319+
mslopl.On("LoadRawPlugin", mock.Anything, "p2").Once().Return(nil, fmt.Errorf("something"))
320+
321+
},
322+
expLoadErr: true,
323+
},
284324
}
285325

286326
for name, test := range tests {
@@ -292,7 +332,7 @@ func TestFilePluginRepoListSLIPlugins(t *testing.T) {
292332
test.mock(mslopl, mslipl)
293333

294334
// Create repository and load plugins.
295-
repo, err := storagefs.NewFilePluginRepo(log.Noop, mslipl, mslopl, test.fss()...)
335+
repo, err := storagefs.NewFilePluginRepo(log.Noop, test.failOnError, mslipl, mslopl, test.fss()...)
296336
if test.expLoadErr {
297337
assert.Error(err)
298338
return
@@ -350,7 +390,7 @@ func TestFilePluginRepoGetSLIPlugin(t *testing.T) {
350390
test.mock(mslopl, mslipl)
351391

352392
// Create repository and load plugins.
353-
repo, err := storagefs.NewFilePluginRepo(log.Noop, mslipl, mslopl, test.fss()...)
393+
repo, err := storagefs.NewFilePluginRepo(log.Noop, false, mslipl, mslopl, test.fss()...)
354394
require.NoError(err)
355395

356396
plugin, err := repo.GetSLIPlugin(t.Context(), test.pluginID)

‎pkg/lib/gen.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ type PrometheusSLOGeneratorConfig struct {
4040
WindowsFS fs.FS
4141
// PluginsFS are the FSs where custom SLO and SLI plugins exist.
4242
PluginsFS []fs.FS
43+
// StrictPlugins makes the plugin loader fail when a plugin can't be loaded.
44+
StrictPlugins bool
4345
// DefaultSLOPeriod is the default SLO period to use when not specified in the SLO definition.
4446
DefaultSLOPeriod time.Duration
4547
// DisableDefaultPlugins disables the default SLO plugins, normally used along with custom SLO plugins to fully customize Sloth behavior.
@@ -110,7 +112,7 @@ func NewPrometheusSLOGenerator(config PrometheusSLOGeneratorConfig) (*Prometheus
110112
}
111113

112114
// Create plugin repo.
113-
pluginRepo, err := createPluginLoader(ctx, config.Logger, config.PluginsFS)
115+
pluginRepo, err := createPluginLoader(ctx, config.Logger, config.PluginsFS, config.StrictPlugins)
114116
if err != nil {
115117
return nil, fmt.Errorf("could not create plugin repository: %w", err)
116118
}

‎pkg/lib/helper.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ import (
1717
storagefs "github.com/slok/sloth/internal/storage/fs"
1818
)
1919

20-
func createPluginLoader(ctx context.Context, logger log.Logger, pluginsFS []fs.FS) (*storagefs.FilePluginRepo, error) {
20+
func createPluginLoader(ctx context.Context, logger log.Logger, pluginsFS []fs.FS, strict bool) (*storagefs.FilePluginRepo, error) {
2121
// We should load at least the Sloth embedded default ones.
2222
fss := append([]fs.FS{}, pluginsFS...)
2323
if len(fss) == 0 {
2424
fss = append(fss, plugin.EmbeddedDefaultSLOPlugins)
2525
}
2626

27-
pluginsRepo, err := storagefs.NewFilePluginRepo(logger, pluginenginesli.PluginLoader, pluginengineslo.PluginLoader, fss...)
27+
pluginsRepo, err := storagefs.NewFilePluginRepo(logger, strict, pluginenginesli.PluginLoader, pluginengineslo.PluginLoader, fss...)
2828
if err != nil {
2929
return nil, fmt.Errorf("could not create file SLO and SLI plugins repository: %w", err)
3030
}

0 commit comments

Comments
 (0)