RunAttestors refactor (#131)

* improving run attestors Signed-off-by: chaosinthecrd <[email protected]> * finalising changes. Signed-off-by: chaosinthecrd <[email protected]> * improving run attestors Signed-off-by: chaosinthecrd <[email protected]> * finalising changes. Signed-off-by: chaosinthecrd <[email protected]> * addressing review, restoring run type order Signed-off-by: chaosinthecrd <[email protected]> * updating error handling logic Signed-off-by: chaosinthecrd <[email protected]> * updating to go 1.21 for errors.Join Signed-off-by: chaosinthecrd <[email protected]> --------- Signed-off-by: chaosinthecrd <[email protected]> Signed-off-by: Tom Meadows <[email protected]>
in-toto · Feb 2, 2024 · 4f2a630 · 4f2a630
1 parent be75142
commit 4f2a630
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 81 deletions.
diff --git a/attestation/commandrun/commandrun.go b/attestation/commandrun/commandrun.go
@@ -115,9 +115,10 @@ type CommandRun struct {
 
 func (rc *CommandRun) Attest(ctx *attestation.AttestationContext) error {
 	if len(rc.Cmd) == 0 {
-		return attestation.ErrInvalidOption{
-			Option: "Cmd",
-			Reason: "CommandRun attestation requires a command to run",
+		return attestation.ErrAttestor{
+			Name:    rc.Name(),
+			RunType: rc.RunType(),
+			Reason:  "CommandRun attestation requires a command to run",
 		}
 	}
 

diff --git a/attestation/context.go b/attestation/context.go
@@ -35,17 +35,22 @@ const (
 	PostProductRunType RunType = "postproduct"
 )
 
+func runTypeOrder() []RunType {
+	return []RunType{PreMaterialRunType, MaterialRunType, ExecuteRunType, ProductRunType, PostProductRunType}
+}
+
 func (r RunType) String() string {
 	return string(r)
 }
 
-type ErrInvalidOption struct {
-	Option string
-	Reason string
+type ErrAttestor struct {
+	Name    string
+	RunType RunType
+	Reason  string
 }
 
-func (e ErrInvalidOption) Error() string {
-	return fmt.Sprintf("invalid value for option %v: %v", e.Option, e.Reason)
+func (e ErrAttestor) Error() string {
+	return fmt.Sprintf("error returned for attestor %s of run type %s: %s", e.Name, e.RunType, e.Reason)
 }
 
 type AttestationContextOption func(ctx *AttestationContext)
@@ -117,82 +122,40 @@ 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:
-			return ErrInvalidOption{
-				Option: "attestor.RunType",
-				Reason: fmt.Sprintf("unknown run type %v", attestor.RunType()),
+		if attestor.RunType() == "" {
+			return ErrAttestor{
+				Name:    attestor.Name(),
+				RunType: attestor.RunType(),
+				Reason:  "attestor run type not set",
 			}
 		}
-	}
 
-	for _, attestor := range preAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
-		}
+		attestors[attestor.RunType()] = append(attestors[attestor.RunType()], attestor)
 	}
 
-	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
-		}
-	}
-
-	for _, attestor := range postAttestors {
-		if err := ctx.runAttestor(attestor); err != nil {
-			return err
+	order := runTypeOrder()
+	for _, k := range order {
+		log.Debugf("starting %s attestors...", k.String())
+		for _, att := range attestors[k] {
+			log.Infof("Starting %v attestor...", att.Name())
+			ctx.runAttestor(att)
 		}
 	}
 
 	return nil
 }
 
-func (ctx *AttestationContext) runAttestor(attestor Attestor) error {
-	log.Infof("Starting %v attestor...", attestor.Name())
+func (ctx *AttestationContext) runAttestor(attestor Attestor) {
 	startTime := time.Now()
 	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,
 			EndTime:   time.Now(),
 			Error:     err,
 		})
-		return err
 	}
 
 	ctx.completedAttestors = append(ctx.completedAttestors, CompletedAttestor{
@@ -205,17 +168,15 @@ 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
+	out := make([]CompletedAttestor, len(ctx.completedAttestors))
+	copy(out, ctx.completedAttestors)
+	return out
 }
 
 func (ctx *AttestationContext) WorkingDir() string {
@@ -233,21 +194,19 @@ func (ctx *AttestationContext) Context() context.Context {
 }
 
 func (ctx *AttestationContext) Materials() map[string]cryptoutil.DigestSet {
-	matCopy := make(map[string]cryptoutil.DigestSet)
+	out := make(map[string]cryptoutil.DigestSet)
 	for k, v := range ctx.materials {
-		matCopy[k] = v
+		out[k] = v
 	}
-
-	return matCopy
+	return out
 }
 
 func (ctx *AttestationContext) Products() map[string]Product {
-	prodCopy := make(map[string]Product)
+	out := make(map[string]Product)
 	for k, v := range ctx.products {
-		prodCopy[k] = v
+		out[k] = v
 	}
-
-	return ctx.products
+	return out
 }
 
 func (ctx *AttestationContext) addMaterials(materialer Materialer) {

diff --git a/go.mod b/go.mod
@@ -1,6 +1,6 @@
 module github.com/in-toto/go-witness
 
-go 1.19
+go 1.21
 
 require (
 	github.com/digitorus/pkcs7 v0.0.0-20230220124406-51331ccfc40f