-
Notifications
You must be signed in to change notification settings - Fork 110
feat: implement mindev test cli command and rule loading by name #6551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
8f49124
2c9d69f
0b195db
96953ef
1671dbe
faee005
873de46
ff1ceff
1991991
f7beb5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| // SPDX-FileCopyrightText: Copyright 2026 The Minder Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Package test provides the test command for mindev | ||
| package test | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
||
| "github.com/mindersec/minder/pkg/ruletest" | ||
| ) | ||
|
|
||
| // CmdTest returns the test cobra command | ||
| func CmdTest() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "test [paths...]", | ||
| Short: "Run Minder rule tests", | ||
| Long: "Run Starlark-based tests for Minder rules. Each path may be a file or directory. " + | ||
| "If no paths are provided, tests the current directory.", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if len(args) == 0 { | ||
| args = []string{"."} | ||
| } | ||
|
|
||
| runner := ruletest.NewRunner() | ||
| results, err := runner.RunPaths(args) | ||
| if err != nil { | ||
| cmd.PrintErrf("Error(s) running tests:\n%v\n", err) | ||
| } | ||
|
|
||
| if len(results) == 0 { | ||
| cmd.Printf("No tests found\n") | ||
| return err | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| hasFailures := false | ||
| for _, res := range results { | ||
| if len(res.Failures) > 0 { | ||
| hasFailures = true | ||
| cmd.Printf("FAIL: %s\n", res.Name) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you'll want the filename here as well as the test name, particularly if the same test name could occur in multiple |
||
| for _, f := range res.Failures { | ||
| cmd.Printf(" - %s\n", f) | ||
| } | ||
| } else { | ||
| cmd.Printf("PASS: %s\n", res.Name) | ||
| } | ||
| } | ||
|
|
||
| if hasFailures { | ||
| return fmt.Errorf("one or more tests failed") | ||
| } | ||
|
|
||
| return err | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }, | ||
| } | ||
|
|
||
| return cmd | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,36 +21,29 @@ import ( | |||||||||||||
| tkv1 "github.com/mindersec/minder/pkg/testkit/v1" | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| func builtinEval( | ||||||||||||||
| func (tr *testCaseRunner) builtinEval( | ||||||||||||||
| thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple, | ||||||||||||||
| ) (starlark.Value, error) { | ||||||||||||||
| var rulePath string | ||||||||||||||
| var ruleName string | ||||||||||||||
| var entityDict *starlark.Dict | ||||||||||||||
| var profileDict *starlark.Dict | ||||||||||||||
| var mockHttpDict *starlark.Dict | ||||||||||||||
| var mockFSDict *starlark.Dict | ||||||||||||||
|
|
||||||||||||||
| err := starlark.UnpackArgs("eval", args, kwargs, | ||||||||||||||
| "rule", &rulePath, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict) | ||||||||||||||
| "rule", &ruleName, "entity?", &entityDict, "profile?", &profileDict, "mock_http?", &mockHttpDict, "mock_fs?", &mockFSDict) | ||||||||||||||
| if err != nil { | ||||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if !filepath.IsAbs(rulePath) { | ||||||||||||||
| callerFrame := thread.CallFrame(1) | ||||||||||||||
| if callerFile := callerFrame.Pos.Filename(); callerFile != "" { | ||||||||||||||
| rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) | ||||||||||||||
| } | ||||||||||||||
| mockFSMap, err := parseMockFSDict(mockFSDict) | ||||||||||||||
| if err != nil { | ||||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| decoder, closer := fileconvert.DecoderForFile(rulePath) | ||||||||||||||
| if decoder == nil { | ||||||||||||||
| return nil, fmt.Errorf("error opening file: %s", rulePath) | ||||||||||||||
| } | ||||||||||||||
| defer closer.Close() | ||||||||||||||
|
|
||||||||||||||
| rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) | ||||||||||||||
| rt, err := tr.loadRuleTypeFallback(ruleName, thread) | ||||||||||||||
| if err != nil { | ||||||||||||||
| return nil, fmt.Errorf("failed to parse rule type: %w", err) | ||||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| profileMap, err := dictToGoMap(profileDict) | ||||||||||||||
|
|
@@ -75,7 +68,11 @@ func builtinEval( | |||||||||||||
|
|
||||||||||||||
| ctx := context.Background() | ||||||||||||||
|
|
||||||||||||||
| tk := tkv1.NewTestKit(tkv1.WithHandlerFunc(mockHandler.ServeHTTP)) | ||||||||||||||
| tkOpts := []tkv1.Option{tkv1.WithHandlerFunc(mockHandler.ServeHTTP)} | ||||||||||||||
| if len(mockFSMap) > 0 { | ||||||||||||||
| tkOpts = append(tkOpts, tkv1.WithMockFS(mockFSMap)) | ||||||||||||||
| } | ||||||||||||||
| tk := tkv1.NewTestKit(tkOpts...) | ||||||||||||||
|
|
||||||||||||||
| rte, err := rtengine.NewRuleTypeEngine(ctx, rt, tk) | ||||||||||||||
| if err != nil { | ||||||||||||||
|
|
@@ -121,6 +118,51 @@ func formatEvalResult(evalErr error) *starlark.Dict { | |||||||||||||
| return result | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func parseMockFSDict(mockFSDict *starlark.Dict) (map[string]string, error) { | ||||||||||||||
| mockFSMap := make(map[string]string) | ||||||||||||||
| if mockFSDict != nil { | ||||||||||||||
| for _, item := range mockFSDict.Items() { | ||||||||||||||
| k, v := item[0], item[1] | ||||||||||||||
| ks, ok1 := k.(starlark.String) | ||||||||||||||
| vs, ok2 := v.(starlark.String) | ||||||||||||||
| if !ok1 || !ok2 { | ||||||||||||||
| return nil, fmt.Errorf("mock_fs keys and values must be strings") | ||||||||||||||
| } | ||||||||||||||
| mockFSMap[string(ks)] = string(vs) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| return mockFSMap, nil | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (tr *testCaseRunner) loadRuleTypeFallback(ruleName string, thread *starlark.Thread) (*minderv1.RuleType, error) { | ||||||||||||||
| if tr.ruleTypes != nil { | ||||||||||||||
| if ruleType, ok := tr.ruleTypes[ruleName]; ok { | ||||||||||||||
| return ruleType, nil | ||||||||||||||
| } | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that you're storing
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| rulePath := ruleName | ||||||||||||||
| if !filepath.IsAbs(rulePath) { | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this check? What would happen if the rulename started with |
||||||||||||||
| callerFrame := thread.CallFrame(1) | ||||||||||||||
| if callerFile := callerFrame.Pos.Filename(); callerFile != "" { | ||||||||||||||
| rulePath = filepath.Join(filepath.Dir(callerFile), rulePath) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| decoder, closer := fileconvert.DecoderForFile(rulePath) | ||||||||||||||
| if decoder == nil { | ||||||||||||||
| return nil, fmt.Errorf("rule %q not found in loaded rule types and no file found at path %s", ruleName, rulePath) | ||||||||||||||
| } | ||||||||||||||
| defer closer.Close() | ||||||||||||||
|
|
||||||||||||||
| rt, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) | ||||||||||||||
| if err != nil { | ||||||||||||||
| return nil, fmt.Errorf("failed to parse rule type: %w", err) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return rt, nil | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm worried about this code being hit unexpectedly, and things "working" when the primary path is broken. Alternatively, I'm worried that we have no coverage for this path, and that it's code that seems like it does something but doesn't. Is there a good reason to keep this code vs removing it and updating the test cases? |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| //nolint:gocyclo // this is a simple switch over many entity types | ||||||||||||||
| func mapToProto(entityType string, entityMap map[string]any) (proto.Message, error) { | ||||||||||||||
| if len(entityMap) == 0 { | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,12 +95,36 @@ func TestBuiltinEval_InvalidArgs(t *testing.T) { | |
| kwargs: []starlark.Tuple{{starlark.String("entity"), starlark.String("not a dict")}}, | ||
| wantErr: "got string, want dict", | ||
| }, | ||
| { | ||
| name: "invalid mock_fs type", | ||
| args: starlark.Tuple{starlark.String("rule.yaml")}, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I know we don't really use this here, but this should be |
||
| kwargs: []starlark.Tuple{ | ||
| {starlark.String("mock_fs"), starlark.String("not a dict")}, | ||
| }, | ||
| wantErr: "got string, want dict", | ||
| }, | ||
| { | ||
| name: "mock_fs non-string keys", | ||
| args: starlark.Tuple{starlark.String("rule.yaml")}, | ||
| kwargs: []starlark.Tuple{ | ||
| { | ||
| starlark.String("mock_fs"), | ||
| func() *starlark.Dict { | ||
| d := starlark.NewDict(1) | ||
| _ = d.SetKey(starlark.MakeInt(1), starlark.String("content")) | ||
| return d | ||
| }(), | ||
| }, | ||
| }, | ||
| wantErr: "mock_fs keys and values must be strings", | ||
| }, | ||
|
evankanderson marked this conversation as resolved.
|
||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| _, err := builtinEval(thread, starlark.NewBuiltin("eval", builtinEval), tt.args, tt.kwargs) | ||
| tr := &testCaseRunner{} | ||
|
evankanderson marked this conversation as resolved.
|
||
| _, err := tr.builtinEval(thread, starlark.NewBuiltin("eval", tr.builtinEval), tt.args, tt.kwargs) | ||
| if err == nil { | ||
| t.Fatal("expected error, got nil") | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.