Skip to content

Commit 490d3c7

Browse files
author
Cole Kennedy
committed
feat: add continue-on-error flags with robust error handling
- Add flags to continue execution when the wrapped command succeeds: - --continue-on-attestor-error for attestor-related errors - --continue-on-infra-error for infrastructure errors - --continue-on-errors for both error types - Create proper error type system for error classification - Implement robust error type detection and propagation - Improve error handling throughout command execution - Add comprehensive unit tests for error handling
1 parent 449c4e4 commit 490d3c7

File tree

6 files changed

+655
-219
lines changed

6 files changed

+655
-219
lines changed

cmd/run.go

+170-12
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/in-toto/go-witness/log"
3131
"github.com/in-toto/go-witness/registry"
3232
"github.com/in-toto/go-witness/timestamp"
33+
"github.com/in-toto/witness/internal/errors"
3334
"github.com/in-toto/witness/options"
3435
"github.com/spf13/cobra"
3536
)
@@ -63,15 +64,91 @@ func RunCmd() *cobra.Command {
6364
return cmd
6465
}
6566

67+
// isAttestorError determines if an error is attestor-related
68+
func isAttestorError(err error) bool {
69+
return errors.IsAttestorError(err)
70+
}
71+
72+
// handleInfraError handles infrastructure operation errors based on continue flags
73+
// Returns true if execution should continue, and the error if it should be tracked
74+
func handleInfraError(ro options.RunOptions, err error, operationDesc string, commandSucceeded bool) (bool, error) {
75+
if !commandSucceeded {
76+
return false, nil
77+
}
78+
79+
// Wrap the error with infrastructure error type
80+
infraErr := errors.NewInfrastructureError(operationDesc, err)
81+
82+
if ro.ContinueOnAllErrors {
83+
log.Warnf("Failed to %s: %v", operationDesc, err)
84+
log.Warnf("Continuing due to --continue-on-errors flag")
85+
return true, infraErr
86+
} else if ro.ContinueOnInfraError {
87+
log.Warnf("Failed to %s: %v", operationDesc, err)
88+
log.Warnf("Continuing due to --continue-on-infra-error flag")
89+
return true, infraErr
90+
}
91+
92+
return false, nil
93+
}
94+
95+
// handleErrorWithContinueFlags applies the appropriate error handling logic based on flags
96+
// Returns true if execution should continue, false if the error should be returned
97+
func handleErrorWithContinueFlags(ro options.RunOptions, err error, commandSucceeded bool) (bool, error, error) {
98+
var infraError, attestorError error
99+
100+
// If command didn't succeed or no continue flags are set, don't continue
101+
if !commandSucceeded {
102+
return false, nil, nil
103+
}
104+
105+
// Check if the all-errors flag is set, which takes precedence
106+
if ro.ContinueOnAllErrors {
107+
log.Warnf("Encountered error: %v", err)
108+
log.Warnf("Continuing due to --continue-on-errors flag")
109+
110+
// Still classify the error for summary purposes
111+
if isAttestorError(err) {
112+
attestorError = err
113+
} else {
114+
// Default to infrastructure error if not an attestor error
115+
infraError = errors.NewInfrastructureError("run command", err)
116+
}
117+
return true, infraError, attestorError
118+
}
119+
120+
// Check specific error type flags
121+
isAttestor := isAttestorError(err)
122+
if isAttestor && ro.ContinueOnAttestorError {
123+
log.Warnf("Encountered attestor error: %v", err)
124+
log.Warnf("Continuing due to --continue-on-attestor-error flag")
125+
attestorError = err
126+
return true, infraError, attestorError
127+
} else if !isAttestor && ro.ContinueOnInfraError {
128+
log.Warnf("Encountered infrastructure error: %v", err)
129+
log.Warnf("Continuing due to --continue-on-infra-error flag")
130+
infraError = errors.NewInfrastructureError("run command", err)
131+
return true, infraError, attestorError
132+
}
133+
134+
// No applicable flag was set, don't continue
135+
return false, nil, nil
136+
}
137+
66138
func runRun(ctx context.Context, ro options.RunOptions, args []string, signers ...cryptoutil.Signer) error {
67139
if len(signers) > 1 {
68-
return fmt.Errorf("only one signer is supported")
140+
return errors.NewInfrastructureError("signer validation", fmt.Errorf("only one signer is supported"))
69141
}
70142

71143
if len(signers) == 0 {
72-
return fmt.Errorf("no signers found")
144+
return errors.NewInfrastructureError("signer validation", fmt.Errorf("no signers found"))
73145
}
74146

147+
// Track if wrapped command succeeded but we had errors
148+
var commandSucceeded bool
149+
var infraError error
150+
var attestorError error
151+
75152
timestampers := []timestamp.Timestamper{}
76153
for _, url := range ro.TimestampServers {
77154
timestampers = append(timestampers, timestamp.NewTimestamper(timestamp.TimestampWithUrl(url)))
@@ -101,7 +178,7 @@ func runRun(ctx context.Context, ro options.RunOptions, args []string, signers .
101178
if !duplicate {
102179
attestor, err := attestation.GetAttestor(a)
103180
if err != nil {
104-
return fmt.Errorf("failed to create attestor: %w", err)
181+
return errors.NewAttestorError(a, fmt.Errorf("failed to create attestor: %w", err))
105182
}
106183
attestors = append(attestors, attestor)
107184
}
@@ -115,23 +192,23 @@ func runRun(ctx context.Context, ro options.RunOptions, args []string, signers .
115192

116193
attestor, err := registry.SetOptions(attestor, setters...)
117194
if err != nil {
118-
return fmt.Errorf("failed to set attestor option for %v: %w", attestor.Type(), err)
195+
return errors.NewAttestorError(attestor.Name(), fmt.Errorf("failed to set attestor option for %v: %w", attestor.Type(), err))
119196
}
120197
}
121198

122199
var roHashes []cryptoutil.DigestValue
123200
for _, hashStr := range ro.Hashes {
124201
hash, err := cryptoutil.HashFromString(hashStr)
125202
if err != nil {
126-
return fmt.Errorf("failed to parse hash: %w", err)
203+
return errors.NewInfrastructureError("parse hash", fmt.Errorf("failed to parse hash: %w", err))
127204
}
128205
roHashes = append(roHashes, cryptoutil.DigestValue{Hash: hash, GitOID: false})
129206
}
130207

131208
for _, dirHashGlobItem := range ro.DirHashGlobs {
132209
_, err := glob.Compile(dirHashGlobItem)
133210
if err != nil {
134-
return fmt.Errorf("failed to compile glob: %v", err)
211+
return errors.NewInfrastructureError("compile glob", fmt.Errorf("failed to compile glob: %v", err))
135212
}
136213
}
137214

@@ -149,14 +226,49 @@ func runRun(ctx context.Context, ro options.RunOptions, args []string, signers .
149226
),
150227
witness.RunWithTimestampers(timestampers...),
151228
)
229+
230+
// Check if command ran successfully
231+
if len(args) > 0 { // Only check for command success if a command was run
232+
for _, result := range results {
233+
if result.AttestorName == "command-run" {
234+
// Command completed and we have the attestation, so it succeeded
235+
commandSucceeded = true
236+
break
237+
}
238+
}
239+
} else {
240+
// If no command was specified, we're just collecting attestations
241+
// In this case, treat as if command succeeded for flag purposes
242+
commandSucceeded = true
243+
}
244+
152245
if err != nil {
153-
return err
246+
// Apply error handling logic based on flags
247+
shouldContinue, newInfraErr, newAttestorErr := handleErrorWithContinueFlags(ro, err, commandSucceeded)
248+
if shouldContinue {
249+
// Update the error tracking variables
250+
if newInfraErr != nil {
251+
infraError = newInfraErr
252+
}
253+
if newAttestorErr != nil {
254+
attestorError = newAttestorErr
255+
}
256+
} else {
257+
// If we shouldn't continue, return the error
258+
return err
259+
}
154260
}
155261

156262
for _, result := range results {
157263
signedBytes, err := json.Marshal(&result.SignedEnvelope)
158264
if err != nil {
159-
return fmt.Errorf("failed to marshal envelope: %w", err)
265+
shouldContinue, newInfraErr := handleInfraError(ro, err, "marshal envelope", commandSucceeded)
266+
if shouldContinue {
267+
infraError = newInfraErr
268+
continue // Skip to next result
269+
} else {
270+
return fmt.Errorf("failed to marshal envelope: %w", err)
271+
}
160272
}
161273

162274
// TODO: Find out explicit way to describe "prefix" in CLI options
@@ -167,22 +279,68 @@ func runRun(ctx context.Context, ro options.RunOptions, args []string, signers .
167279

168280
out, err := loadOutfile(outfile)
169281
if err != nil {
170-
return fmt.Errorf("failed to open out file: %w", err)
282+
shouldContinue, newInfraErr := handleInfraError(ro, err, fmt.Sprintf("open out file %s", outfile), commandSucceeded)
283+
if shouldContinue {
284+
infraError = newInfraErr
285+
continue // Skip to next result
286+
} else {
287+
return fmt.Errorf("failed to open out file: %w", err)
288+
}
171289
}
172290
defer out.Close()
173291

174292
if _, err := out.Write(signedBytes); err != nil {
175-
return fmt.Errorf("failed to write envelope to out file: %w", err)
293+
shouldContinue, newInfraErr := handleInfraError(ro, err, fmt.Sprintf("write envelope to file %s", outfile), commandSucceeded)
294+
if shouldContinue {
295+
infraError = newInfraErr
296+
continue // Skip to next result
297+
} else {
298+
return fmt.Errorf("failed to write envelope to out file: %w", err)
299+
}
176300
}
177301

178302
if ro.ArchivistaOptions.Enable {
179303
archivistaClient := archivista.New(ro.ArchivistaOptions.Url)
180-
if gitoid, err := archivistaClient.Store(ctx, result.SignedEnvelope); err != nil {
181-
return fmt.Errorf("failed to store artifact in archivista: %w", err)
304+
gitoid, err := archivistaClient.Store(ctx, result.SignedEnvelope)
305+
if err != nil {
306+
shouldContinue, newInfraErr := handleInfraError(ro, err, "store artifact in archivista", commandSucceeded)
307+
if shouldContinue {
308+
infraError = newInfraErr
309+
} else {
310+
return fmt.Errorf("failed to store artifact in archivista: %w", err)
311+
}
182312
} else {
183313
log.Infof("Stored in archivista as %v\n", gitoid)
184314
}
185315
}
186316
}
317+
318+
// Display summary warnings if we had errors but continued
319+
if commandSucceeded && (attestorError != nil || infraError != nil) {
320+
// Show a combined message if we used the combined flag
321+
if ro.ContinueOnAllErrors {
322+
log.Warnf("Command completed successfully, but encountered errors")
323+
if attestorError != nil {
324+
log.Warnf("Some attestations may be missing")
325+
}
326+
if infraError != nil {
327+
log.Warnf("Some attestation functionality may have been compromised")
328+
}
329+
} else {
330+
// Show specific messages for specific flags
331+
if attestorError != nil && ro.ContinueOnAttestorError {
332+
log.Warnf("Command completed successfully, but encountered attestor errors")
333+
log.Warnf("Some attestations may be missing")
334+
}
335+
336+
if infraError != nil && ro.ContinueOnInfraError {
337+
log.Warnf("Command completed successfully, but encountered infrastructure errors")
338+
log.Warnf("Some attestation functionality may have been compromised")
339+
}
340+
}
341+
342+
// We had errors but continued, so return success
343+
return nil
344+
}
187345
return nil
188346
}

0 commit comments

Comments
 (0)