support glob.match without delimiters #4923 #4933
Conversation
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
docs/content/policy-reference.md
Outdated
| @@ -337,6 +337,7 @@ The following table shows examples of how ``glob.match`` works: | |||
| | -------- | ---------- | ----------- | | |||
| | ``output := glob.match("*.github.com", [], "api.github.com")`` | ``true`` | A glob with the default ``["."]`` delimiter. | | |||
| | ``output := glob.match("*.github.com", [], "api.cdn.github.com")`` | ``false`` | A glob with the default ``["."]`` delimiter. | | |||
| | ``output := glob.match("*.github.com", null, "api.cdn.github.com")`` | ``true`` | A glob without delimiter. | | |||
There was a problem hiding this comment.
Can we make the example a little more obvious, please? Maybe match with pattern *hub.com?
| @@ -18,16 +18,20 @@ func builtinGlobMatch(a, b, c ast.Value) (ast.Value, error) { | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| var delimiters []rune | |||
| switch b.(type) { | |||
There was a problem hiding this comment.
I think we need a default case here, otherwise it'll accept any input, too
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
|
I have changed as your comment above. |
| @@ -31,6 +31,8 @@ func builtinGlobMatch(a, b, c ast.Value) (ast.Value, error) { | |||
| if len(delimiters) == 0 { | |||
| delimiters = []rune{'.'} | |||
| } | |||
| default: | |||
There was a problem hiding this comment.
Sorry this isn't what I meant -- if it's not null and not an array, it might be something like
123true"foo"
i.e. some other type ofValue. We should not start to silently ignore that bad input, so let's return some sort of OperandError here, please.
| @@ -2604,7 +2604,10 @@ var GlobMatch = &Builtin{ | |||
| Decl: types.NewFunction( | |||
| types.Args( | |||
| types.Named("pattern", types.S), | |||
| types.Named("delimiters", types.NewArray(nil, types.S)).Description("glob pattern delimiters, e.g. `[\".\", \":\"]`, defaults to `[\".\"]` if unset."), | |||
| types.Named("delimiters", types.NewAny( | |||
There was a problem hiding this comment.
I think this line only gives a call by type ArrayString or null. So any types else have raised error before function.
There was a problem hiding this comment.
That's for static inputs. There are still inputs whose type is only known at runtime. For example glob.match(input.m, input.d, input.x)
There was a problem hiding this comment.
I have made a change to return OperandError with default.
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
| sort_bindings: true | ||
| want_result: | ||
| - x: | ||
| - true |
There was a problem hiding this comment.
Would you mind adding one case like
glob.match("foo*", null, "foo.bar", x)just to make the "no delimiters" case more obvious?
There was a problem hiding this comment.
I have added another test case to more clearly about this.
Signed-off-by: vinhph0906 <vinhph0906@gmail.com>
|
urm there's something I hadn't thought about -- wasm! glob.match is natively implemented, i.e., there's C(++) code backing that built-in function in our wasm modules... that will need to learn about |
|
So I jumped ahead a bit -- I think this is what we need: srenatus@32ff3ee -- but I'm still waiting for the CI run over here. |
|
Another thing to note is that glob.match calls without a delimiter end up not getting tracked in the rule indexer. That's OK for now, it's just something to keep in mind, and perhaps revisit later. |
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
|
Would you mind cherry-picking this commit? srenatus@21beae4 it should resolve the wasm parts of this, and let us merge the enhancement. :) |
wasm/glob: allow for NULL delimiters argument
|
somehow it fails the tests :(. What I can do to fix this? |
|
Annoying, but unrelated flakey test. I'll merge this later |
As discussed at #4923, I have changed the code to support glob.match without delimiter, by using delimiter as
null.