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

RHINENG-1197: fix meta links #1277

Merged
merged 11 commits into from
Aug 4, 2023
Merged

Conversation

MichaelMraka
Copy link
Collaborator

Secure Coding Practices Checklist GitHub Link

Secure Coding Checklist

  • Input Validation
  • Output Encoding
  • Authentication and Password Management
  • Session Management
  • Access Control
  • Cryptographic Practices
  • Error Handling and Logging
  • Data Protection
  • Communication Security
  • System Configuration
  • Database Security
  • File Management
  • Memory Management
  • General Coding Practices

@jira-linking
Copy link

jira-linking bot commented Aug 1, 2023

Referenced Jiras:
https://issues.redhat.com/browse/RHINENG-1197

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 84.82% and project coverage change: -0.08% ⚠️

Comparison is base (1790976) 60.54% compared to head (48fc1ea) 60.47%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1277      +/-   ##
==========================================
- Coverage   60.54%   60.47%   -0.08%     
==========================================
  Files         106      106              
  Lines        6631     6646      +15     
==========================================
+ Hits         4015     4019       +4     
- Misses       2085     2096      +11     
  Partials      531      531              
Flag Coverage Δ
unittests 60.47% <84.82%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
tasks/caches/refresh_packages_caches.go 0.00% <0.00%> (ø)
manager/controllers/packages.go 57.14% <50.00%> (-6.10%) ⬇️
manager/controllers/utils.go 78.69% <88.09%> (-1.03%) ⬇️
base/utils/config.go 64.84% <100.00%> (+0.21%) ⬆️
manager/controllers/advisories.go 67.79% <100.00%> (ø)
manager/controllers/advisories_export.go 48.97% <100.00%> (ø)
manager/controllers/advisory_systems.go 42.85% <100.00%> (ø)
manager/controllers/advisory_systems_export.go 41.86% <100.00%> (ø)
manager/controllers/baseline_systems.go 48.59% <100.00%> (ø)
manager/controllers/baselines.go 83.78% <100.00%> (ø)
... and 10 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MichaelMraka
Copy link
Collaborator Author

/retest

1 similar comment
@psegedy
Copy link
Member

psegedy commented Aug 2, 2023

/retest

Comment on lines -32 to -40
var validSystemProfileFilters = map[string]bool{
"sap_sids": true,
"sap_system": true,
"mssql": true,
"mssql->version": true,
"ansible": true,
"ansible->controller_version": true,
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need this map to allow only certain keywords for filter? We should be able to return 400 if user tries something like filter[system_profile][asdf]=asdf but I don't know if we can do it with the old code or not

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@psegedy psegedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should make sure we don't break any checks for invalid filters if we have them now, I'd rely on unit tests and QE to provide an answer whether the functionality is correct or not

if it doesn't break any current checks for invalid filters, then LGTM

PR checks are broken until Brandon and app-sre make necessary changes

@psegedy
Copy link
Member

psegedy commented Aug 2, 2023

/retest

@MichaelMraka
Copy link
Collaborator Author

There's couple of invalid filter tests, e.g.
TestSystemsFilterNotExisting
TestSystemsIDsFilterNotExisting

@MichaelMraka MichaelMraka merged commit 70902dc into RedHatInsights:master Aug 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants