Skip to content
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

glob.match without delimiters #4923

Closed
vinhph0906 opened this issue Jul 21, 2022 · 12 comments
Closed

glob.match without delimiters #4923

vinhph0906 opened this issue Jul 21, 2022 · 12 comments
Labels

Comments

@vinhph0906
Copy link
Contributor

Short description

I used glob.match to valid path. However, without delimiters it will set default to ["."]. Therefore, I fail the test case below:

Steps To Reproduce

My rego looks like:
result := glob.match("test*",[],"test.txt")

Expected behavior

I expected output is "result": true but it is "result": false.

@vinhph0906 vinhph0906 added the bug label Jul 21, 2022
@srenatus
Copy link
Contributor

I'm not sure I follow. From my understanding of globs, test.txt isn't matched by test* with delimiter . -- test.* would be a glob that matches test.txt with delimiter . 🤔

@vinhph0906
Copy link
Contributor Author

if globs with delimiter . is as you understand. But if globs without delimiter here, its matched.

@srenatus
Copy link
Contributor

💡 Got it. So you'd expect glob.match("test*", [], "test.txt") to be true, but since if delimiters is empty, OPA will use ".", there's no way to have it use no delimiters.

Hmm I wonder how we could pull that off without breaking backwards-compatibility. I think it would have to be a new built-in, like glob.match_no_delim(pattern, text)? What do you think?

@vinhph0906
Copy link
Contributor Author

Can I contribute to this function?

@anderseknert
Copy link
Member

Would a simple regex.match not do the trick then? 🤔

@vinhph0906
Copy link
Contributor Author

In the past, I have used regex for my rego. But I realized golang is not good with regex. You can see results in this benchmark. Then I use glob for this and my application latency goes down.

@srenatus
Copy link
Contributor

Can I contribute to this function?

Yeah sure, contributions are always welcome. I'm still not sure what the best name is here... 🤔 But we can discuss that later, too.

@anderseknert
Copy link
Member

Fair enough @vinhph0906! Perhaps we could use null as the delimiter argument to say “use no delimiter” over adding another builtin function?

@vinhph0906
Copy link
Contributor Author

In the glob.match function, delimiters is array string type. So I can't use null as delimiters argument.

@srenatus
Copy link
Contributor

Not right now, but I think Anders meant that as a proposal for how to enhance that built-in instead of creating a new one. It's a good idea, I think.

@vinhph0906
Copy link
Contributor Author

delimiters is StringSlice type, but in code, it is used as RuneSlice. I think we can modify the hander error if it is ArrayString we say “use no delimiter”. What do you think?

@srenatus
Copy link
Contributor

Yeah we can inspect the argument. There are examples of similar functions that work on multiple types, like count, I suppose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants