-
Notifications
You must be signed in to change notification settings - Fork 30
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(APIv2): RHINENG-11772 restrict search/sort to specific parents #2198
fix(APIv2): RHINENG-11772 restrict search/sort to specific parents #2198
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2198 +/- ##
=======================================
Coverage 98.83% 98.83%
=======================================
Files 232 232
Lines 5066 5074 +8
=======================================
+ Hits 5007 5015 +8
Misses 59 59 ☔ View full report in Codecov by Sentry. |
71dd6e7
to
24d6606
Compare
24d6606
to
92b0476
Compare
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.
LGTM 👍
sort_params_v2(V2::Report) | ||
search_params_v2(V2::Report) | ||
sort_params_v2(V2::Report, except: %i[os_major_version]) | ||
search_params_v2(V2::Report, except: %i[os_major_version with_reported_systems]) |
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.
@@ -149,8 +149,8 @@ | |||
operationId 'PolicySystems' | |||
content_types | |||
pagination_params_v2 | |||
sort_params_v2(V2::System) | |||
search_params_v2(V2::System) | |||
sort_params_v2(V2::System, except: %i[os_major_version]) |
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.
Sorting is still possible:
!master ~/Documents/WORK/IQE/iqe-compliance-plugin> curl -XGET -u $CREDENTIALS -H 'Content-Type: application/json' --proxy "squid.corp.redhat.com:3128" "https://console.stage.redhat.com/api/compliance/v2/policies/a5a8a68d-ecb5-403d-8356-d73f76133c48/systems?sort_by=os_major_version" | jq
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3198 100 3198 0 0 1600 0 0:00:01 0:00:01 --:--:-- 1600
{
"data": [
{
"id": "589b3c21-52cf-4a8e-9f73-5062b9fbe69a",
"display_name": "demo_passing_system_pci-dss_0",
"groups": [],
"culled_timestamp": "2024-09-24T15:24:40.030Z",
"stale_timestamp": "2024-09-10T15:24:40.030Z",
"stale_warning_timestamp": "2024-09-17T15:24:40.030Z",
"updated": "2024-09-09T10:24:40.030Z",
"insights_id": "e41a8b97-b3ad-4041-9727-d340dfb860d4",
As we are heavily limited by scoped_search, I chose the validation feature to restrict searching to specific parent hierarchies. The solution uses the
except_parents
andonly_parents
keywords that we already have in our YAML-generated tests.In order to make the validator access the list of current parents, they have been moved into a thread-current context variable. This is not the nicest way, but any other solution would unfortunately involve monkey-patching of
scoped_search
. The second hack is that the error message from the validation is regexp-replaced to make sure that the API response gets the correct message referring to the context and not the invalid value.Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist