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

feat(evaluate): add observation details #540

Merged
merged 14 commits into from
Jul 26, 2024

Conversation

meganwolf0
Copy link
Collaborator

@meganwolf0 meganwolf0 commented Jul 15, 2024

Description

Adds some additional detail to the evaluate command when findings between New and Threshold assessments are different. Ideally this will give the person enough information to quickly know what changes changed the satisfaction of the findings.

Added a --summary flag to evaluate to print all observations.

Current output for changed assessments is in a table format for all observations so as to not duplicate single observations that fail on multiple controls.

Related Issue

Fixes #533

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@meganwolf0
Copy link
Collaborator Author

meganwolf0 commented Jul 17, 2024

Sample view for the user - shows both sets of results leading to failing eval + "summary" to get all observations. I think these can be rearranged slightly and can have more fidelity over the info returned to try and not overwhelm the user, but wanted to throw out for spears

Screenshot 2024-07-17 at 3 23 32 PM

@meganwolf0 meganwolf0 marked this pull request as ready for review July 18, 2024 20:07
@brandtkeller
Copy link
Member

The information at play here makes the workflow very observation-centric. I don't see anything wrong with that and this is a huge improvement for exposing information.

I would be curious if you have thoughts for the output and the Observation to Finding relationship?

IE if a control that was previously passing now has multiple failing observations - this results in multiple line-items in the output - because multiple observations are failing. Ultimately these multiple failing observations roll-up to a single failing finding (for which has an actual field for capturing state).

Example (ignore the new/threshold remarks columns):

dev@dev:~/work/lula$ ./bin/lula evaluate -f assessment-results.yaml

 NOTE  Saving log file to /tmp/lula-2024-07-18-23-11-19-4230766809.log
  ⠋  Evaluating Assessment Results 5a0a9538-e734-48a5-a327-02e6aa6891b0 against b57d4be5-75a7-4b3b-be66-e3cfcb1cac15 (0s)
 WARNING  Evaluation Failed against the following:                                                                                                                                                                                                                                                                                                                             

Control ID(s) | Observation                                                | Satisfied | Change                     | New Remarks                                                         | Threshold Remarks                                                                                                                                                                  
sc-23         | 67456ae8-4505-4c93-b341-d977d90cb125 - istio-health-check  | false     | SATISFIED TO NOT SATISFIED | istiohealth.deployment_message: All deployment conditions are true. | istiohealth.deployment_message: All deployment conditions are true.
              |                                                            |           |                            | istiohealth.hpa_message: HPA has sufficient replicas.               | istiohealth.hpa_message: HPA has sufficient replicas.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
sc-23         | ca49ac97-487a-446a-a0b7-92b20e2c83cb - enforce-mtls-strict | false     | SATISFIED TO NOT SATISFIED | validate.msg: All PeerAuthentications have mtls mode set to STRICT. | validate.msg: All PeerAuthentications have mtls mode set to STRICT.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

     ERROR:  failed to meet established threshold 

Current approach works well for the summary containing the state of observations and those mapped to many Findings (control-ids really) - but I wonder if we need to augment or add something in the future that is more Finding-centric?

@meganwolf0
Copy link
Collaborator Author

Current approach works well for the summary containing the state of observations and those mapped to many Findings (control-ids really) - but I wonder if we need to augment or add something in the future that is more Finding-centric?

Yeah I think this would be totally do-able. I do a kind of refactor of the paired results to get the observations by controls, so reporting out more info about the original object would probably be a simple thing to add. Probably would need to talk more about what would be best to try and tease out of that info for the user? This alternate table tried to just report each control along with the observations.. so you could kind of slice it that way too. I just didn't love that for the scenario where you had like a single observation that's attached to a bunch of different findings failing.

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

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

Quick cleanup items and I believe this is ready. One of the first instances of improving UX and feedback will probably be the main driver for updates outside of general assumptions.

meganwolf0 and others added 2 commits July 26, 2024 08:32
Co-authored-by: Brandt Keller <43887158+brandtkeller@users.noreply.github.com>
@brandtkeller brandtkeller merged commit 8a07833 into main Jul 26, 2024
4 checks passed
@brandtkeller brandtkeller deleted the 553-additional-information-on-evaluate branch July 26, 2024 15:51
This was referenced Jul 26, 2024
mjnagel referenced this pull request in defenseunicorns/uds-core Aug 2, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [defenseunicorns/lula](https://togithub.com/defenseunicorns/lula) |
patch | `v0.4.3` -> `v0.4.4` |

---

### Release Notes

<details>
<summary>defenseunicorns/lula (defenseunicorns/lula)</summary>

###
[`v0.4.4`](https://togithub.com/defenseunicorns/lula/releases/tag/v0.4.4)

[Compare
Source](https://togithub.com/defenseunicorns/lula/compare/v0.4.3...v0.4.4)

This release includes new output during `lula evaluate` through the use
of the `--summary` flag to better highlight areas of improved,
unchanged, or worse compliance-at-a-glance.

OSCAL writes for the current models supported are now written in a
deterministic format. This alleviates long-lived data from being
re-arranged, specifically when stored in version control. Better
highlighting the areas of change as you maintain your OSCAL.

As always - keeping our dependencies - project or pipeline - up to date
is a constant focus of of review.

##### Features

- **evaluate:** add observation summary
([#&#8203;540](https://togithub.com/defenseunicorns/lula/issues/540))
([8a07833](https://togithub.com/defenseunicorns/lula/commit/8a07833c5a563d8e857515a083137785cade5eb5))

##### Bug Fixes

- **oscal:** deterministic OSCAL model write
([#&#8203;553](https://togithub.com/defenseunicorns/lula/issues/553))
([5493df1](https://togithub.com/defenseunicorns/lula/commit/5493df122b803d11542f29cfe80dfa4d5aaa10a8))

##### Miscellaneous

- **deps:** update github/codeql-action action to v3.25.14
([#&#8203;557](https://togithub.com/defenseunicorns/lula/issues/557))
([5bfd94f](https://togithub.com/defenseunicorns/lula/commit/5bfd94febc467e5a455ed32d97ce2e82e20409c2))
- **deps:** update github/codeql-action action to v3.25.15
([#&#8203;564](https://togithub.com/defenseunicorns/lula/issues/564))
([60e128a](https://togithub.com/defenseunicorns/lula/commit/60e128a0a34ce8686c67e22ea2aebb61212b97fc))
- **deps:** update golang to version 1.22.5
([#&#8203;562](https://togithub.com/defenseunicorns/lula/issues/562))
([97ff760](https://togithub.com/defenseunicorns/lula/commit/97ff7602f30f0709bd2ca16b74e53008607c3a61))
- **deps:** update module github.com/open-policy-agent/opa to v0.67.0
([#&#8203;561](https://togithub.com/defenseunicorns/lula/issues/561))
([4378242](https://togithub.com/defenseunicorns/lula/commit/43782420b8b34362d03bcc965e00df2a850715c6))
- **docs:** fix simple demo command for evaluate file
([33fb97c](https://togithub.com/defenseunicorns/lula/commit/33fb97cccc9d4a589da65c03cc433b4f05c79d5d))
- **docs:** updated broken links
([#&#8203;554](https://togithub.com/defenseunicorns/lula/issues/554))
([8dd24b0](https://togithub.com/defenseunicorns/lula/commit/8dd24b083c86b12af8740fe788c4222f4c1c8718))
- **docs:** updated README for docs badge
([#&#8203;558](https://togithub.com/defenseunicorns/lula/issues/558))
([72fd3fc](https://togithub.com/defenseunicorns/lula/commit/72fd3fc8137477a4f10507481f8464eb5685b781))

#### What's Changed

- chore(docs): correcting cli command in simple demo by
[@&#8203;ogijaoh](https://togithub.com/ogijaoh) in
[https://github.com/defenseunicorns/lula/pull/549](https://togithub.com/defenseunicorns/lula/pull/549)
- docs: updated broken links by
[@&#8203;meganwolf0](https://togithub.com/meganwolf0) in
[https://github.com/defenseunicorns/lula/pull/554](https://togithub.com/defenseunicorns/lula/pull/554)
- docs: updated README by
[@&#8203;meganwolf0](https://togithub.com/meganwolf0) in
[https://github.com/defenseunicorns/lula/pull/558](https://togithub.com/defenseunicorns/lula/pull/558)
- chore(deps): update github/codeql-action action to v3.25.14 by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/defenseunicorns/lula/pull/557](https://togithub.com/defenseunicorns/lula/pull/557)
- chore(deps): update module github.com/open-policy-agent/opa to v0.67.0
by [@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/defenseunicorns/lula/pull/561](https://togithub.com/defenseunicorns/lula/pull/561)
- chore(deps): update golang to version 1.22.5 by
[@&#8203;brandtkeller](https://togithub.com/brandtkeller) in
[https://github.com/defenseunicorns/lula/pull/562](https://togithub.com/defenseunicorns/lula/pull/562)
- feat(evaluate): add observation details by
[@&#8203;meganwolf0](https://togithub.com/meganwolf0) in
[https://github.com/defenseunicorns/lula/pull/540](https://togithub.com/defenseunicorns/lula/pull/540)
- fix(oscal): deterministic OSCAL model write by
[@&#8203;brandtkeller](https://togithub.com/brandtkeller) in
[https://github.com/defenseunicorns/lula/pull/553](https://togithub.com/defenseunicorns/lula/pull/553)
- chore(deps): update github/codeql-action action to v3.25.15 by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/defenseunicorns/lula/pull/564](https://togithub.com/defenseunicorns/lula/pull/564)
- chore(main): release 0.4.4 by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[https://github.com/defenseunicorns/lula/pull/546](https://togithub.com/defenseunicorns/lula/pull/546)

#### New Contributors

- [@&#8203;ogijaoh](https://togithub.com/ogijaoh) made their first
contribution in
[https://github.com/defenseunicorns/lula/pull/549](https://togithub.com/defenseunicorns/lula/pull/549)

**Full Changelog**:
defenseunicorns/lula@v0.4.3...v0.4.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/defenseunicorns/uds-core).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
This was referenced Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Additional information on Evaluate
3 participants