docs: 0008 validation refactor adr

[meganwolf0]

Description

ADR for Validation library refactor

Sample branch and PR (for commentary on specific usage/methods): #882

Related Issue

Fixes #858

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

• Test, docs, adr added or updated as needed
• Schema Updates applied
• Contributor Guide Steps followed

[nacx]

This looks great. Thank you! :)
I've added just one comment about coupling the interfaces to an implementation of the ValdiationStore that may carry logic that may not be related to these new semantics.

[nacx]

How much of the ValidationStore is really related to the Requirements interface? (I'm guessing the implementations of the producer will probably end up calling the AddRequirement methods?). Looking at the current wip branch I can see that most of the logic in the store is not related to them.

WDYT then, about extracting an interface for the ValidationStore as well, exposing the methods that are relevant for new Validation interfaces? This way these new interfaces will not be coupled to existing logic in the store that is not really related to this validation semantics.

[meganwolf0]

Thanks for the suggestion! I've been chewing on this and trying to figure out the implications. I'm wondering if the intended effect would basically be like "you can put any validation in the validation store, it doesn't need to be a LulaValidation"? Looking at the usage of the ValidationStore right now, seems very specific to the fact that there are LulaValidations in there, so I'm just trying to figure out how that would be generalized... PS if you have direct changes/suggestions on how to do this feel free to comment them in the branch!

My thought process behind sticking requirements in the validation store was mostly that a context-less validation (i.e., one without the thing it's proving) feels like not actually a validation, so conceptually (to me anyway) it made sense to stick them together. The other thing I was trying to do was knit the producer (who creates the requirements) to the consumer (who reports out the results of the requirements) by creating that single interface (well not interface) of the ValidationStore, so they don't depend on each other directly.

Again, there is most likely a better design to doing all this, if you do want to make suggestions directly in the code, I think much better when seeing the actual thing in action 😁

[nacx]

Thanks! I have a couple follow-up questions, mostly driven because there are three interfaces here that are completely independent, although internally they seem to be tightly related, and that is a bit confusing to me :)

If the idea is that "you can put any validation in the validation store", why is the Requirement interface needed at all? I mean, if the Producer and Consumer don't use it, why does it even exist?
Following this line, would it make sense for the Populate method to receive as an argument, not the validation store, but some interface that "accepts requirements"?

[meganwolf0]

If the idea is that "you can put any validation in the validation store"

Sorry if there was some confusion about that - I meant to ask it as a question to you, if that's what you were intending with the request of an interface. I think that only Lula Validations should go in the validation store, and the Lula Validation engine executes them, which is why I was hesitant to make that an interface, there's just a lot of stuff that feel particular to those that would make an interface there challenging.

if the Producer and Consumer don't use it, why does it even exist

The producer and consumer are using the requirements - essentially the Requirement interface is populated by the Producer and used/reported on by the Consumer, and the reporting or evaluation of those requirements is a function of the results of the validations. So a couple instances I tried to draw out in the WIP branch are:

• The ComponentDefinition establishes the components of the system which perform the controls, e.g., AC-1, AC-2. The Requirement (so called ComponentDefinitionRequirement) is successful when each component that performs a control (because there can be several) are all linked to validations AND those validations are all passing. The ComponentDefinition producer populates the requirements from its spec, and the AssessmentResults consumer yields the results of the requirements in the format particular to its spec
• The Simple case is trying to illustrate a much more straightforward "requirement" where essentially the requirement is a simple pass/fail if all corresponding validations pass or fail.

After looking a bit more though, I do think possibly sticking requirements in the validation store is just a little lazy on my part, and perhaps breaking them into like a RequirementsStore to have separate methods would make a bit more sense.

I'm going to try and make a couple changes, and include the comment below about EvaluateResults, in the WIP branch and will make the according changes here 😄 thank you for the feedback so far, much appreciated!

[soltysh]

One nit, otherwise lgtm :)

[nacx]

Thanks for the clarifications!

I better understand the rationale now, and I like the semantics these three interfaces bring. Nice! 😄
I just have some small follow-up questions about the relationship between these interfaces.

[nacx]

Thanks! I have a couple follow-up questions, mostly driven because there are three interfaces here that are completely independent, although internally they seem to be tightly related, and that is a bit confusing to me :)

If the idea is that "you can put any validation in the validation store", why is the Requirement interface needed at all? I mean, if the Producer and Consumer don't use it, why does it even exist?
Following this line, would it make sense for the Populate method to receive as an argument, not the validation store, but some interface that "accepts requirements"?

[nacx]

Would it make sense for this method to return the results as well? For example it could return each Requirement ID with the evaluation result. Otherwise getting the details of the actual results is hidden in internal state and hard to consume. WDYT?

[meganwolf0]

@nacx - I made a few updates to the wip branch, and this doc:

• Added a RequirementStore to hold the requirements and redefined the consumer interface to use this to provide results. I'm not sure if this addresses any gaps you might have seen, but I think this might be cleaner, and also has methods to extract all the Requirements (which reference the validations), so I think all the data should be available from that structure? I.e., the point above about data being hidden I don't think is an issue (please lmk if I missed or misunderstood that point)
• Since I'm coming into this within the frame of mind of the Lula CLI and I think you are considering this more from a controller standpoint, I added a quick sample controller consumer to try and feel out how it might interface with the methods provided (here) - this is absolutely not my wheelhouse so if this doesn't make sense and there's still methods/data that are missing I'd like to better understand!

[nacx]

I'm coming into this within the frame of mind of the Lula CLI

The proposed interfaces look good and help better conceptualize the whole thing outside the context of a CLI.

There is another point, probably more internal, that is one of the main challenges when trying to use Lula outside of the context of the CLI: the message package. Right now the codebase is coupled to the fact that the message package is meant for a CLI, and there are places like this one deep in the validation framework that log feedback directly assuming a CLI.

Refactoring the message package into a Logging interface (whatever) that could be injected would go a long way, allowing consumers to instantiate how they want the output to be produced, and have an implementation that can produce all the fancy table outputs, colors, etc, and another that is just normal logging as expected by a library (but methods such as Table, Title should be probably moved completely to the cmd or a cli package and not used by the core primitives (validators, etc) of the project.