Root Volume Encryption Validation

[andrewpurdin]

Hi There,

In testing the policy to validate encryption on EBS volumes I've noticed it works well for secondary volumes but doesn't run a test against the root volume. I'd like to fully validate all EBS volumes are encrypted.

I hacked something together locally in the transpiled encryptedVolumes.js, here are the lines I added:

      // If the root volume is not modified it is `undefined` on the instance which means it defaults
      // to unencrypted
      if (!instance.rootBlockDevice && !instance.ebsBlockDevices) {
          reportViolation(`The EC2 instance root block device must be encrypted.`)
      }

      if (instance.rootBlockDevice) {
          const rootBlockDevice = instance.rootBlockDevice
          if (!rootBlockDevice.encrypted) {
              reportViolation(`The EC2 instance root block device must be encrypted.`);
          }
          if (kmsId && rootBlockDevice.kmsKeyId !== kmsId) {
              reportViolation(`The EC2 instance root block device must be encrypted with required key: ${kmsId}.`);
          }
      }

I am happy to contribute via PR, but I was unable to get the integration tests to run successfully and there is no specific CONTRIBUTING.md file for this repo yet so I figured I'd open an issue instead.

Let me know if I can help contribute back :)

[justinvp]

Hi @andrewpurdin, thanks for opening the issue! Contributions are welcome!

We should add a CONTRIBUTING.md with instructions on what’s needed, but in the meantime, you’ll need Go 1.14 installed to run the integration tests and Pulumi configured to access AWS.

Though, the integration tests are actually going to create resources in AWS, so I can understand if you don’t want to run those locally. Also be aware, when you open a PR from a forked branch, the tests aren’t going to run in CI (as to not leak AWS creds), but I can pull down your changes from a PR and run tests locally before merging. Hope this helps! Let me know if you have any questions.

[andrewpurdin]

Awesome, I got the integration tests working (though a couple are failing with getVpc: no matching VPC found, probably not an issue I need to worry about though). I have 2 hopefully simple questions though. Can I run one integration test group (compute) or do I have to run the whole suite?

Secondly, I'd like to write a useful test or two, I noticed there's no unit tests for compute though, is that intentional? Should I focus on writing a passing integration test or should I also try to add unit tests?

Thanks for pointing me in the right direction :)

[justinvp]

Can I run one integration test group (compute) or do I have to run the whole suite?

You can run individual tests like:

go test -run TestComputeEC2 ./integration-tests/ -v

This will run the TestComputeEC2 test that's in integration-tests/integration_test.go

Secondly, I'd like to write a useful test or two, I noticed there's no unit tests for compute though, is that intentional? Should I focus on writing a passing integration test or should I also try to add unit tests?

The missing unit tests isn't intentional. I think we had some of the compute policies written before we had an approach for writing the unit tests in place, and haven't had a chance to go back and fill-in the missing tests. If you could add both unit tests and an integration test related to your change, that'd be much appreciated. Thank you!

[andrewpurdin]

Submitted #68 for your review @justinvp 😄 🤞

[justinvp]

Fixed by #68 🎉

[andrewpurdin]

Awesome, thanks so much for your help @justinvp. Just curious about the release step, is this an update that will roll out automatically?

[justinvp]

Just curious about the release step, is this an update that will roll out automatically?

I just opened https://github.com/pulumi/pulumi-policy-aws/pull/69 to prepare a v0.2.2 release. v0.2.2 should be available shortly after https://github.com/pulumi/pulumi-policy-aws/pull/69 has been merged (expecting sometime today).

In the meantime, you can update your package.json to depend on 0.2.2-dev.1591639817.

[justinvp]

@andrewpurdin, 0.2.2 is now available on NPM.