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

Upgrade GitHub CI/CD Workflows to Current Stable Version #116

Closed
6 tasks done
aj-stein-nist opened this issue Jul 12, 2022 · 15 comments · Fixed by #144, #156, #204 or usnistgov/OSCAL#1947
Closed
6 tasks done

Upgrade GitHub CI/CD Workflows to Current Stable Version #116

aj-stein-nist opened this issue Jul 12, 2022 · 15 comments · Fixed by #144, #156, #204 or usnistgov/OSCAL#1947
Assignees
Labels
enhancement The issue adds a new feature, capability, or artifact to the repository. Scope: CI/CD A task issue to modify the repo's continuous integration and continuous deployment capability. User Story The issue is a user story for a development task.

Comments

@aj-stein-nist
Copy link
Contributor

aj-stein-nist commented Jul 12, 2022

User Story:

As an OSCAL tool developer, in order to make sure CI/CD is properly developed and integrated in relevant OSCAL projects, I want the current version of GitHub Actions CI/CD workflows updated to current stable.

Goals:

The NIST OSCAL Team will complete cleaning, deduplification, and misc improvements to the GitHub Actions in usnistgov/OSCAL for improved maintenance, stability, and resiliency.

Dependencies:

Acceptance Criteria

  • All readme documentation affected by the changes in this issue have been updated.
  • A Pull Request (PR) is submitted that fully addresses the goals of this User Story. This issue is referenced in the PR.
  • The CI-CD build process runs without any reported errors on the PR. This can be confirmed by reviewing that all checks have passed in the PR.
@aj-stein-nist aj-stein-nist added enhancement The issue adds a new feature, capability, or artifact to the repository. User Story The issue is a user story for a development task. labels Jul 12, 2022
@aj-stein-nist aj-stein-nist added the Scope: CI/CD A task issue to modify the repo's continuous integration and continuous deployment capability. label Jul 12, 2022
@aj-stein-nist
Copy link
Contributor Author

Earlier in the week I spent I looked deeply into this and noticed that very little of the updated between the main OSCAL CI/CD and oscal-content because the different outcomes desired from the repository (oscal-content only converts content). If anything, this means maybe only the steps for setting up Java and Saxon and/or Calabash matter here. That means at this point simple copy-paste.

I did some experiment with cleanup of this and had a think, or quick diversionary spike to think of how we can finish the modularization here better than copy-pasting, and that would mean beginning the move to reusable workflows. The alternative, composite actions, are not able to access tokens, so flat out could not commit back content (this is a very important stumbling block and the final step in pipeline processing for this repo). So here are the spike experiments.

https://github.com/aj-stein-nist/test-gha-reusable-workflows/tree/main/.github/workflows

I am worried because of token management from forks and other things from testing this likely will not work particularly well is token management around this committing documents issue. Tokens are determined in the checkout step, in the default action provided by the GHA team. I am struggling to get around actions/checkout#298 which means this spike might be wrapping up with a "meh, this is not going to work either" result.

@aj-stein-nist
Copy link
Contributor Author

Ok, #1470 created to tie off the spike work and return back to the AC here of just upgrading things to a stable version. New PR incoming.

@aj-stein-nist
Copy link
Contributor Author

@david-waltermire-nist I will also wrap this up for current sprint post-haste. We had discussed in last weekly status meeting that I fix this this up and defer other work, I started working in it and will finish soon, but I did forget to add it to the current sprint. Apologies! Will ping you again when it is ready for review.

@david-waltermire
Copy link
Contributor

@aj-stein-nist I want to keep this open until PR 157 is resolved.

@aj-stein-nist
Copy link
Contributor Author

Moving to Sprint 61.

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Jan 31, 2023

Need to move to sprint 63. Much headway was made but more needed to wrap this up with testing metaschema submodule of OSCAL and OSCAL submodule of oscal-content.

@aj-stein-nist
Copy link
Contributor Author

Testing this again in #183. I accidentally closed an older branch and messed it up.

@aj-stein-nist
Copy link
Contributor Author

Still working through usnistgov/metaschema#235 which is breaking builds. I am primarily focused on that for now to further debug, fix, and unblock this work. More to follow next week.

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Mar 2, 2023

I had not yet updated this before the conclusion of Sprint 63. I needed to move this issue to Sprint 64.

I was able to completely validate test #183 to get end to end testing of touched-up changes needed in develop with #235. This particular issue is blocked until we can merge #235 changes (updated unit tests pending) and usnistgov/OSCAL#1639 changes when they are cherry-picked and re-adapted. The latter needs to be done after testing the commit because that PR, per community developer galtm, is connected to feature work that is not specific to the bug fix.

I expect to work towards resolution on both of these and unblock the latter before the end of Sprint 64.

aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
Per PR review with Nikita, cite documentation for `make` and `Makefile`
on how `$(@d)` is an alias to show to the the target wildcard file
without the filename, useful for creating nested directories of content
before copying.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
After debugging with @nikitawootten-nist, we determined during pairing
that the SECONDEXPANSION calls, that are necessary to avoid some more
hacky kludges with macros and functions. As it stands, when running with
concurrent jobs (`-jN` with N>=2) it fails. We determined the cause of
the failures for `convert-min-json-content` and `convert-yaml-content`
targets are because those target prerequisites are flawed.

This change will be greedy and presume all respective XML files for JSON
targets and all JSON files for YAML targets. This will limit the upper
bound of concurrent jobs for these targets, but makes it correct and
safe.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
There are a number of changes we need to make to the structure and the
organization of example documents in the repository to improve the
maintainability and stability of building examples.

1. We need to remove now invalid docs, e.g. usnistgov#208.
1. Remove JSON examples in src and convert them to XML first.
1. Remove old draft and FedRAMP directories, warning has been up for
long enough.
1. Remove old notes in src/examples/ssp/xml directory that are not
examples.
1. Relocate oscal submodule to build/oscal to match style of other
repositories in the project.
1. Update for OSCAL v1.1.1 release of models.
1. Align SP 800-53 Rev 5 catalog version with guidance in ADR in
usnistgov/OSCAL#1947, make version correctly 5.1.2.
1. Update last-modified, published, and version metadata as applicable.
1. Add jq and yq dependency install.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
This makes it more clear what is going on as it is like default.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
During testing, it became clear this URI scrubbing for the source-profile
prop in the resuling catalog is useful, but it scrubs the full path,
including the filename, not just the path itself. This behavior seems to
be divergent from previous behavior from the transforms, obsolete shell
scripting, or the combination of both. Local testing by changing calling
paths and other environmental changes did not seem to alter the behavior.

For now, I will leave the full path and during code review we can discuss
the merits of prioritizing the work in the backlog, using a workaround
in the Makefile and/or support shell scripting temporarily, or both
until the issue is resolved.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
As picked up in review with Nikita, we are converting the source XML
files to JSON and YAML, but not all. We need to adjust the make vars
that collect the source files for conversion to minified JSON to Include
the resolved catalogs from profiles from the previous step. Reviewing
the auto-committed test content confirm these files were being missed in
conversion to JSON and YAML and we obviously need those.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
Per discussion with Nikita during PR review and pairing, we should try
to adjust the jq and yq dep install and management approach to be a
little more ergonomic. As for xmllint, it was requested we align that
to work like the dep mgmt in the usnistgov/OSCAL repo, since that is a
compiled dep that is not offered statically and is best handled as a
concern by the developer in their operating system, or by a dedicated
install step in GHA workflows.

Additionally, for these reasons, the clean wrapper target will not
include deleting it as a default, but can be added later by a developer
by explicitly requesting it.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
Per PR review and consultation from Nikita, add in additional wrapper
targets for artifacts and checks for better organization and to make a
cleaner hierarchy for the make all target.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
As discussed in the branch under review in usnistgov#204, we now have absolute
paths in the resolved catalogs based on the full paths to the profile,
which is less than idea for published catalogs.

Comment with explanation:
usnistgov#204 (comment)

Example:
https://github.com/aj-stein-nist/oscal-content/blob/dc5500ed9371b485fc21cb095d2fb9db6e2a1fd3/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_LOW-baseline-resolved-profile_catalog.xml#L11

To work around this for the time being, we are using sed in the Makefile
to clean up once profile resolution is done and before the XML source is
converted to the target JSON and YAML.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
Per PR review with Nikita, cite documentation for `make` and `Makefile`
on how `$(@d)` is an alias to show to the the target wildcard file
without the filename, useful for creating nested directories of content
before copying.
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Oct 27, 2023
After debugging with @nikitawootten-nist, we determined during pairing
that the SECONDEXPANSION calls, that are necessary to avoid some more
hacky kludges with macros and functions. As it stands, when running with
concurrent jobs (`-jN` with N>=2) it fails. We determined the cause of
the failures for `convert-min-json-content` and `convert-yaml-content`
targets are because those target prerequisites are flawed.

This change will be greedy and presume all respective XML files for JSON
targets and all JSON files for YAML targets. This will limit the upper
bound of concurrent jobs for these targets, but makes it correct and
safe.
aj-stein-nist added a commit to usnistgov/OSCAL that referenced this issue Nov 9, 2023
aj-stein-nist added a commit to usnistgov/OSCAL that referenced this issue Nov 9, 2023
Compton-US pushed a commit to usnistgov/OSCAL that referenced this issue Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment