From 9f64383d91d2d266372e7c71ae2b2ef9fa2a2812 Mon Sep 17 00:00:00 2001 From: Rashmi Gottipati Date: Wed, 26 Apr 2017 15:09:46 -0700 Subject: [PATCH] Fixes #1614 - Protect cleanup code of plugin unload --- control/control.go | 2 +- control/plugin_manager.go | 38 ++++++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/control/control.go b/control/control.go index f4a172999..c0f474d80 100644 --- a/control/control.go +++ b/control/control.go @@ -218,7 +218,7 @@ func New(cfg *Config) *pluginControl { }).Debug("metric catalog created") // Plugin Manager - c.pluginManager = newPluginManager(OptSetPprof(cfg.Pprof)) + c.pluginManager = newPluginManager(OptSetPprof(cfg.Pprof), OptSetTempDirPath(cfg.TempDirPath)) controlLogger.WithFields(log.Fields{ "_block": "new", }).Debug("plugin manager created") diff --git a/control/plugin_manager.go b/control/plugin_manager.go index 378f0afd8..8e962122e 100644 --- a/control/plugin_manager.go +++ b/control/plugin_manager.go @@ -236,6 +236,7 @@ type pluginManager struct { pluginConfig *pluginConfig pluginTags map[string]map[string]string pprof bool + tempDirPath string } func newPluginManager(opts ...pluginManagerOpt) *pluginManager { @@ -260,6 +261,13 @@ func newPluginManager(opts ...pluginManagerOpt) *pluginManager { type pluginManagerOpt func(*pluginManager) +// OptSetPprof sets the pprof flag on the plugin manager +func OptSetTempDirPath(path string) pluginManagerOpt { + return func(p *pluginManager) { + p.tempDirPath = path + } +} + // OptSetPprof sets the pprof flag on the plugin manager func OptSetPprof(pprof bool) pluginManagerOpt { return func(p *pluginManager) { @@ -582,22 +590,32 @@ func (p *pluginManager) UnloadPlugin(pl core.Plugin) (*loadedPlugin, serror.Snap "plugin-version": plugin.Version(), "plugin-path": plugin.Details.Path, }).Debugf("Removing plugin") - if err := os.RemoveAll(filepath.Dir(plugin.Details.Path)); err != nil { + if strings.Contains(plugin.Details.Path, p.tempDirPath) { + if err := os.RemoveAll(filepath.Dir(plugin.Details.Path)); err != nil { + pmLogger.WithFields(log.Fields{ + "plugin-type": plugin.TypeName(), + "plugin-name": plugin.Name(), + "plugin-version": plugin.Version(), + "plugin-path": plugin.Details.Path, + }).Error(err) + se := serror.New(err) + se.SetFields(map[string]interface{}{ + "plugin-type": plugin.TypeName(), + "plugin-name": plugin.Name(), + "plugin-version": plugin.Version(), + "plugin-path": plugin.Details.Path, + }) + return nil, se + } + } else { pmLogger.WithFields(log.Fields{ "plugin-type": plugin.TypeName(), "plugin-name": plugin.Name(), "plugin-version": plugin.Version(), "plugin-path": plugin.Details.Path, - }).Error(err) - se := serror.New(err) - se.SetFields(map[string]interface{}{ - "plugin-type": plugin.TypeName(), - "plugin-name": plugin.Name(), - "plugin-version": plugin.Version(), - "plugin-path": plugin.Details.Path, - }) - return nil, se + }).Debug("Nothing to delete as temp path is empty") } + p.loadedPlugins.remove(plugin.Key()) // Remove any metrics from the catalog if this was a collector