Add KmsKeyId to LogGroup

[benbridts]

~This is a Work in Progress, if that's better as an issue, let me know.~

I tried to tackle the difficult parts first, which for me besides the whole of Java is writing test. I haven't started writing code yet. Assistance with both parts is very welcome.

I'm opening this PR mainly to get feedback on the approach. Currently I have the following questions related to the work already in here:

• Spec is step 1, so that can already get a decent review :)
• Is this the way to update the tests, or do we prefer completely new tests that only deal with the KmsKeyId

I also have question for the work that isn't in here yet:

• There are three related methods in the API: "CreateLogGroup" with the "kmsKeyId" parameter, "AssociateKmsKey" and "DisassociateKmsKey"
• This allows to approaches in the CreateHandler. Doing it in two calls (CreateLogGroup without kmsKeyId and AssociateKmsKey if it's not Null on the model), which leaves the LogGroup without encryption for some time (and in that time some messages might appear). Or doing it in one call, which uses a different flow for the createHandler and the updateHandler.
• In the updateHandler we have to do it in two steps, which is something that we either shouldn't support (only allowing going from encrypted to plaintext and vice versa) or should highlight in the documentation. The risk of log lines arriving while this change is in progress is even higher than with the CreateHandler.

We should probably decide on the best approach before writing the code.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[rjlohan]

Not super important, but you can do 'Draft' PRs to signal WIP. We can review them entirely, but they're not merge-able.

[benbridts]

Current blocked from (easily/comfortably) writing the implementation by #25 (and a little bit by #28, but I got that working locally by pulling in #29 )

[rjlohan]

If you merge from master, you'll get a Travis build for this PR from now on. 👍

Also, to be consistent with other repos, I updated the space indenting for JSON files from 4-->2 so you'll need to run pre-commit run --all to fix the schema file after rebasing/merging.

[rjlohan]

@ikben how's this PR going?

[benbridts]

I didn't get to pulling in the recent changes yet, hopefully I will have some time to look at it in the next few days

[benbridts]

I think this is what the implementation would look like. Please note:

• contract tests still fail, because of #25 additionally they now also do not return the KmsKeyId (for the same reason). The ARN is the bigger issue in my opinion, until there is a decision made about haw to tackle that, it does not make a lot of sense to fix that here.
• 7596883d220658aa5f78113e94339a289f3340e4 and ff7dc2d813ab662e5d99abd04be9c60cdd18ebce are useful even without adding the KmsKeyId, so if it turns out that there is still a lot more work on this, they can be cherry-picked.
• I wanted to add the docs (similar to how resource-role.yaml is included), but that file does not have a newline at the end.

[benbridts]

I rewrote a few if statements and added a bunch of tests to get the required coverage.

The issue about having model properties that can be null is still open, but I'll wait on feedback for the best approach there

[Lukas-Franz]

Thanks for the implementation! We've just tested this out and it seems not to be in production as of yet.

A bit of topic, but important nonethelss: Is there a way to get notified or keep track of changes like these as they make their way through quality assurance and to production? As far as I'm aware, the only way right now is trial and error, which is quite frustrating.

[benbridts]

@OfficialTermi

• The best way to see that should be the CloudFormation Coverage Roadmap Issue. Unfortunately, that issue hasn't been moved to the roadmap yet (for the AWS-folks here, is the coverage roadmap updated if the PR is coming from outside of AWS?)
• That does not mean you have to do it with trial and error though. You can look at:
  • The CloudFormation Release History (it also has an RSS feed)
  • The CloudFormation Resource Specification
  • The API, using the CLI to describe the type definition and check the Schema for "KmsKeyId": aws cloudformation describe-type --type RESOURCE --type-name AWS::Logs::LogGroup | grep KmsKeyId