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

Add VDP Hash to the Domain Results CSV #15

Merged
merged 3 commits into from
Jul 23, 2021

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Jul 22, 2021

🗣 Description

This PR adds the hash of a visited URL to the domain results output CSV.

💭 Motivation and context

This will allow for analysts to compare domains week-to-week to see if a change has occurred by comparing the respective hashes. This will close #12 .

🧪 Testing

I verified this worked locally and automatic tests pass.

✅ Pre-Approval Checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Pre-Merge Checklist

  • Bump the patch version

mcdonnnj added 2 commits July 21, 2021 16:18
This adds a "VDP Hash" field to the domain CSV output. This field will contain
the hash of the retrieved and parsed VDP per the cisagov/hash-http-content
project. This field contains the hash on an HTTP OK response for the resolved
URL and an empty string for anything else.
Switch to using zip() and casting to a dict to populate the results dictionary.
This will allow existing definition to be the "source of truth" for the
contents of the dictionary. This will reduce human error (or at least make it
consistent) and improve maintainability.
@mcdonnnj mcdonnnj added the improvement This issue or pull request will add or improve functionality, maintainability, or ease of use label Jul 22, 2021
@mcdonnnj mcdonnnj requested a review from dav3r as a code owner July 22, 2021 14:37
@mcdonnnj mcdonnnj self-assigned this Jul 22, 2021
@mcdonnnj mcdonnnj requested review from felddy and jsf9k as code owners July 22, 2021 14:37
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.

These changes look great to me. Great work!

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.

Great work here! :electron:

# contents of result as values. This leverages the fact that the
# DomainResult NamedTuple is positionally aligned with the contents of
# the domain_csv_header list.
self.domain_results.append(dict(zip(self.domain_csv_header, result)))
Copy link
Member

Choose a reason for hiding this comment

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

Nice! 👍

mcdonnnj added a commit to cisagov/ansible-role-vdp-scanner that referenced this pull request Jul 23, 2021
This release includes maintenance updates and the functionality described in
cisagov/vdp-scanner-docker#15.
@mcdonnnj mcdonnnj merged commit 7315678 into develop Jul 23, 2021
@mcdonnnj mcdonnnj deleted the improvement/add_hash_to_domain_csv branch July 23, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Hash of Site's VDP to CSV Output
3 participants