feat: initial lula report #599

CloudBeard · 2024-08-14T14:30:24Z

Description

Adds report functionality starting with component definition.

Will report how many controls are mapped per source and per framework for each component. There is the possibility that a component-definition file has multiple sources and multiple frameworks. The current report will list how many controls are in each and will give a % mapped based on the source provided for each.

Running lula generate component --catalog-source https://raw.githubusercontent.com/GSA/fedramp-automation/master/dist/content/rev5/baselines/json/FedRAMP_rev5_MODERATE-baseline-resolved-profile_catalog.json --component 'Keycloak' --requirements ac-12,ac-2,ac-2.1,ac-2.3,ac-2.4,ac-2.5,ac-2.7,ac-3,ac-5,ac-6,ac-6.1,ac-6.2,ac-6.5,ac-6.9,ac-6.10,ac-7,ac-8,au-2,au-3,au-3.1,au-8,au-9.4,au-12,cm-5.1,cm-5.5,ia-2,ia-4.4,ia-5.1,ia-5.2,ia-6,ia-8.1,ia-11,sc-13 --remarks assessment-objective --framework il4

Will show that ac-2 is fixed but also show that other controls aren't broken as a side effect of the fix.

Related Issue

Relates to #223 and #247

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

…with

brandtkeller · 2024-08-14T15:43:33Z

Quick feedback loop - I can trace the code but I would always recommend starting with docs on a new feature.

Capture the purpose - let people challenge that intent.

I don't believe this needs an ADR - so docs would be sufficient.

At the same time I think we should keep challenging if we know enough to pursue it further. Does this provide enough value to be of use or does some infrastructure require more maturation first. Transparently I do not know if we have enough context to make the result meaningful.

CloudBeard · 2024-08-20T03:17:30Z

Still need to add tests but want to figure out where/what packages to create to tidy up the main report code.

brandtkeller

Trending in the right direction now that we're getting to core of what we need (or can provide currently).

I think we're still missing a bit of big picture vision -> Which I/the team can help define.

See my comment about the context that drives the reporting (as we see it today). Ideally we as a team need to provide a clearer deliverable on our issues (WIP) - but I appreciate you pursuing this nonetheless.

My current take:
Reporting should be a function of collecting available context and reporting accordingly.

First iteration starts with component-definition and reports controls available.
With the presence of a catalog/profile -> we can overlay that control information to calculate percentages.
with the presence of assessment-results we can check satisfied/not-satisfied

Given that - we can start migrating functionality to the oscal model libraries for each model as applicable to get the context we need to return here for collective reporting.

My opinion is that reporting is not singly-model focused and rather total context driven. I may be very wrong - let's chat as a team if anyone feels otherwise.

brandtkeller · 2024-08-28T23:16:29Z

src/cmd/report/report.go

+	Short:   "Build a compliance report",
+	Example: reportHelp,
+	Run: func(_ *cobra.Command, args []string) {
+		if opts.InputFile == "" {


If we mark this flag as required then we can remove this check

Swapped that to required removed the check.

src/cmd/report/report.go

brandtkeller · 2024-08-28T23:23:55Z

src/cmd/report/report.go

+		spinner.Success()
+
+		spinner = message.NewProgressSpinner("Determining OSCAL model type")
+		modelType, err := determineOSCALModel(oscalModels)


This may be a good spot to establish a separation of duties -> We've performed the essential operation of retrieving/parsing the data -> now let's create a function that handles the next steps.

I ended up splitting that up twice, the first time just to separate duties but ended up adding 2 new functions. 1 that is public that will run the actual logic and the second that is a handler for model handlers.

Ran into an issue testing so I came back to add the function that runs the logic.

src/cmd/report/report.go

brandtkeller

No issues with the current direction.

I would remove the current documentation and run a lula internal gen-cli-docs in order to generate the new CLI docs for this command. Iterate as necessary to try and capture all context in the CLI command code as possible. If you need additional docs place them outside the docs/cli-commands directory.

Otherwise this gets us moving in the right direction - as I mentioned previously, I expect future iterations to be less of a model-by-model reporting structure and rather wholistic reporting of all models - but that is out of scope currently.

brandtkeller · 2024-09-19T22:43:18Z

src/cmd/report/report.go

+	spinner.Success()
+
+	spinner = message.NewProgressSpinner("Determining OSCAL model type")
+	modelType, err := determineOSCALModel(oscalModels)


The oscal pkg under src/pkg/common/oscal contains a GetOscalModel() that returns the same values.

I would consider removing this -> removing the required parameter from the handleOSCALModel() function and call it directly inside handleOSCALModel().

Unless you have some unique requirement.

I ended up using a few more functions from common where I found redundancy.

brandtkeller · 2024-09-19T22:45:15Z

src/test/e2e/report_test.go

+    "github.com/defenseunicorns/lula/src/cmd/report"
+)
+
+func TestReportCommand(t *testing.T) {


Any reason these cannot serve as more of a unit test to be coupled with the report package.

I'd like to start iterating E2E tests to be more CLI-driven context.

Swapped those to unit and I think created a more CLI approach.

brandtkeller · 2024-09-19T22:50:02Z

src/test/e2e/report_test.go

+			expectErr:  false,
+		},
+        {
+            name:       "Catalog OSCAL Model",


in addition to the conversion of this test to more of a unit test -> it may help reduce testing file requirements. Possible use of data structures and objects if possible (look at your function inputs/outputs and consider the requirements).

Created a mock component definition data structure to have some data for testing.

brandtkeller · 2024-09-19T22:53:47Z

src/cmd/report/report.go

+}
+
+// Handler for Component Definition OSCAL files to create the report
+func handleComponentDefinition(componentDefinition *oscalTypes_1_1_2.ComponentDefinition, format string) error {


Should we consider composing comp-defs here prior to processing?

Good call! added the compose pieces to the beginning there.

brandtkeller · 2024-09-30T21:27:55Z

src/cmd/report/report.go

+	Use:     "report",
+	Hidden:  false,
+	Aliases: []string{"r"},
+	Short:   "Build a compliance report",


Let's challenge this a bit more -> what function does or can this command serve?

Report on the current state of compliance
or....

Maybe not build a compliance report as in a pdf but I can see it pulling various statics and data out of the OSCAL to be easier to read and understand. I am picturing a thousands of lines SSP and I want to know what controls am I missing, what controls do I have, do I have multiple frameworks and can I see that per framework. For POAMs Im thinking how do I see my critical or highs specifically, how do I see time left on moderates to patch.

For components I would like to have a quick count of controls per framework, how many have validations, how many controls are in this component-definition that aren't in a profile/catalog.

I can also see parts of it being reused similar to lula evaluate -f assessment-result.yaml --sumnmary doing the summary part.

Maybe the part that gets the data for UDS Runtime to present too. Thought there is it would be similar questions/data points around the OSCAL to show.

brandtkeller · 2024-09-30T21:32:49Z

src/cmd/report/report.go

+lula report -f oscal-component-definition.yaml --file-format yaml
+`
+
+var reportCmd = &cobra.Command{


I know this has been up for some time - Can we align the creation of this command to template such that the same cobra patterns are followed?

Believe ive restructured this

src/cmd/report/report.go

brandtkeller · 2024-09-30T21:42:42Z

src/test/e2e/report_test.go

+)
+
+// TestLulaReportValidComponent checks that the 'lula report' command works with a valid component definition with multiple components and framework prop.
+func TestLulaReportValidComponent(t *testing.T) {


Heading in the right direction. We have some examples and test utilities to pull from. Would like to standardize on that and @meganwolf0 may have some insights for what we need for structure.

Looks like this might be in reference to doing some golden file checks, I think the PR for compose should provide some reusable functions for checking the data from report, and the tools_compose_test.go and tools_template_test.go might be decent-ish places to start from

(I think there was also a discussion we were going to have as a team about e2e testing practices/structure so some of that could change)

I believe ive followed the same structure

meganwolf0 · 2024-10-02T13:44:47Z

src/cmd/report/report.go

+	return strings.HasPrefix(str, "http://") || strings.HasPrefix(str, "https://")
+}
+
+func PrintReport(data ReportData, format string) error {


Haven't picked through this, so this function might not be relevant, but the Table under messages might save some of this functionality

I added that table functionality, I like the way that looks.

…nitial-lula-report

meganwolf0 · 2024-10-11T13:09:27Z

Just tested locally - is this il4 supposed to be down in the framework table?

CloudBeard · 2024-10-11T16:19:40Z

Just tested locally - is this il4 supposed to be down in the framework table?

Yea that would make sense. Ill fix that real quick-ish

brandtkeller · 2024-10-11T17:56:22Z

src/internal/reporting/helpers.go

+	return err == nil
+}
+
+func fetchOrReadFile(source string) ([]byte, error) {


network.Fetch() will return file bytes for local files on the filesystem or remote files on network. I don't believe this functionality is needed unless you have an edge case you're solving for.

brandtkeller · 2024-10-11T17:57:35Z

src/internal/reporting/helpers.go

+)
+
+// Helper to determine if the controlMap source is a URL
+func isURL(str string) bool {


Given the comment below I imagine this will be removed - but I do think this has some gaps worth addressing if it were to be kept.

IE the return statement does not indicate a value that aligns to the function intent.

CloudBeard added 4 commits August 5, 2024 17:57

initial command

95568b1

initial command

3f7bb0c

initial report - still wip

d1543fb

still wip, need to pull controls from catalog and profile to do math …

0ea6bde

…with

CloudBeard changed the title ~~Initial lula report~~ feat: Initial lula report Aug 14, 2024

added report doc

8027410

CloudBeard changed the title ~~feat: Initial lula report~~ feat: initial lula report Aug 20, 2024

brandtkeller requested changes Aug 28, 2024

View reviewed changes

brandtkeller mentioned this pull request Sep 4, 2024

feat: initial poc for report #372

Closed

4 tasks

brandtkeller added this to the OSCAL Reporting data milestone Sep 4, 2024

brandtkeller linked an issue Sep 4, 2024 that may be closed by this pull request

Reporting of number of controls met by component definitions #223

Open

6 tasks

CloudBeard and others added 2 commits September 18, 2024 21:49

refactored to test main function, added test and test data

b10e5ff

Merge branch 'main' into initial-lula-report

bba7581

CloudBeard marked this pull request as ready for review September 18, 2024 22:02

CloudBeard requested review from mike-winberry and meganwolf0 as code owners September 18, 2024 22:02

CloudBeard requested a review from brandtkeller September 18, 2024 22:02

brandtkeller requested changes Sep 19, 2024

View reviewed changes

CloudBeard marked this pull request as draft September 20, 2024 14:19

CloudBeard and others added 9 commits September 23, 2024 21:52

Merge branch 'main' into initial-lula-report

e793cd1

update tests

3227f7b

re run doc generation

6296154

fix e2e test

d707143

removed failed case, its covered in unit test for the function itself

9371ff8

add compose to handleComponentDefinittion

ee25f9f

update e2e test file to contain validations

9d58163

Merge branch 'main' into initial-lula-report

ddd7fa8

extra space?

60827db

chore: empty commit to re-run CI

b4430e5

CloudBeard requested a review from brandtkeller September 30, 2024 20:08

CloudBeard marked this pull request as ready for review September 30, 2024 20:08

CloudBeard requested a review from a team as a code owner September 30, 2024 20:08

brandtkeller requested changes Sep 30, 2024

View reviewed changes

CloudBeard marked this pull request as draft October 1, 2024 15:08

update report structure

1862398

meganwolf0 reviewed Oct 2, 2024

View reviewed changes

CloudBeard and others added 6 commits October 3, 2024 16:04

update go fmt

07e630a

Merge branch 'main' of https://github.com/defenseunicorns/lula into i…

79a034b

…nitial-lula-report

still wip need to fix to work with new compose

09592ac

updated compose calls

c363e45

update to table function and I think fixed e2e tests to match

df8a3cd

Merge branch 'main' into initial-lula-report

baf8f91

CloudBeard marked this pull request as ready for review October 10, 2024 16:24

CloudBeard requested a review from brandtkeller October 10, 2024 18:20

Merge branch 'main' into initial-lula-report

5698e54

brandtkeller requested changes Oct 11, 2024

View reviewed changes

CloudBeard marked this pull request as draft October 11, 2024 20:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: initial lula report #599

feat: initial lula report #599

CloudBeard commented Aug 14, 2024 •

edited

Loading

brandtkeller commented Aug 14, 2024

CloudBeard commented Aug 20, 2024

brandtkeller left a comment

brandtkeller Aug 28, 2024

CloudBeard Sep 18, 2024

brandtkeller Aug 28, 2024

CloudBeard Sep 18, 2024

brandtkeller left a comment

brandtkeller Sep 19, 2024

CloudBeard Sep 30, 2024

brandtkeller Sep 19, 2024

CloudBeard Sep 30, 2024

brandtkeller Sep 19, 2024

CloudBeard Sep 30, 2024

brandtkeller Sep 19, 2024

CloudBeard Sep 30, 2024

brandtkeller Sep 30, 2024

CloudBeard Oct 1, 2024

brandtkeller Sep 30, 2024

CloudBeard Oct 3, 2024

brandtkeller Sep 30, 2024

meganwolf0 Oct 2, 2024

meganwolf0 Oct 2, 2024

CloudBeard Oct 10, 2024

meganwolf0 Oct 2, 2024

CloudBeard Oct 10, 2024

meganwolf0 commented Oct 11, 2024

CloudBeard commented Oct 11, 2024

brandtkeller Oct 11, 2024

brandtkeller Oct 11, 2024

feat: initial lula report #599

Are you sure you want to change the base?

feat: initial lula report #599

Conversation

CloudBeard commented Aug 14, 2024 • edited Loading

Description

Related Issue

Type of change

Checklist before merging

brandtkeller commented Aug 14, 2024

CloudBeard commented Aug 20, 2024

brandtkeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandtkeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meganwolf0 commented Oct 11, 2024

CloudBeard commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CloudBeard commented Aug 14, 2024 •

edited

Loading