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

2521 Refactor CVE and Product Info Tables #2526

Merged
merged 11 commits into from
Feb 22, 2024

Conversation

Matthew-Grayson
Copy link
Contributor

@Matthew-Grayson Matthew-Grayson commented Feb 12, 2024

🗣 Description

  • Refactor column names of cve and product_info tables to camel case
  • Rename product_info table to cpe
  • Remove redundant cpe_list column.
  • Update associated endpoints, joins, API calls, and tests
  • Update syncdb populate to include cve and cpe tables

💭 Motivation and context

We recently added two tables, cve and product_info. The column names follow a different naming convention than the rest of the codebase. For one thing they use snake case instead of camel case. This makes our API inconsistent and adds confusion to frontend development.

I renamed the product_info table to cpe so that references to entries are more informative: cpe = single entry and cpes is a collection of entries. This isn't the case now with product_info being used as both singular and plural.

I also removed the cpe_list column since there is already a join table between cves and cpes coded into the db model.

Closes issue #2534
Closes issue #2521

🧪 Testing

Modified tests for API functionality. Manually tested Vulnerability Details page.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Revert dependencies to default branches.
  • Finalize version.

✅ Post-merge checklist

  • Create a release.

@Matthew-Grayson Matthew-Grayson linked an issue Feb 12, 2024 that may be closed by this pull request
@Matthew-Grayson Matthew-Grayson changed the title 2521 refactor CVE and product info tables 2521 Refactor CVE and Product Info Tables Feb 12, 2024
…tency; update associated endpoints, tests, and API calls; remove redundant cpe_list column.
@Matthew-Grayson Matthew-Grayson added Blocked Something must be completed before this task can proceed.. Refactor Restructuring existing computer code without changing its behavior. labels Feb 12, 2024
@Matthew-Grayson
Copy link
Contributor Author

PR currently includes commits from #2352. I will mark this ready for review once #2352 is merged.

@Matthew-Grayson Matthew-Grayson added Backend ASM-VDB Technical Infrastructure and Database API Establishes how two apps communicate with each other using requests and responses. and removed Blocked Something must be completed before this task can proceed.. labels Feb 13, 2024
@Matthew-Grayson Matthew-Grayson added the Blocked Something must be completed before this task can proceed.. label Feb 13, 2024
@Matthew-Grayson
Copy link
Contributor Author

This is difficult to test due to issues calling the PE API today.
Let's prioritize adding the new tables to the syncdb populate method to assist local development.

Copy link
Contributor

@ameliav ameliav left a comment

Choose a reason for hiding this comment

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

Looks good so far after rebuilding the app from scratch, but won't be able to properly review until after the P&E API is working again and I can get some data in with cveSync.

@Matthew-Grayson Matthew-Grayson marked this pull request as draft February 15, 2024 17:42
auto-merge was automatically disabled February 15, 2024 17:42

Pull request was converted to draft

…Db method; separate leftJoinAndSelect() and where() methods to restore cpe/cve endpoint functionality.
@Matthew-Grayson Matthew-Grayson marked this pull request as ready for review February 15, 2024 20:37
@Matthew-Grayson
Copy link
Contributor Author

I've updated syncdb's populate method to include the cve and cpe tables to assist local development. To this PR's changes:

  1. Run npm run syncdb -- -d dangerouslypopulate
  2. Run npm run syncdb
  3. Run npm run syncdb -- -d populate
  4. Confirm that vulnerability details page populates data properly
  5. Confirm that all tests pass

@Matthew-Grayson Matthew-Grayson removed the Blocked Something must be completed before this task can proceed.. label Feb 16, 2024
@Matthew-Grayson Matthew-Grayson marked this pull request as draft February 16, 2024 15:30
Matthew-Grayson and others added 3 commits February 16, 2024 11:39
….onConflict method so that modifiedAt field isn't overwritten with current time, and add skipUpdateIdNotValuesChanged property to reduce inserts. Add optional parameters to CveEntry interface to match response schema from PE endpoint. Add CVE name to console logs.
@Matthew-Grayson Matthew-Grayson marked this pull request as ready for review February 16, 2024 18:39
@Matthew-Grayson Matthew-Grayson self-assigned this Feb 16, 2024
@ameliav ameliav self-requested a review February 16, 2024 19:00
Copy link
Contributor

@ameliav ameliav left a comment

Choose a reason for hiding this comment

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

LGTM, confirmed that test data, cveSync, and vulnerability details page all works/loads correctly

…ishedAt/modifiedAt columns nullable to match PE API. Minor formating and refactoring.
…ishedAt/modifiedAt columns nullable to match PE API. Minor formating and refactoring.
…m:cisagov/crossfeed into 2521-refactor-cve-and-product_info-tables
@Matthew-Grayson Matthew-Grayson merged commit 0d20066 into master Feb 22, 2024
18 of 19 checks passed
@Matthew-Grayson Matthew-Grayson deleted the 2521-refactor-cve-and-product_info-tables branch February 22, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Establishes how two apps communicate with each other using requests and responses. Backend ASM-VDB Technical Infrastructure and Database Refactor Restructuring existing computer code without changing its behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor cve and product_info Tables
3 participants