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(cmd): #281 Create upgrade command implementing go-oscal revise. #295

Conversation

mike-winberry
Copy link
Collaborator

@mike-winberry mike-winberry commented Mar 6, 2024

  • feat: lula tools upgrade command
    • version defaults to latest supported by go-oscal
    • output file defaults to in place upgrade of input file
    • takes optional validation result file path to output a result
  • feat: uprade command will warn if not upgrading to latest version.
  • feat: lint command will warn if not the latest version.
  • feat: lint command now has optional -r --result-file param that will output the ValidationResult if defined.
  • chore: update lula tools lint with changes to ValidationCommand
  • chore: upgrade go-oscal to v0.2.5
  • docs: add disclaimer for visual changes to yaml while maintaining object equality when using the upgrade tool.
  • fix: scenarios/pod-label/component-definition.yaml to pass lula lint.

…utput to file. Wait on next go-oscal release for utilities needed to handle more succinctly
@mike-winberry mike-winberry linked an issue Mar 6, 2024 that may be closed by this pull request
@mike-winberry mike-winberry changed the title feat(cmd): Create upgrade command implementing go-oscal revise. feat(cmd): #281 Create upgrade command implementing go-oscal revise. Mar 6, 2024
Copy link

netlify bot commented Mar 6, 2024

Deploy Preview for lula-docs canceled.

Name Link
🔨 Latest commit da008b1
🔍 Latest deploy log https://app.netlify.com/sites/lula-docs/deploys/65f0bacce14c1d0008a23835

@mike-winberry mike-winberry marked this pull request as ready for review March 7, 2024 01:37
@meganwolf0
Copy link
Collaborator

So I tried this out on the tempo validation I've been working and it had the mildly annoying result of moving all the fields around and smashing down my formatted description field with all the payload data down into a single line... Wondering if that could be pretty printed out and ideally the backmatter could still be at end of the oscal file? I'm not entirely sure if the intended use of this is to create a file a user would continue to work with, so also maybe some documentation to go along with the intent/use case specified?

@brandtkeller
Copy link
Member

So I tried this out on the tempo validation I've been working and it had the mildly annoying result of moving all the fields around and smashing down my formatted description field with all the payload data down into a single line... Wondering if that could be pretty printed out and ideally the backmatter could still be at end of the oscal file? I'm not entirely sure if the intended use of this is to create a file a user would continue to work with, so also maybe some documentation to go along with the intent/use case specified?

hhmm... yeah this makes a lot of sense that it would do this given the underlying sting datatype and how we write validations. Unfortunate side-effect - if there is anything we can do to retain that structure then great - otherwise it certainly adds more evidence to needing a system that is more compatible.

@mike-winberry
Copy link
Collaborator Author

mike-winberry commented Mar 7, 2024

So I tried this out on the tempo validation I've been working and it had the mildly annoying result of moving all the fields around and smashing down my formatted description field with all the payload data down into a single line... Wondering if that could be pretty printed out and ideally the backmatter could still be at end of the oscal file? I'm not entirely sure if the intended use of this is to create a file a user would continue to work with, so also maybe some documentation to go along with the intent/use case specified?

I will create a story to look into it and see what I can do today. My understanding right now as to why, is that the golang map is unordered, and since json and yaml are considered to also be unordered data, the marshalling and unmarshaling process doesn't guarantee order. I have some ideas on how to deal with that but will have to play a bit in go-oscal today and see what works out.

I think another thing that could be done is looking at yaml formatters and run one before output. (for the string issue).

@mike-winberry mike-winberry marked this pull request as draft March 9, 2024 16:32
@mike-winberry mike-winberry marked this pull request as ready for review March 9, 2024 16:32
src/test/e2e/pod_validation_test.go Outdated Show resolved Hide resolved
mike-winberry and others added 3 commits March 12, 2024 08:49
… -r ResultFile optional param for the output of the ValidationResult file. Update pod_validation_test to upgrade the component-definition to a temp file and then use that temp file to run the assess tests, updated the associated oscal-component.yaml to pass linting.
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.

Functionality is working as I would expect. Appreciate updates / modifications and testing in a manner that won't modify existing artifacts.

@mike-winberry
Copy link
Collaborator Author

mike-winberry commented Mar 12, 2024

Also I found the issue that @meganwolf0 was encountering. It has to do with trailing whitespace. It is a current issue with the go yaml implementation go-yaml/yaml#880. But if all trailing spaces are removed it no longer serializes the block/folded strings into double quotes. And is something that could be brought into go-oscal where we trim all empty trailing spaces from lines. So it is a yaml formatting problem. I think that with the right linter it would be caught but an empty space after something like Group: instead of Group: breaks the scalar and tells the parser to wrap in double quotes.

@brandtkeller
Copy link
Member

@mike-winberry I need to think more about edge cases but otherwise that sounds like a great issue for go-OSCAL

@mike-winberry
Copy link
Collaborator Author

@mike-winberry I need to think more about edge cases but otherwise that sounds like a great issue for go-OSCAL

defenseunicorns/go-oscal#167

Have this open right now. With the resulting after removing whitespace.

@brandtkeller brandtkeller merged commit 9b25856 into main Mar 13, 2024
7 checks passed
@brandtkeller brandtkeller deleted the 281-implement-go-oscal-revision-command-into-a-tools-upgrade-command branch March 13, 2024 03:59
This was referenced Jun 29, 2024
This was referenced Jul 12, 2024
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
None yet
Development

Successfully merging this pull request may close these issues.

Implement go-oscal revision command into a tools upgrade command
3 participants