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

Skeletonize #46

Merged
merged 553 commits into from
Mar 2, 2022
Merged

Skeletonize #46

merged 553 commits into from
Mar 2, 2022

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Jan 28, 2022 •
edited
Loading

🗣 Description

This pull request skeletonizes this repository and brings it into line with the analogous changes that were made to cisagov/trustymail_reporter.

💭 Motivation and context

This repository deserves to get Lineage updates too!

This pull request also resolves #8.

🧪 Testing

I built a new Docker image with these changes and successfully used it to perform a full run of the BOD 18-01 scanning.

✅ 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.

✅ Post-merge checklist

  • Add a tag.

mcdonnnj and others added 30 commits October 22, 2020 13:54
@mcdonnnj
Merge pull request #55 from cisagov/migrate_prettier_hook
e44762c 
Change repo for the prettier pre-commit hook
@jsf9k
Merge remote-tracking branch 'skeleton-generic/develop' into lineage/…
bcd7a3a 
…skeleton
@jsf9k
Merge pull request #37 from cisagov/lineage/skeleton
b6f9d1f 
⚠️ CONFLICT! Lineage pull request for: skeleton
@felddy
Add script to generate cross-platform Dockerfile.
4f14cc6
@felddy
Add git ignore for generated Dockerfile-x.
ba039e8
@felddy
Replace previous Docker workflows with a moderinzed, complete-workflow.
3d0720b 
These changes are based on the "complete workflow" from
https://github.com/docker/build-push-action

Additionally it adds support for the new "workflow_dispatch" event type.
@felddy
Add CodeQL workflow.
1e8dd53 
Needed to check tests written in Python.
@felddy
Update README publisher to use new Docker URL.
7530392
@felddy
Expand command option names to long form.
76460df
@felddy
Update Dockerfile to use opencontainers metadata and build-time labels.
7601051
@felddy
Prevent image pushes when linter checks fail.
e01481b
@felddy @jsf9k
Modify cache keys to follow team-established naming conventions.
297e527 
Co-authored-by: Shane Frasier <[email protected]>
@felddy
Add comments pointing to the opencontainers image-spec documentation.
f1e097a
@felddy
Add comments to help document and break up a monolitic file.
a88d550
@jsf9k
Insist that the cisagov devs are the owners of the .github directory
a842abb 
This additional clause must remain at the _end_ of the CODEOWNERS file
so that it cannot be overridden by a later clause.

We want to make it so that all the .github files including CODEOWNERS
are protected so only code owners (the dev team) can approve
modifications to them.

This will prevent configuration changes from breaking Actions and
other management-type functions that the files in this directory
control. By setting the .github files/folder to require code owner
approval for changes, workflow and management changes will require dev
team review and checking.

Resolves #56.
@jsf9k
Remove offending slash
5b199bc 
Thanks to @dav3r and @mcdonnnj for the suggestion.
@felddy
Apply correct typographic designation to character replacement comment.
a29dc98
@felddy
Modernize Docker shields in README to point to working links.
e942672
@felddy @jsf9k
Elaborate on the Solidus.
da037f5 
Add some additional comments in case anyone is confused about the nature of the solidus and its place within our tagging system.

Co-authored-by: Shane Frasier <[email protected]>
@felddy
Merge pull request #38 from cisagov/improvement/modern_docker_workflow
9e993fb 
Modernize Docker workflow
@jsf9k
Use the python version output by actions/setup-python
02c5a6f 
There is no need to run python code to determine the python version.
Resolves #58.

See here for details:
https://github.com/actions/setup-python/blob/main/action.yml#L14-L16
@mcdonnnj
Organize pre-commit hooks
64c3fb6 
Group related pre-commit hooks together. Make sure that hooks are
alphabetically sorted within those groups.
@mcdonnnj
Merge pull request #60 from cisagov/organize_pre-commit_requirements
763d247 
Organize and Sort pre-commit Hooks
@jsf9k
Merge branch 'develop' into improvement/insist-that-devs-own-gihub-dir
1310cfd
@jsf9k
Merge pull request #57 from cisagov/improvement/insist-that-devs-own-…
2a2067f 
…gihub-dir

Ensure that the cisagov devs are the owners of the .github directory
@jsf9k
Merge branch 'develop' into improvement/get-python-version-more-simply
b638143
@jsf9k
Merge pull request #59 from cisagov/improvement/get-python-version-mo…
3b312c4 
…re-simply

Use the python version output by actions/setup-python
@jsf9k
Merge remote-tracking branch 'skeleton-generic/develop' into lineage/…
96de3d8 
…skeleton
@mcdonnnj
Fix test job missing Python version in cache key
9b62ff0
@mcdonnnj
Merge pull request #40 from cisagov/bugfix/fix_incorrect_caching
dc1b7c7 
Add Python Version Missing from actions/cache Key
@jsf9k jsf9k requested review from dav3r and mcdonnnj February 4, 2022 22:42
@jsf9k jsf9k mentioned this pull request Feb 7, 2022
Open
dav3r
dav3r approved these changes Feb 7, 2022
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.

Approval intensifies

mcdonnnj
mcdonnnj approved these changes Feb 14, 2022
View reviewed changes
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Another slew of Python 3 changes for the most part: Using f-strings and removing object from class descriptions. I did have a question about a repeated line of code and a request about the docopt definition for the generate_https_scan_report.py file. Thanks again for putting up with all of my nitpicky requests.

tests/conftest.py Outdated Show resolved Hide resolved
tests/container_test.py Outdated Show resolved Hide resolved
src/report.sh Show resolved Hide resolved
src/report/create_all_reports.py Outdated Show resolved Hide resolved
src/report/create_all_reports.py Outdated Show resolved Hide resolved
src/report/graphs.py Outdated Show resolved Hide resolved
src/report/graphs.py Outdated Show resolved Hide resolved
src/report/graphs.py Outdated Show resolved Hide resolved
src/report/graphs.py Outdated Show resolved Hide resolved
src/report/graphs.py Outdated Show resolved Hide resolved
jsf9k added 2 commits February 22, 2022 16:15
@jsf9k
Correct some path references
f478b2e 
This commit fixes a few paths that are still referring to the old
/home/reporter path.  Such references should instead be referring to
/home/cisa.
@jsf9k
Use Collection.count_documents() instead of Cursor.count()
71e0043 
Cursor.count() has been deprecated for some time, and is now
disallowed.
@jsf9k jsf9k requested review from mcdonnnj and dav3r February 23, 2022 04:20
dav3r
dav3r approved these changes Feb 23, 2022
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.

Excellent work! 💪

@jsf9k
Copy link
Member Author

jsf9k commented Feb 23, 2022 •
edited
Loading

I have confirmed via a full BOD 18-01 reporting run that this code - with the reversion described here - functions as expected.

@jsf9k jsf9k mentioned this pull request Feb 25, 2022
Open
@jsf9k
Revert "Use an absolute import"
fa8f6e3 
This reverts commit 61cf660.

This change was rejected at runtime since the code in src/report does
not constitute a Python package.  See also issue #50.
mcdonnnj
mcdonnnj approved these changes Mar 1, 2022
View reviewed changes
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Just some last little suggestions. Really strong work getting this into a more modern state!

Dockerfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/report/generate_https_scan_report.py Outdated Show resolved Hide resolved
src/report/generate_https_scan_report.py Outdated Show resolved Hide resolved
@jsf9k jsf9k merged commit 0fee977 into develop Mar 2, 2022
@jsf9k jsf9k deleted the skeletonize branch March 2, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcdonnnj mcdonnnj mcdonnnj approved these changes

@dav3r dav3r dav3r approved these changes

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

Assignees

@jsf9k jsf9k

Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use version bump This issue or pull request increments the version number
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use the --no-cache-dir flag with pip
4 participants
@jsf9k @dav3r @mcdonnnj @felddy