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 sha256 checksum to json #1796

Merged
merged 1 commit into from
May 11, 2017
Merged

add sha256 checksum to json #1796

merged 1 commit into from
May 11, 2017

Conversation

arlimus
Copy link
Contributor

@arlimus arlimus commented May 10, 2017

Fixes #1658

Signed-off-by: Dominik Richter dominik.richter@gmail.com

@alexpop
Copy link
Contributor

alexpop commented May 10, 2017

Nicely done Dom, works as expected:

$ bundle exec inspec exec ~/git/mycompliance-profile/mylinux.tar.gz https://github.com/dev-sec/apache-baseline/archive/master.zip --format json | jq . | grep sha256
      "sha256": "ddee0a628a81c947edd6ffec146a2fb1e2fe1b3e556ff4e6122b5d7ca92e9657"
      "sha256": "3e1310b071dc4d706263e9d07083e10a92b4b69e4a36cffa1eda7eaecc09969a"

$ bundle exec inspec json ~/git/mycompliance-profile/mylinux/ | jq . | grep sha256
  "sha256": "ddee0a628a81c947edd6ffec146a2fb1e2fe1b3e556ff4e6122b5d7ca92e9657"

$ echo ' ' >> ~/git/mycompliance-profile/mylinux/inspec.yml

$ bundle exec inspec json ~/git/mycompliance-profile/mylinux/ | jq . | grep sha256
  "sha256": "479b196e121dd93ba767d7019647f0c48fbbbf099c4301cbc57fb5034acdf207"

Shouldn't json-min contain this field as well?

@arlimus arlimus changed the title WIP add sha256 checksum to json add sha256 checksum to json May 10, 2017
@arlimus
Copy link
Contributor Author

arlimus commented May 10, 2017

@alexpop you are spot on about json-min 😁 I wanted to do this step first and let json-min come in another PR.

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Great addition @arlimus

Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

Before we merge, any chance we can add a test with an existing dummy profile that checks to ensure we return the correct checksum for the contents? I don't see any tests for the sha256 method.

Otherwise, this is awesome.

@arlimus
Copy link
Contributor Author

arlimus commented May 11, 2017

@adamleff I had a test in the functional bits: https://github.com/chef/inspec/pull/1796/files#diff-ce23f8753648cc3a9d77312e123c452cR61

I will add the unit test as well, thank you for the suggestion @adamleff !

Fixes #1658

Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
@arlimus arlimus merged commit ef77392 into master May 11, 2017
@arlimus arlimus deleted the dr/checksum branch May 11, 2017 08:10
@adamleff adamleff added the Type: New Feature Adds new functionality label May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: New Feature Adds new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants