feat(kms): Add KMS under beta

[JanStern]

Description

relates to #934

KMS has been added to the CLI. Now the following commands exist:

• key (create, delete, import, list, restore, rotate)
• key ring (create, delete, list)
• version (destroy, disable, enable, list, restore)
• wrapping key (create, destroy, list)

Checklist

• Issue was linked above
• Code format was applied: make fmt
• Examples were added / adjusted (see e.g. here)
• Docs are up-to-date: make generate-docs (will be checked by CI)
• Unit tests got implemented or updated
• Unit tests are passing: make test (will be checked by CI)
• No linter issues: make lint (will be checked by CI)

---

Important Decisions

The CLI implementation of KMS reflects the state of the API, which includes some seemingly unfinished decisions.

1. The Region doesn't matter even though it exists: Every API Endpoint has the region as a required attribute. However, the value is currently meaningless. All requests have the same result no matter what the region is. Still I haven't removed/mocked the structure in the code since I assume that in the future KMS will be region specific.
2. Backend is a fixed but required value: When creating a wrapped key or a key, backend is a required body parameter that is currently only "software". Here following the same argument as before I have made it a flag in anticipation of future updates.
3. I recommend extra detail to Importing a Key: Following this example I experimented with the key importing and tried to import an invalid key. I did encode random text in base64 and it was accepted. It created a new key version but with the status "Errors existing", which feels wrong. Nonetheless, I don't think that the formatting checks in the CLI should be stricter. (I just wanted to draw attention to that)

---

Hope this actually helps and huge thanks to whomever tries to tackle this monster merge.

[JanStern]

@rubenhoenle are there any updates on the PR? I would love to use it in my pipeline.

[rubenhoenle]

The Region doesn't matter even though it exists: Every API Endpoint has the region as a required attribute. However, the value is currently meaningless. All requests have the same result no matter what the region is. Still I haven't removed/mocked the structure in the code since I assume that in the future KMS will be region specific.

Well, the region is required for every API endpoint, it will just be "eu01" for now all the time. But please use the regular multi-region implementation like we do for all the other commands. So we're ready for the future :)

Backend is a fixed but required value: When creating a wrapped key or a key, backend is a required body parameter that is currently only "software". Here following the same argument as before I have made it a flag in anticipation of future updates.

Backend was deprecated in the v1beta version of the KMS service API already and got removed with the switch to the v1 version. So you'll have to remove it anyways, sorry 😄

[ruslan-18]

With that said, it would be awesome to be able to overwrite the strict 30 days deletion schedulation period. This is my formal request for that.

Hi Jan,
thanks for the request. As Ruben mentioned, he'd forward it to the KMS team (which I'm a part of), so here's our view on that matter:

In general, I understand that you want to be able to overwrite the deletion schedule. In a lot of cases, this would be more convenient for sure.
However, for us it's not that simple. What we really want to ensure with this deletion period is that users of the KMS don't accidentally lock themselves out of their systems. Since keys are used to encrypt all sorts of data, and are very critical in that matter, we want to make sure that accidental key deletion doesn't happen. And if it does happen for some reason, there should be enough time to react to it, hence the 30 days. The important part here is that once the key/version is deleted, there is absolutely no way for us to restore it. That's why we're sticking to the scheduled deletion.

I'm sorry that we can't fulfill your request, but all things considered, we have to stay on the safe side here.

Feel free to reach out to us at any time if anything KMS-related should arise.

Kind regards
Ruslan

[marceljk]

Thanks for your contribution. Code looks good, only some nitpicks, to be consistent with our other commands in the CLI

[JanStern]

@marceljk thank you for your reviews. You brought up excellent points that I both fully agree with and should have done a better job catching it myself.
I addressed all your request to the best of my abilities. I went through the code again myself and tried to proactively fix further nitpicks and mistakes.

I probably still missed something since I feel blind to my own mistakes after so many rewrites and adaptations. Thus, I appreciate your effort and request it one more time 🙃

[marceljk]

Thanks again for your work! I really appreciate the improved docs, this helps to have an easier start with the kms commands 😄

[JanStern]

Got it @marceljk. These are small and quickly addressed changes 😊

I'm ready for the next round 🙃

[marceljk]

Looks good to me and is approved from my side

[marceljk]

There are some open comments from @rubenhoenle which aren't resolve yet.
#935 (comment)
#935 (comment)

[JanStern]

• feat(kms): Add KMS under beta #935 (comment): just left a comment there, as I addressed it a little bit further down in the code.
• feat(kms): Add KMS under beta #935 (comment): missed that. Just pushed it.