-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix(policy): DSP-54 make MatchSubjectMappings operator agnostic #1658
base: main
Are you sure you want to change the base?
Conversation
where += "((" + hasField + " AND " + hasValue + " AND " + hasInOperator + ")" + | ||
" OR " + | ||
"(" + hasField + " AND NOT " + hasValue + " AND " + hasNotInOperator + "))" | ||
// Parses the json and matches the row if the selector exists: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't touch this RPC to avoid merge conflicts, but perhaps you could remove this check and move it to the proto validation as part of this effort? https://github.com/opentdf/platform/pull/1658/files#diff-79178ab55805aac9e744330a45936ecde9e39baef7e8838764209f44b6a3f862R535 Similar to the work just done in GetAttributesByValueFQNs: #1657
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we can ignore a check for values now that this route only drives based off selectors (explicitly not values after your current work here): https://github.com/opentdf/platform/pull/1658/files#diff-79178ab55805aac9e744330a45936ecde9e39baef7e8838764209f44b6a3f862R542
resolves: #1499