Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat(samples): add integrity verification to Cloud KMS crypto samples #409

Merged
merged 25 commits into from
Jan 8, 2021
Merged

feat(samples): add integrity verification to Cloud KMS crypto samples #409

merged 25 commits into from
Jan 8, 2021

Conversation

iamtamjam
Copy link
Contributor

@iamtamjam iamtamjam commented Dec 14, 2020 •
edited
Loading

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #408 🦕

@product-auto-label product-auto-label bot added the api: cloudkms Issues related to the googleapis/nodejs-kms API. label Dec 14, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 14, 2020
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 14, 2020
@codecov
Copy link

codecov bot commented Dec 14, 2020 •
edited
Loading

Codecov Report

Merging #409 (23125f0) into master (fbbabe6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #409   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files           5        5           
  Lines        3918     3918           
  Branches      127      127           
=======================================
  Hits         3837     3837           
  Misses         80       80           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbbabe6...23125f0. Read the comment docs.

@iamtamjam iamtamjam changed the title Add integrity verification to Cloud KMS crypto samples feat(samples): add integrity verification to Cloud KMS crypto samples Dec 14, 2020
@iamtamjam
feat(samples) pass plaintextCrc32c to encrypt
caf2d96
@iamtamjam
feat(samples): add fast-crc32c to allowModules for node/no-extraneous…
1e9daf5 
…-require rule

This module is required in order to demo the use of an external package to calculate crc32c of critical data fields.
@iamtamjam
feat(samples): fix build error
c3cd81d
@snippet-bot
Copy link

snippet-bot bot commented Dec 15, 2020 •
edited
Loading

Here is the summary of changes.

You deleted 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@iamtamjam iamtamjam marked this pull request as ready for review December 15, 2020 22:31
@iamtamjam iamtamjam requested a review from a team as a code owner December 15, 2020 22:31
@iamtamjam
Copy link
Contributor Author

@googleapis/yoshi-nodejs Hello reviewers. I'll be updating several files with changes similar to those in samples/encryptSymmetric.js. Can you please provide preliminary feedback to speed up the process?

Also, despite adding a rule to allow the use of external fast-crc32c module in package.json, I'm still seeing the following Lint error: "fast-crc32c" is extraneous node/no-extraneous-require. Can you please advise on how to resolve this?

@iamtamjam
Copy link
Contributor Author

@bcoe Hi bcoe! Hope you're the right person to ping. I'm awaiting review on the PR. Are you able to help with that?

I also need help with a persistent lint error: "fast-crc32c" is extraneous node/no-extraneous-require.
I followed instructions here and added a rules section to package.json with fast-crc32c module in the allowModules section, but to no avail.

Do I need to do something different?

Thanks in advance!
Tamara

bcoe
bcoe suggested changes Dec 21, 2020
View reviewed changes
Copy link
Contributor

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

Sorry for the slow review, I was away last week.

This looks reasonable to me, once you address the notes related to extraneous require.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@iamtamjam iamtamjam closed this Jan 5, 2021
@iamtamjam
Copy link
Contributor Author

iamtamjam commented Jan 5, 2021 •
edited
Loading

@bcoe Can you please take another look and merge?

@iamtamjam iamtamjam reopened this Jan 5, 2021
@iamtamjam
Copy link
Contributor Author

Ah.. accidentally closed this PR. @bcoe , your proposed changes have resolved the lint issue. Can you please merge this PR? Thanks in advance!

@iamtamjam iamtamjam requested a review from bcoe January 7, 2021 17:02
@iamtamjam
Copy link
Contributor Author

@bcoe I see you updated this branch. Just to confirm: this PR is ready to be merged, right?

@bcoe bcoe merged commit d2897f6 into googleapis:master Jan 8, 2021
@bcoe
Copy link
Contributor

bcoe commented Jan 8, 2021

@iamtamjam sorry for the delay, merged 👍

@iamtamjam
Copy link
Contributor Author

Thanks for reviewing and merging, @bcoe !

gcf-merge-on-green bot pushed a commit that referenced this pull request Jan 12, 2021
@release-please
chore: release 2.3.0 (#414)
6a48a69 
🤖 I have created a release \*beep\* \*boop\* 
---
## [2.3.0](https://www.github.com/googleapis/nodejs-kms/compare/v2.2.0...v2.3.0) (2021-01-08)


### Features

* **samples:** add integrity verification to Cloud KMS crypto samples ([#409](https://www.github.com/googleapis/nodejs-kms/issues/409)) ([d2897f6](https://www.github.com/googleapis/nodejs-kms/commit/d2897f681ae409b34a50b91ea718fa9e294895c5))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bcoe bcoe bcoe approved these changes

Assignees
No one assigned
Labels
api: cloudkms Issues related to the googleapis/nodejs-kms API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[kms] Add integrity verification to Cloud KMS's node crypto samples
2 participants
@iamtamjam @bcoe