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

(bug) Invalid query was not detected by linter nor evaluation #2786

Open
AfikAtashga opened this issue Dec 9, 2024 · 3 comments
Open

(bug) Invalid query was not detected by linter nor evaluation #2786

AfikAtashga opened this issue Dec 9, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@AfikAtashga
Copy link

Observed behaviour

I created a rule, with several targeting queries and variations.
Query: (param eq "someValue or param eq "someValue2" or param eq "someValue3") and secondParam eq "someOtherValue"
I had a missing " which was not detected by linter.
Afterwards, I expected that when evaluation occur it'll return an error for failing to evaluate, but it's just skipped the evaluation of this specific query, and returned the default.

Expected Behavior

I expected the following behavior:

  1. The linter should've fail the validation process because there was a missing "
  2. Because there was an invalid query, it should've return an error, and possibly the default value - but there is a need for some sort of indication for issue.

Steps to reproduce

I created a few go tests the simulate the problem.
The attached .yaml file is passing the linter verification.
feature_flag_reproduce.zip

@AfikAtashga AfikAtashga added bug Something isn't working needs-triage A priority should be added to the issue labels Dec 9, 2024
@thomaspoignant thomaspoignant removed the needs-triage A priority should be added to the issue label Dec 9, 2024
@thomaspoignant
Copy link
Owner

Hello @AfikAtashga and thanks for opening this issue.
As of Today the only validation made by the linter is that we have a non empty string, which I know is not enough.

The rule engine is based on nikunjy/rules and as of Today there is no real way to validate the rules.
I have open an issue to know if there is a better way to validate but for now I got no answer to it.
That being said, I think I can add the initial validation but it won't help with your problem.

When it comes to error, here we are using the defaultRule as default value.

@thomaspoignant thomaspoignant removed their assignment Dec 9, 2024
@thomaspoignant thomaspoignant added enhancement New feature or request and removed bug Something isn't working labels Dec 13, 2024
@thomaspoignant
Copy link
Owner

I've added a basic check in the version v1.40.0, it will not solve your issue but as soon as the rule library has more checks it will works directly after the upgrade.

Copy link
Contributor

github-actions bot commented Jan 2, 2025

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added Stale When an issue is open for too long and removed Stale When an issue is open for too long labels Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants