Fact gathering errors

internal/factsengine/entities/facts_gathered.go

@@ -1,10 +1,12 @@
 package entities
 
+import "fmt"
+
 const (
 	FactsGathererdEventSource = "https://github.com/trento-project/agent"
 )
 
-type Error struct {
+type FactGatheringError struct {
 	Message string
 	Type    string
 }
@@ -13,7 +15,7 @@ type Fact struct {
 	Name    string
 	CheckID string
 	Value   interface{}
-	Error   *Error
+	Error   *FactGatheringError
 }
 
 type FactsGathered struct {
@@ -22,6 +24,17 @@ type FactsGathered struct {
 	FactsGathered []Fact
 }
 
+func (e *FactGatheringError) Error() string {
+	return fmt.Sprintf("fact gathering error: %s - %s", e.Type, e.Message)
+}
+
+func (e *FactGatheringError) Wrap(msg string) *FactGatheringError {
+	return &FactGatheringError{
+		Message: fmt.Sprintf("%s: %v", e.Message, msg),
+		Type:    e.Type,
+	}
+}
+
 func NewFactGatheredWithRequest(factReq FactRequest, value interface{}) Fact {
 	return Fact{
 		Name:    factReq.Name,
@@ -30,3 +43,21 @@ func NewFactGatheredWithRequest(factReq FactRequest, value interface{}) Fact {
 		Error:   nil,
 	}
 }
+
+func NewFactGatheredWithError(req FactRequest, err *FactGatheringError) Fact {
+	return Fact{
+		Name:    req.Name,
+		CheckID: req.CheckID,
+		Value:   nil,
+		Error:   err,
+	}
+}
+
+func NewFactsGatheredListWithError(reqs []FactRequest, err *FactGatheringError) []Fact {
+	factsWithErrors := []Fact{}
+	for _, req := range reqs {
+		factsWithErrors = append(factsWithErrors, NewFactGatheredWithError(req, err))
+	}
+
+	return factsWithErrors
+}

internal/factsengine/gatherers/corosyncconf.go

@@ -24,6 +24,24 @@ var (
 	valuePatternCompiled        = regexp.MustCompile(`^\s*(\w+)\s*:\s*(\S+).*`)
 )
 
+// nolint:gochecknoglobals
+var (
+	CorosyncConfFileError = entities.FactGatheringError{
+		Type:    "corosync-conf-file-error",
+		Message: "error reading corosync.conf file",
+	}
+
+	CorosyncConfDecodingError = entities.FactGatheringError{
+		Type:    "corosync-conf-decoding-error",
+		Message: "error decoding corosync.conf file",
+	}
+
+	CorosyncConfValueNotFoundError = entities.FactGatheringError{
+		Type:    "corosync-conf-value-not-found",
+		Message: "requested field value not found",
+	}
+)
+
 type CorosyncConfGatherer struct {
 	configFile string
 }
@@ -44,16 +62,25 @@ func (s *CorosyncConfGatherer) Gather(factsRequests []entities.FactRequest) ([]e
 
 	corosyncConfile, err := readCorosyncConfFileByLines(s.configFile)
 	if err != nil {
-		return facts, err
+		return nil, CorosyncConfFileError.Wrap(err.Error())
 	}
 
-	corosycnMap, err := corosyncConfToMap(corosyncConfile)
+	corosyncMap, err := corosyncConfToMap(corosyncConfile)
 	if err != nil {
-		return facts, err
+		return nil, CorosyncConfDecodingError.Wrap(err.Error())
 	}
 
 	for _, factReq := range factsRequests {
-		fact := entities.NewFactGatheredWithRequest(factReq, getValue(corosycnMap, strings.Split(factReq.Argument, ".")))
+		var fact entities.Fact
+
+		if value := getValue(corosyncMap, strings.Split(factReq.Argument, ".")); value != nil {
+			fact = entities.NewFactGatheredWithRequest(factReq, value)
+
+		} else {
+			gatheringError := CorosyncConfValueNotFoundError.Wrap(factReq.Argument)
+			log.Error(gatheringError)
+			fact = entities.NewFactGatheredWithError(factReq, gatheringError)
+		}
 		facts = append(facts, fact)
 	}
 

internal/factsengine/gatherers/corosyncconf_test.go

@@ -23,7 +23,7 @@ func (suite *CorosyncConfTestSuite) TestCorosyncConfDefault() {
 func (suite *CorosyncConfTestSuite) TestCorosyncConfBasic() {
 	c := NewCorosyncConfGatherer("../../../test/fixtures/gatherers/corosync.conf.basic")
 
-	factRequests := []entities.FactRequest{
+	factsRequest := []entities.FactRequest{
 		{
 			Name:     "corosync_token",
 			Gatherer: "corosync.conf",
@@ -56,7 +56,7 @@ func (suite *CorosyncConfTestSuite) TestCorosyncConfBasic() {
 		},
 	}
 
-	factResults, err := c.Gather(factRequests)
+	factsGathered, err := c.Gather(factsRequest)
 
 	expectedResults := []entities.Fact{
 		{
@@ -98,42 +98,59 @@ func (suite *CorosyncConfTestSuite) TestCorosyncConfBasic() {
 		{
 			Name:  "corosync_not_found",
 			Value: nil,
-			Error: nil,
+			Error: &entities.FactGatheringError{
+				Message: "requested field value not found: totem.not_found",
+				Type:    "corosync-conf-value-not-found",
+			},
 		},
 	}
 
 	suite.NoError(err)
-	suite.ElementsMatch(expectedResults, factResults)
+	suite.ElementsMatch(expectedResults, factsGathered)
 }
 
 func (suite *CorosyncConfTestSuite) TestCorosyncConfFileNotExists() {
 	c := NewCorosyncConfGatherer("not_found")
 
-	factRequests := []entities.FactRequest{
+	factsRequest := []entities.FactRequest{
 		{
 			Name:     "corosync_token",
 			Gatherer: "corosync.conf",
 			Argument: "totem.token",
 		},
 	}
 
-	_, err := c.Gather(factRequests)
+	factsGathered, err := c.Gather(factsRequest)
+
+	expectedError := &entities.FactGatheringError{
+		Message: "error reading corosync.conf file: could not open corosync.conf file: " +
+			"open not_found: no such file or directory",
+		Type: "corosync-conf-file-error",
+	}
 
-	suite.EqualError(err, "could not open corosync.conf file: open not_found: no such file or directory")
+	suite.EqualError(err, expectedError.Error())
+	suite.Empty(factsGathered)
 }
 
 func (suite *CorosyncConfTestSuite) TestCorosyncConfInvalid() {
 	c := NewCorosyncConfGatherer("../../../test/fixtures/gatherers/corosync.conf.invalid")
 
-	factRequests := []entities.FactRequest{
+	factsRequest := []entities.FactRequest{
 		{
 			Name:     "corosync_token",
 			Gatherer: "corosync.conf",
 			Argument: "totem.token",
 		},
 	}
 
-	_, err := c.Gather(factRequests)
+	factsGathered, err := c.Gather(factsRequest)
+
+	expectedError := &entities.FactGatheringError{
+		Message: "error decoding corosync.conf file: invalid corosync file structure. " +
+			"some section is not closed properly",
+		Type: "corosync-conf-decoding-error",
+	}
 
-	suite.EqualError(err, "invalid corosync file structure. some section is not closed properly")
+	suite.EqualError(err, expectedError.Error())
+	suite.Empty(factsGathered)
 }

internal/factsengine/gathering.go

@@ -1,6 +1,7 @@
 package factsengine
 
 import (
+	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	"github.com/trento-project/agent/internal/factsengine/entities"
 	"github.com/trento-project/agent/internal/factsengine/gatherers"
@@ -36,9 +37,16 @@ func gatherFacts(
 
 		// Execute the fact gathering asynchronously and in parallel
 		g.Go(func() error {
-			if newFacts, err := gatherer.Gather(factsRequest); err == nil {
+			var gatheringError *entities.FactGatheringError
+
+			newFacts, err := gatherer.Gather(factsRequest)
+			switch {
+			case err == nil:
 				factsCh <- newFacts
-			} else {
+			case errors.As(err, &gatheringError):
+				log.Error(gatheringError)
+				factsCh <- entities.NewFactsGatheredListWithError(factsRequest, gatheringError)
+			default:
 				log.Error(err)
 			}
 

internal/factsengine/gathering_test.go

@@ -1,7 +1,6 @@
 package factsengine
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
@@ -64,7 +63,7 @@ func NewErrorGatherer() *ErrorGatherer {
 }
 
 func (s *ErrorGatherer) Gather(_ []entities.FactRequest) ([]entities.Fact, error) {
-	return []entities.Fact{}, fmt.Errorf("kabum")
+	return nil, &entities.FactGatheringError{Type: "dummy-type", Message: "some error"}
 }
 
 func (suite *GatheringTestSuite) TestGatheringGatherFacts() {
@@ -183,6 +182,16 @@ func (suite *GatheringTestSuite) TestFactsEngineGatherFactsErrorGathering() {
 			Name:    "dummy1",
 			Value:   "1",
 			CheckID: "check1",
+			Error:   nil,
+		},
+		{
+			Name:    "error",
+			Value:   nil,
+			CheckID: "check1",
+			Error: &entities.FactGatheringError{
+				Type:    "dummy-type",
+				Message: "some error",
+			},
 		},
 	}
 

internal/factsengine/mapper_test.go

@@ -97,7 +97,7 @@ func (suite *MapperTestSuite) TestFactsGatheredWithErrorToEvent() {
 				Name:    "dummy1",
 				Value:   nil,
 				CheckID: "check1",
-				Error: &entities.Error{
+				Error: &entities.FactGatheringError{
 					Message: "some message",
 					Type:    "some_type",
 				},