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

First commits to create cyhy-kevsync #3

Merged
merged 59 commits into from
Oct 21, 2024
Merged

First commits to create cyhy-kevsync #3

merged 59 commits into from
Oct 21, 2024

Conversation

felddy
Copy link
Member

@felddy felddy commented Oct 11, 2024 •
edited by dav3r
Loading

🗣 Description

This PR contains the initial functionality of the cyhy-kevsync module, which synchronizes CISAs known exploited vulnerabilities list to the Cyber Hygiene database.

💭 Motivation and context

We have two main motivations here:

  1. As a demonstration of the usage of our new database object model for Python 3.
  2. Conversion of our previous kev-sync tool to Python 3 and its final form as an AWS lambda.

🧪 Testing

This code is currently 92% covered by unit tests. Additionally, it has been tested against live KEV data, and a MongoDB instance in AWS.

✅ 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 (install_requires block in setup.py)
  • Finalize version.

✅ Post-merge checklist

  • Create a release.

@felddy felddy added documentation This issue or pull request improves or adds to documentation hacktoberfest-accepted Marks a pull request as counting toward Hacktoberfest participation. improvement This issue or pull request will add new or improve existing functionality labels Oct 11, 2024
@felddy felddy self-assigned this Oct 11, 2024
@felddy felddy added hacktoberfest-accepted Marks a pull request as counting toward Hacktoberfest participation. and removed hacktoberfest-accepted Marks a pull request as counting toward Hacktoberfest participation. labels Oct 11, 2024
@felddy felddy marked this pull request as ready for review October 11, 2024 18:49
@felddy felddy marked this pull request as draft October 11, 2024 19:09
jsf9k
jsf9k requested changes Oct 11, 2024
View reviewed changes
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

I verified that all tests run via pytest -vs --mongo-express pass locally.

setup-env Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
jsf9k
jsf9k requested changes Oct 11, 2024
View reviewed changes
setup.py Outdated Show resolved Hide resolved
jsf9k
jsf9k requested changes Oct 11, 2024
View reviewed changes
setup.py Show resolved Hide resolved
@felddy felddy marked this pull request as ready for review October 11, 2024 20:44
dav3r
dav3r approved these changes Oct 15, 2024
View reviewed changes
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍 though I noted some small items that should be cleaned up.

src/cyhy_kevsync/__init__.py Outdated Show resolved Hide resolved
src/cyhy_kevsync/kev_sync.py Outdated Show resolved Hide resolved
src/cyhy_kevsync/kev_sync.py Outdated Show resolved Hide resolved
tests/test___main__.py Outdated Show resolved Hide resolved
tests/test_kevsync.py Show resolved Hide resolved
dav3r
dav3r approved these changes Oct 18, 2024
View reviewed changes
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

felddy and others added 18 commits October 18, 2024 16:32
@felddy
Change minimum Python version to 3.12
d46e681
@felddy
Refactor sync code to keep track of create, update, and delete
44f4083
@felddy
Refactor to use new version of cyhy_config API
413b3b8
@felddy
Add optional log_level to config
e61efdc
@felddy
Move mypy later in the setup process
7f0d955 
mypy can fail and prevent the installation of pre-commit hooks.  It isn't obvious when this occurs.   Moving it after
the `set +o errexit` will prevent this.
@felddy
Refactor main logic to support external calls
ba3915c
@felddy
Move constants above imports to prevent circular import error
7131c80
@felddy
Refactor do_kev_sync function signature to make config_file optional
d7e1a44
@felddy
Ensure log_level argument is case-insensitive
4a6d159
@felddy
Add missing release test and clean comments
723781b
@felddy
Replace logging with cyhy-logging
efb187d
@dav3r @felddy
Output mongodb URI at end of pytest run when used with --mongo-expres…
d46e61f 
…s option

Co-authored-by: Mark Feldhousen <mark.feldhousen@gwe.cisa.dhs.gov>
@dav3r @felddy
Replace skeleton info with useful info in README
3ff14fe
@dav3r @felddy
Make typing changes to appease mypy
2c5dd8f 
Add more unit tests

Add output assertions to unit tests

Add two additional unit tests
@felddy
Reduce Python matrix to support versions
7dbe828
@dav3r @felddy
Use the default branch of cisagov/cyhy-config
9fb5948 
We can do this now that the first-commits branch of cyhy-config has been merged.
@dav3r @felddy
Shorten assertion check text
03ad659 
This is to avoid situations where the stdout is split across multiple lines (e.g. when this test is run in GitHub Actions) and the full text of the warning message is not contiguous.

To clarify, here is how the pervious version of this test was failing in GHA:

AssertionError: assert 'No schema URL provided, skipping KEV JSON validation' in '[19:05:08] WARNING  No schema URL provided, skipping KEV JSON         main.py:46\n                    validation.                                                 \nProcessing KEV feed ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00\nDeleting KEV docs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   \n'

Update cyhy-config dependency to version 1

Remove unnecessary dependency pins
@felddy
Update CLI tool entry point name comment
6fc0571 
Sort `__all__` list

Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Make error message text more specific

Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Make error message text more specific

Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Add an assertion error message

Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Increase granularity of exceptions

Add tests for specific validation failure modes

Fix docstring and normalize http exception

Improve test coverage

Add test for non-200 responses during schema fetch
@felddy felddy added this pull request to the merge queue Oct 21, 2024
@felddy felddy removed this pull request from the merge queue due to a manual request Oct 21, 2024
@felddy felddy added this pull request to the merge queue Oct 21, 2024
Merged via the queue into develop with commit 28e8ccc Oct 21, 2024
17 checks passed
@felddy felddy deleted the first-commits branch October 21, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dav3r dav3r dav3r approved these changes

@jsf9k jsf9k jsf9k approved these changes

@mcdonnnj mcdonnnj Awaiting requested review from mcdonnnj mcdonnnj is a code owner

Assignees

@felddy felddy

Labels
documentation This issue or pull request improves or adds to documentation hacktoberfest-accepted Marks a pull request as counting toward Hacktoberfest participation. improvement This issue or pull request will add new or improve existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@felddy @jsf9k @dav3r