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

fix(validate): allow for optionality among potential standards in a component definition #532

Merged
merged 23 commits into from
Aug 2, 2024

Conversation

brandtkeller
Copy link
Member

@brandtkeller brandtkeller commented Jul 11, 2024

Description

Component definitions are comprised of 1-> components of 1 -> N Control Implementations of 1 -> N Implemented Requirements

The intent of validate should support a default behavior of assessing all control implementations (containing common control-id's) and producing a result per targeted source of controls.

In doing so - Lula can then support validate/evaluate of specific control-implementations via a specified flag instead of needing to execute against non-applicable standards/controls. This will also be an expected behavior for generation of an assessment-plan where we will need to know which standard/framework to choose.

Core Functionality Updates

  • Introduces a framework and target Lula oscal prop
    • framework will be used in the component definition and allows for the combination of multiple standards/control-implementations into a single result
    • target is a generated prop that is used by Lula to conduct evaluate in a targeted manner vs the filtering and evaluation of all unique "targets"
  • Adds a -t / --target flag to validate and evaluate
  • The assessment-result is now generated from a slice of results.
  • Each unique control-implementation identified will create a unique result
    • When the framework prop is utilized an additional result will be created for the framework
    • This allows targeting a single standard/framework OR creating results for all standards/frameworks by default

Constraints

  • Proposed changes remove the connection between an implemented-requirement/control-implementation and it's subsequent component (which wasn't thoroughly leveraged before. I'd like to consider implications and determine if adding this back in the future is helpful.
  • This update is not entirely backwards compatible. Existing thresholds will need to be updated after determining what the new execution means for impact (if using multiple standards).

Local Testing

  • Unit tests and E2E tests were updated to fix removed/modified functions
  • see src/test/unit/common/oscal/valid-multi-component.yaml as a target for performing various permutations of validate/evaluate and noting the amount of results created

Release Note

TODO: insert release not here for how to adopt these changes in an existing validate/evaluate workflow with an existing threshold.

Related Issue

Closes #327
Closes #499
Closes #500

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

@brandtkeller brandtkeller changed the title 327 standards optionality fix(validate): allow for optionality among potential standards in a component definition Jul 30, 2024
@brandtkeller brandtkeller marked this pull request as ready for review July 31, 2024 02:37
@brandtkeller brandtkeller self-assigned this Jul 31, 2024
docs/ns/framework.md Outdated Show resolved Hide resolved
@meganwolf0
Copy link
Collaborator

A couple other questions/comments as I'm reviewing:

  • Looks like observations aren't all appended to related-observations when two different components are calling the same control (perhaps we need a test case for this scenario)
  • This one is more personal preference - but the "Findings" section in the validate output is taking up a lot of space in the terminal, especially just with line breaks, would it be possible to condense those to a single line -> Control ID: %s, Finding UUID: %s, Status: %s? Maybe there's a reason they should be that expansive though so happy to hear the counterargument there.
  • How does framework get added? It appears to be manual right now, is that something that ideally will happen via lula generate?
  • When I run lula validate against the valid-multi-component.yaml I get 4 results - I thought there'd be two for just each of the "frameworks" specified, but seems like the catalog source also gets added as a target... can you ELI5?

@CloudBeard
Copy link
Collaborator

I noticed the 4 findings but I think it's a good thing.

You'll get one per framework and one per catalog/source.

Im thinking if you have impact levels and you don't specify a target you'll get the FedRAMP result and the impact level result.

@meganwolf0
Copy link
Collaborator

meganwolf0 commented Aug 1, 2024

I noticed the 4 findings but I think it's a good thing.

You'll get one per framework and one per catalog/source.

Im thinking if you have impact levels and you don't specify a target you'll get the FedRAMP result and the impact level result.

Would there be a situation where source and framework would be different or like independent? They just seemed to kind of indicate the same thing but different values, basically. But there might be more nuance that the example doesn't really cover

Edit: Talked to Brandt, I get it :) disregard this one

@brandtkeller brandtkeller marked this pull request as draft August 1, 2024 20:25
@brandtkeller
Copy link
Member Author

A couple other questions/comments as I'm reviewing:

  • Looks like observations aren't all appended to related-observations when two different components are calling the same control (perhaps we need a test case for this scenario)

This is a real bug - should be fixed by checking for existing related observations instead of overwriting when generating findings.

  • This one is more personal preference - but the "Findings" section in the validate output is taking up a lot of space in the terminal, especially just with line breaks, would it be possible to condense those to a single line -> Control ID: %s, Finding UUID: %s, Status: %s? Maybe there's a reason they should be that expansive though so happy to hear the counterargument there.

Agreed - I re-used the table functionality from message and consolidated the output to Control ID / Status. Open to feedback on if we really need the finding UUID (which are really just unique control ID's to my knowledge)

  • How does framework get added? It appears to be manual right now, is that something that ideally will happen via lula generate?

Manually or as part of generate in another PR: #574

@brandtkeller brandtkeller marked this pull request as ready for review August 1, 2024 22:26
@brandtkeller brandtkeller merged commit ac0befb into main Aug 2, 2024
4 checks passed
@brandtkeller brandtkeller deleted the 327_standards_optionality branch August 2, 2024 20:11
This was referenced Aug 2, 2024
This was referenced Aug 9, 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
3 participants