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.

Sign up for GitHub

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

Jump to bottom

Fix #412, git error in CodeQL Analyze Action #413

Merged
merged 2 commits into from
Feb 3, 2022
Merged

Fix #412, git error in CodeQL Analyze Action #413

merged 2 commits into from
Feb 3, 2022

Conversation

astrogeco
Copy link
Contributor

@astrogeco astrogeco commented Jan 21, 2022 •
edited
Loading

Checklist (Please check before submitting)

Describe the contribution

Fix #412

  • Adds new component-path input parameter
  • Use git clone instead of checkout@v2 for the cFS-Bundle
  • Use symlink to map calling repo workspace to expected cFS Bundle
    directory location
  • Repurpose tests input to be a boolean tied to "ENABLE_UNIT_TESTS" flag
  • Enable "code snippets" option to CodeQL Analyze action
  • Archives sarif files from analysis output
  • Removes code duplication by using a matrix build for security and coding standard analyses
  • Alphabetizes workflow inputs and order based on "required" flag

Testing performed
Called codeql analysis action in bundle repo
https://github.com/astrogeco/cFS/actions/runs/1791252476

Called fork implementation from cFE repo. See actions run in nasa/cFE#2035,
https://github.com/astrogeco/cFE/runs/5056853287?check_suite_focus=true

CodeQL-cFEFix-2035

Expected behavior changes

  • CodeQL Action executes successfully.
  • Sarif files are stored as a run artifact
  • Unit Test code is scanned

CodeQL-cFEFix-2035-UTAlerts

System(s) tested on

  • Github actions, Ubuntu 18.04

Additional context

Will break current CodeQL implementation in cFS components.

Contributor Info - All information REQUIRED for consideration of pull request
Gerardo E. Cruz-Ortiz, NASA

This was referenced Jan 21, 2022
Closed
Merged
@astrogeco astrogeco added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jan 21, 2022
@ArielSAdamsNASA
Copy link
Contributor

Since component-path is required, does this need to be added in codeql-build-reuse.yml?

astrogeco added a commit to astrogeco/cFE that referenced this pull request Jan 24, 2022
@astrogeco
Fix nasa#2034, Fix git error in CodeQL workflow
aa984b8 
Adds new parameters to match updated cFS-CodeQL workflow interface
introduced in nasa/cFS#413
astrogeco
astrogeco commented Jan 24, 2022
View reviewed changes
.github/workflows/codeql-build.yml Outdated

- name: Run tests
run: ${{ inputs.tests }}
# - name: Run tests
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArielSAdamsNASA, what is the use case for including tests in the CodeQL workflow?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That can be removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't run the unit tests here, where do they get run?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I just went through a significant effort to fix a large number of uninitialized variables being used in app unit tests, something that I'd expect analysis tools to catch early and avoid. I'm in favor of doing analysis on the unit tests, since the sort of warnings/errors that show up really should be addressed (earlier in the unit test development phase the better).

Copy link
Contributor Author

@astrogeco astrogeco Jan 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as long as we build the tests, the analysis tools should catch problems with them. My question was whether it makes sense to run them here or in a different workflow like https://github.com/nasa/cFS/blob/main/.github/workflows/build-cfs.yml

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy and concur! As long as they still get analyzed, and the tests get run somewhere in CI I'm happy!

@astrogeco astrogeco force-pushed the fix-codeql-workflow branch 7 times, most recently from 7839272 to 9078af5 Compare January 26, 2022 23:24
astrogeco added a commit to astrogeco/cFE that referenced this pull request Jan 26, 2022
@astrogeco
Fix nasa#2034, Fix git error in CodeQL workflow
ca1a43e 
Adds new parameters to match updated cFS-CodeQL workflow interface
introduced in nasa/cFS#413
@astrogeco astrogeco force-pushed the fix-codeql-workflow branch 4 times, most recently from 1f2cf94 to f281f71 Compare January 26, 2022 23:46
astrogeco added a commit to astrogeco/cFE that referenced this pull request Jan 26, 2022
@astrogeco
Fix nasa#2034, Fix git error in CodeQL workflow
43de2bf 
- Add new parameters to match updated cFS-CodeQL workflow interface
introduced in nasa/cFS#413
- Add file-exclusion checks to Action trigger so workflow doesn't run if
only changes in commit or pull request are to documentation
@astrogeco astrogeco added breaking and removed breaking CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jan 26, 2022
@astrogeco astrogeco force-pushed the fix-codeql-workflow branch 4 times, most recently from 7986c51 to e3f8b77 Compare February 1, 2022 23:55
@astrogeco @ArielSAdamsNASA
Fix nasa#412, Rename CodeQL workflows
1d80995 
Renames workflows to better describe what each one does. The CodeQL
"reusable" workflow is meant to be used by other workflows.

The CodeQL "Analysis" workflow calls the "reusable" CodeQL workflow to
execute the static analysis runs.

Co-authored-by: Ariel Adams   <ArielSAdamsNASA@users.noreply.github.com>
@astrogeco astrogeco force-pushed the fix-codeql-workflow branch 16 times, most recently from b56c955 to 169cfdb Compare February 3, 2022 19:00
@astrogeco
Fix nasa#412, resolve error in CodeQL Analyze Action
98e2ef8 
Fixes errors in CodeQL results uploads step.

Update parameters in CodeQL "reusable" workflow.

BREAKING Interface changes:

- Renames callable workflow to `codeql-reusable.yml`, submodules will
have to be updated
- Adds required `component-path` input parameter
- Repurpose tests input to be a boolean tied to "ENABLE_UNIT_TESTS" flag

Internal changes:

- Use git clone instead of checkout@v2 for the cFS-Bundle
- Use symlink to map calling repo workspace to expected cFS Bundle directory location

- Enable "code snippets" option to CodeQL Analyze action
- Archives sarif files from analysis output
- Removes code duplication by using a matrix build for security and coding standard analyses
- Alphabetizes workflow inputs and order based on "required" flag
astrogeco added a commit that referenced this pull request Feb 3, 2022
@astrogeco
Merge pull request #413 from astrogeco/fix-codeql-workflow
1864992 
Fix #412, git error in CodeQL Analyze Action
astrogeco added a commit to astrogeco/cFE that referenced this pull request Feb 3, 2022
@astrogeco
Fix nasa#2034, Fix git error in CodeQL workflow
81b3a9c 
- Add new parameters to match updated cFS-CodeQL workflow interface
introduced in nasa/cFS#413
- Add file-exclusion checks to Action trigger so workflow doesn't run if
only changes in commit or pull request are to documentation
@astrogeco astrogeco mentioned this pull request Feb 3, 2022
Merged
2 tasks
@astrogeco astrogeco merged commit d6bca6e into nasa:main Feb 3, 2022
This was referenced Feb 23, 2022
Closed
Closed
Closed
This was referenced Apr 7, 2022
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper left review comments

@ArielSAdamsNASA ArielSAdamsNASA Awaiting requested review from ArielSAdamsNASA

Assignees
No one assigned
Labels
breaking continuous-integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reusable CodeQL Workflow Fails after "upload" stage
3 participants
@astrogeco @ArielSAdamsNASA @skliper