Skip to content

Commit 1e67bb4

Browse files
committed
refactor: use errors.Is instead of string parsing in isEngineIncompatibleError
Replace fragile string parsing with errors.Is check using the exported ErrNoDriver sentinel error from the constraint framework. This is more robust and won't break if error messages change in the framework. Signed-off-by: Sertac Ozercan <[email protected]>
1 parent c9fbc95 commit 1e67bb4

File tree

3 files changed

+13
-38
lines changed

3 files changed

+13
-38
lines changed

pkg/gator/bench/bench.go

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@ package bench
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"runtime"
7-
"strings"
88
"sync"
99
"sync/atomic"
1010
"time"
1111

1212
"github.com/open-policy-agent/frameworks/constraint/pkg/apis"
1313
constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client"
1414
"github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/rego"
15+
clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors"
1516
"github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews"
1617
"github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel"
1718
"github.com/open-policy-agent/gatekeeper/v3/pkg/gator/reader"
@@ -346,20 +347,8 @@ func isEngineIncompatibleError(err error) bool {
346347
if err == nil {
347348
return false
348349
}
349-
errStr := err.Error()
350-
// CEL engine returns this error when no CEL code block is present
351-
if strings.Contains(errStr, "no CEL code") ||
352-
strings.Contains(errStr, "missing CEL source") ||
353-
strings.Contains(errStr, "No language driver is installed") ||
354-
strings.Contains(errStr, "no validator for driver") {
355-
return true
356-
}
357-
// Rego engine returns this error when no Rego code block is present
358-
if strings.Contains(errStr, "no Rego code") ||
359-
strings.Contains(errStr, "missing Rego source") {
360-
return true
361-
}
362-
return false
350+
351+
return errors.Is(err, clienterrors.ErrNoDriver)
363352
}
364353

365354
// runSequentialBenchmark runs the benchmark sequentially (single-threaded).

pkg/gator/bench/bench_test.go

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package bench
22

33
import (
44
"bytes"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"strings"
89
"testing"
10+
11+
clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors"
912
)
1013

1114
func TestRun_MissingInputs(t *testing.T) {
@@ -667,33 +670,18 @@ func TestIsEngineIncompatibleError(t *testing.T) {
667670
expected: false,
668671
},
669672
{
670-
name: "no CEL code error",
671-
err: &testError{msg: "no CEL code found"},
672-
expected: true,
673-
},
674-
{
675-
name: "no language driver error",
676-
err: &testError{msg: "No language driver is installed"},
677-
expected: true,
678-
},
679-
{
680-
name: "no Rego code error",
681-
err: &testError{msg: "no Rego code found"},
682-
expected: true,
683-
},
684-
{
685-
name: "missing CEL source error",
686-
err: &testError{msg: "missing CEL source"},
673+
name: "ErrNoDriver directly",
674+
err: clienterrors.ErrNoDriver,
687675
expected: true,
688676
},
689677
{
690-
name: "missing Rego source error",
691-
err: &testError{msg: "missing Rego source"},
678+
name: "ErrNoDriver wrapped",
679+
err: fmt.Errorf("constraint template error: %w", clienterrors.ErrNoDriver),
692680
expected: true,
693681
},
694682
{
695-
name: "no validator for driver error",
696-
err: &testError{msg: "no validator for driver"},
683+
name: "ErrNoDriver double wrapped",
684+
err: fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", clienterrors.ErrNoDriver)),
697685
expected: true,
698686
},
699687
{

pkg/gator/bench/compare_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,5 +419,3 @@ func TestFormatComparison_WithRegression(t *testing.T) {
419419
}
420420
}
421421
}
422-
423-

0 commit comments

Comments
 (0)