RunAttestors refactor

attestation/context.go

@@ -117,75 +117,35 @@ func NewContext(attestors []Attestor, opts ...AttestationContextOption) (*Attest
 }
 
 func (ctx *AttestationContext) RunAttestors() error {
-	preAttestors := []Attestor{}
-	materialAttestors := []Attestor{}
-	exeucteAttestors := []Attestor{}
-	productAttestors := []Attestor{}
-	postAttestors := []Attestor{}
-
+	attestors := make(map[RunType][]Attestor)
 	for _, attestor := range ctx.attestors {
-		switch attestor.RunType() {
-		case PreMaterialRunType:
-			preAttestors = append(preAttestors, attestor)
-
-		case MaterialRunType:
-			materialAttestors = append(materialAttestors, attestor)
-
-		case ExecuteRunType:
-			exeucteAttestors = append(exeucteAttestors, attestor)
-
-		case ProductRunType:
-			productAttestors = append(productAttestors, attestor)
-
-		case PostProductRunType:
-			postAttestors = append(postAttestors, attestor)
-
-		default:
+		if attestor.RunType() == "" {
 			return ErrInvalidOption{
-				Option: "attestor.RunType",
+				Option: "RunType",
 				Reason: fmt.Sprintf("unknown run type %v", attestor.RunType()),
 			}
 		}
-	}
-
-	for _, attestor := range preAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
-		}
-	}
-
-	for _, attestor := range materialAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
-		}
-	}
-
-	for _, attestor := range exeucteAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
-		}
-	}
 
-	for _, attestor := range productAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
-		}
+		attestors[attestor.RunType()] = append(attestors[attestor.RunType()], attestor)
 	}
 
-	for _, attestor := range postAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
+	for _, atts := range attestors {
+		for _, att := range atts {
+			log.Infof("Starting %v attestor...", att.Name())
+			if err := ctx.runAttestor(att); err != nil {
+				log.Errorf("Error running %v attestor: %w", att.Name(), err)
+				return err
+			}
 		}
 	}
 
 	return nil
 }
 
 func (ctx *AttestationContext) runAttestor(attestor Attestor) error {
-	log.Infof("Starting %v attestor...", attestor.Name())
 	startTime := time.Now()
+	// NOTE: Not sure if this is the right place to check for an error running the attestor - might be better to let the caller handle it
 	if err := attestor.Attest(ctx); err != nil {
-		log.Errorf("Error running %v attestor: %w", attestor.Name(), err)
 		ctx.completedAttestors = append(ctx.completedAttestors, CompletedAttestor{
 			Attestor:  attestor,
 			StartTime: startTime,
@@ -205,48 +165,34 @@ func (ctx *AttestationContext) runAttestor(attestor Attestor) error {
 		ctx.addMaterials(materialer)
 	}
 
-	if producter, ok := attestor.(Producer); ok {
-		ctx.addProducts(producter)
+	if producer, ok := attestor.(Producer); ok {
+		ctx.addProducts(producer)
 	}
 
 	return nil
 }
 
 func (ctx *AttestationContext) CompletedAttestors() []CompletedAttestor {
-	attestors := make([]CompletedAttestor, len(ctx.completedAttestors))
-	copy(attestors, ctx.completedAttestors)
-	return attestors
+	return ctx.completedAttestors
 }
 
 func (ctx *AttestationContext) WorkingDir() string {
 	return ctx.workingDir
 }
 
 func (ctx *AttestationContext) Hashes() []crypto.Hash {
-	hashes := make([]crypto.Hash, len(ctx.hashes))
-	copy(hashes, ctx.hashes)
-	return hashes
+	return ctx.hashes
 }
 
 func (ctx *AttestationContext) Context() context.Context {
 	return ctx.ctx
 }
 
 func (ctx *AttestationContext) Materials() map[string]cryptoutil.DigestSet {
-	matCopy := make(map[string]cryptoutil.DigestSet)
-	for k, v := range ctx.materials {
-		matCopy[k] = v
-	}
-
-	return matCopy
+	return ctx.materials
 }
 
 func (ctx *AttestationContext) Products() map[string]Product {
-	prodCopy := make(map[string]Product)
-	for k, v := range ctx.products {
-		prodCopy[k] = v
-	}
-
 	return ctx.products
 }
 

attestation/factory.go

@@ -41,6 +41,8 @@ type Subjecter interface {
 	Subjects() map[string]cryptoutil.DigestSet
 }
 
+// NOTE: not sure on the name of this interface, however I can't think of an alternative for now
+
 // Materialer allows attestors to communicate about materials that were observed
 // while the attestor executed. For example the material attestor records the hashes
 // of all files before a command is run.

run.go

@@ -25,6 +25,7 @@ import (
 	"github.com/in-toto/go-witness/cryptoutil"
 	"github.com/in-toto/go-witness/dsse"
 	"github.com/in-toto/go-witness/intoto"
+	"github.com/in-toto/go-witness/log"
 )
 
 type runOptions struct {
@@ -85,6 +86,18 @@ func Run(stepName string, signer cryptoutil.Signer, opts ...RunOption) (RunResul
 		return result, fmt.Errorf("failed to run attestors: %w", err)
 	}
 
+	fail := false
+	for _, r := range runCtx.CompletedAttestors() {
+		if r.Error != nil {
+			fail = true
+			log.Errorf("attestor %s failed: %s", r.Attestor.Name(), r.Error)
+		}
+	}
+
+	if fail {
+		return result, fmt.Errorf("attestors failed with errors")
+	}
+
 	result.Collection = attestation.NewCollection(ro.stepName, runCtx.CompletedAttestors())
 	result.SignedEnvelope, err = signCollection(result.Collection, dsse.SignWithSigners(ro.signer), dsse.SignWithTimestampers(ro.timestampers...))
 	if err != nil {