-
Notifications
You must be signed in to change notification settings - Fork 53
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
Insights Compliance integration (HMS-3829) #2440
Conversation
40435e3
to
c639dcf
Compare
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #2440 +/- ##
==========================================
+ Coverage 75.71% 83.88% +8.17%
==========================================
Files 33 157 +124
Lines 597 17694 +17097
Branches 144 1747 +1603
==========================================
+ Hits 452 14843 +14391
- Misses 139 2830 +2691
- Partials 6 21 +15
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
We could show the OpenSCAP step as fallback, if a customer hasn't set up any policy yet. And then we could show a huge throbbing notice that they can easily create a policy if they want tailoring? |
c639dcf
to
431d960
Compare
src/Components/CreateImageWizard/steps/Compliance/Compliance.tsx
Outdated
Show resolved
Hide resolved
src/test/Components/CreateImageWizard/steps/Compliance/Compliance.test.tsx
Outdated
Show resolved
Hide resolved
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.
This looks awesome so far! Added a few comments that should help with the noise in test outputs.
"radio buttoning" between compliance and Oscap will be great in eliminating the overlap
src/Components/CreateImageWizard/steps/Oscap/OscapProfileInformation.tsx
Outdated
Show resolved
Hide resolved
src/Components/CreateImageWizard/steps/Oscap/OscapProfileInformation.tsx
Outdated
Show resolved
Hide resolved
7f40f8c
to
f0a06a2
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.
Looks great with the radio! Added a few nitpicks.
src/Components/CreateImageWizard/steps/Oscap/OscapProfileInformation.tsx
Outdated
Show resolved
Hide resolved
10bc820
to
87a4608
Compare
/retest |
1 similar comment
/retest |
is currently an expected failure. @jrusz is looking into it. Since the PR failed on the final cleanup, pr_check shouldn't need any adjustment. |
87a4608
to
7eaaedc
Compare
5479dbf
to
54bfded
Compare
Policies to query the list of policies, and policy to get the details of a single policy.
Includes new compliance openscap customisation.
Merges all the compliance related state into a single compliance object.
Reuses the existing oscap profile information.
Let's reuse this component for the compliance step.
The Insights compliance support reuses most of the existing OpenSCAP step. Depending on the state of the feature flag, it will show radio buttons allowing users to switch between regular openscap and Insights compliance.
The title is changed depending on the flag, to make it easier to change the IQE tests.
When switching between openscap and insights compliance, clear the data so the profile information isn't showing anymore.
54bfded
to
aca2bd3
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.
Thank you! Looks and works great ✨
/retest |
The current state:
compliance-radio-.2.mp4
One thing worth discussing is how we want to expose the compliance step. Should we show it alongside the existing openscap step? Should the steps be merged?
Since it would only be in preview, how about we just show both steps for now, knowing that they might conflict?