Gatherers versioning (#277)

* Refactored gatherers registry to support gatherer versioning

* StandardGatherers return a gatherers versioned tree

* Gatherers plugin defaults on v1 version

* Refactored tests due to the new gatherers structure

* Deterministic Version sorting in AvailableGatherers

* Addressing feedbacks on versioned gatherer registry

internal/factsengine/factsengine_integration_test.go

@@ -82,8 +82,10 @@ func (s *FactsEngineIntegrationTestGatherer) Gather(requests []entities.FactRequ
 func (suite *FactsEngineIntegrationTestSuite) TestFactsEngineIntegration() {
 	agentID := "some-agent"
 
-	gathererRegistry := gatherers.NewRegistry(map[string]gatherers.FactGatherer{
-		"integration": NewFactsEngineIntegrationTestGatherer(),
+	gathererRegistry := gatherers.NewRegistry(gatherers.FactGatherersTree{
+		"integration": map[string]gatherers.FactGatherer{
+			"v1": NewFactsEngineIntegrationTestGatherer(),
+		},
 	})
 
 	engine := NewFactsEngine(agentID, suite.factsEngineService, *gathererRegistry)

internal/factsengine/gatherers/gatherer.go

@@ -8,25 +8,61 @@ type FactGatherer interface {
 	Gather(factsRequests []entities.FactRequest) ([]entities.Fact, error)
 }
 
-func StandardGatherers() map[string]FactGatherer {
-	return map[string]FactGatherer{
-		CibAdminGathererName:        NewDefaultCibAdminGatherer(),
-		CorosyncCmapCtlGathererName: NewDefaultCorosyncCmapctlGatherer(),
-		CorosyncConfGathererName:    NewDefaultCorosyncConfGatherer(),
-		DirScanGathererName:         NewDefaultDirScanGatherer(),
-		FstabGathererName:           NewDefaultFstabGatherer(),
-		GroupsGathererName:          NewDefaultGroupsGatherer(),
-		HostsFileGathererName:       NewDefaultHostsFileGatherer(),
-		PackageVersionGathererName:  NewDefaultPackageVersionGatherer(),
-		PasswdGathererName:          NewDefaultPasswdGatherer(),
-		SapControlGathererName:      NewDefaultSapControlGatherer(),
-		SapHostCtrlGathererName:     NewDefaultSapHostCtrlGatherer(),
-		SapProfilesGathererName:     NewDefaultSapProfilesGatherer(),
-		SaptuneGathererName:         NewDefaultSaptuneGatherer(),
-		SBDConfigGathererName:       NewDefaultSBDGatherer(),
-		SBDDumpGathererName:         NewDefaultSBDDumpGatherer(),
-		SysctlGathererName:          NewDefaultSysctlGatherer(),
-		SystemDGathererName:         NewDefaultSystemDGatherer(),
-		VerifyPasswordGathererName:  NewDefaultPasswordGatherer(),
+func StandardGatherers() FactGatherersTree {
+	return FactGatherersTree{
+		CibAdminGathererName: map[string]FactGatherer{
+			"v1": NewDefaultCibAdminGatherer(),
+		},
+		CorosyncCmapCtlGathererName: map[string]FactGatherer{
+			"v1": NewDefaultCorosyncCmapctlGatherer(),
+		},
+		CorosyncConfGathererName: map[string]FactGatherer{
+			"v1": NewDefaultCorosyncConfGatherer(),
+		},
+		DirScanGathererName: map[string]FactGatherer{
+			"v1": NewDefaultDirScanGatherer(),
+		},
+		FstabGathererName: map[string]FactGatherer{
+			"v1": NewDefaultFstabGatherer(),
+		},
+		GroupsGathererName: map[string]FactGatherer{
+			"v1": NewDefaultGroupsGatherer(),
+		},
+		HostsFileGathererName: map[string]FactGatherer{
+			"v1": NewDefaultHostsFileGatherer(),
+		},
+		PackageVersionGathererName: map[string]FactGatherer{
+			"v1": NewDefaultPackageVersionGatherer(),
+		},
+		PasswdGathererName: map[string]FactGatherer{
+			"v1": NewDefaultPasswdGatherer(),
+		},
+		SapControlGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSapControlGatherer(),
+		},
+		SapHostCtrlGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSapHostCtrlGatherer(),
+		},
+		SapProfilesGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSapProfilesGatherer(),
+		},
+		SaptuneGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSaptuneGatherer(),
+		},
+		SBDConfigGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSBDGatherer(),
+		},
+		SBDDumpGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSBDDumpGatherer(),
+		},
+		SysctlGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSysctlGatherer(),
+		},
+		SystemDGathererName: map[string]FactGatherer{
+			"v1": NewDefaultSystemDGatherer(),
+		},
+		VerifyPasswordGathererName: map[string]FactGatherer{
+			"v1": NewDefaultPasswordGatherer(),
+		},
 	}
 }

internal/factsengine/gatherers/plugin.go

@@ -11,6 +11,8 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+const defaultPluginVersion = "v1"
+
 type PluginLoader interface {
 	Load(pluginPath string) (FactGatherer, error)
 }
@@ -20,8 +22,8 @@ type PluginLoaders map[string]PluginLoader
 func GetGatherersFromPlugins(
 	loaders PluginLoaders,
 	pluginsFolder string,
-) (map[string]FactGatherer, error) {
-	pluginFactGatherers := make(map[string]FactGatherer)
+) (FactGatherersTree, error) {
+	pluginFactGatherers := make(FactGatherersTree)
 	log.Debugf("Loading plugins...")
 
 	plugins, err := filepath.Glob(fmt.Sprintf("%s/*", pluginsFolder))
@@ -43,7 +45,9 @@ func GetGatherersFromPlugins(
 
 		name := path.Base(filePath)
 		name = strings.TrimSuffix(name, path.Ext(name))
-		pluginFactGatherers[name] = loadedPlugin
+		pluginFactGatherers[name] = map[string]FactGatherer{
+			defaultPluginVersion: loadedPlugin,
+		}
 		log.Debugf("Plugin %s loaded properly", filePath)
 	}
 

internal/factsengine/gatherers/plugin_test.go

@@ -53,9 +53,14 @@ func (suite *PluginTestSuite) TestPluginLoadPlugins() {
 
 	plugin1Name := path.Base(plugin1.Name())
 	plugin2Name := path.Base(plugin2.Name())
-	expectedGatherers := map[string]gatherers.FactGatherer{
-		plugin1Name: &mocks.FactGatherer{},
-		plugin2Name: &mocks.FactGatherer{},
+
+	expectedGatherers := gatherers.FactGatherersTree{
+		plugin1Name: map[string]gatherers.FactGatherer{
+			"v1": &mocks.FactGatherer{},
+		},
+		plugin2Name: map[string]gatherers.FactGatherer{
+			"v1": &mocks.FactGatherer{},
+		},
 	}
 
 	suite.NoError(err)
@@ -78,7 +83,7 @@ func (suite *PluginTestSuite) TestPluginLoadPluginsError() {
 
 	loadedPlugins, err := gatherers.GetGatherersFromPlugins(loaders, pluginsFolder)
 
-	expectedGatherers := map[string]gatherers.FactGatherer{}
+	expectedGatherers := gatherers.FactGatherersTree{}
 
 	suite.NoError(err)
 	suite.Equal(expectedGatherers, loadedPlugins)

internal/factsengine/gatherers/registry.go

@@ -1,32 +1,88 @@
 package gatherers
 
-import "github.com/pkg/errors"
+import (
+	"fmt"
+	"sort"
+	"strings"
+)
+
+type GathererNotFoundError struct {
+	Name string
+}
+
+func (e *GathererNotFoundError) Error() string {
+	return fmt.Sprintf("gatherer %s not found", e.Name)
+}
+
+// map[gathererName]map[GathererVersion]FactGatherer
+type FactGatherersTree map[string]map[string]FactGatherer
+
+func extractVersionAndGathererName(gathererName string) (string, string, error) {
+	parts := strings.Split(gathererName, "@")
+	if len(parts) == 1 {
+		// no version found, just gatherer name
+		return parts[0], "", nil
+	}
+	if len(parts) != 2 {
+		return "", "", fmt.Errorf(
+			"could not extract the gatherer version from %s, version should follow <gathererName>@<version> syntax",
+			gathererName,
+		)
+	}
+	return parts[0], parts[1], nil
+}
 
 type Registry struct {
-	gatherers map[string]FactGatherer
+	gatherers FactGatherersTree
+}
+
+func NewRegistry(gatherers FactGatherersTree) *Registry {
+	return &Registry{
+		gatherers: gatherers,
+	}
 }
 
 func (m *Registry) GetGatherer(name string) (FactGatherer, error) {
-	if g, found := m.gatherers[name]; found {
+	gathererName, version, err := extractVersionAndGathererName(name)
+	if err != nil {
+		return nil, err
+	}
+	if version == "" {
+		latestVersion, err := m.getLatestVersionForGatherer(name)
+		if err != nil {
+			return nil, err
+		}
+		version = latestVersion
+	}
+
+	if g, found := m.gatherers[gathererName][version]; found {
 		return g, nil
 	}
-	return nil, errors.Errorf("gatherer %s not found", name)
+	return nil, &GathererNotFoundError{Name: name}
 }
 
 func (m *Registry) AvailableGatherers() []string {
 	gatherersList := []string{}
 
-	for gatherer := range m.gatherers {
-		gatherersList = append(gatherersList, gatherer)
+	for gathererName, versions := range m.gatherers {
+		gathererVersions := []string{}
+		for v := range versions {
+			gathererVersions = append(gathererVersions, v)
+		}
+		sort.Strings(gathererVersions)
+		gatherersList = append(
+			gatherersList,
+			fmt.Sprintf("%s - %s", gathererName, strings.Join(gathererVersions, "/")),
+		)
 	}
 
 	return gatherersList
 }
 
 // This is not safe, please not use concurrently.
-func (m *Registry) AddGatherers(gatherers map[string]FactGatherer) {
-	maps := []map[string]FactGatherer{m.gatherers, gatherers}
-	result := make(map[string]FactGatherer)
+func (m *Registry) AddGatherers(gatherers FactGatherersTree) {
+	maps := []FactGatherersTree{m.gatherers, gatherers}
+	result := make(FactGatherersTree)
 
 	for _, m := range maps {
 		for k, v := range m {
@@ -36,8 +92,17 @@ func (m *Registry) AddGatherers(gatherers map[string]FactGatherer) {
 	m.gatherers = result
 }
 
-func NewRegistry(gatherers map[string]FactGatherer) *Registry {
-	return &Registry{
-		gatherers: gatherers,
+func (m *Registry) getLatestVersionForGatherer(name string) (string, error) {
+	availableGatherers, found := m.gatherers[name]
+	if !found {
+		return "", &GathererNotFoundError{Name: name}
 	}
+	versions := []string{}
+	for v := range availableGatherers {
+		versions = append(versions, v)
+	}
+
+	sort.Strings(versions)
+
+	return versions[len(versions)-1], nil
 }