feat: Support for encrypted root disk in node_groups#1428
Conversation
|
+1 for the need for this feature. For some reason, the managed node group launch templates are lacking quite a few options compared to the self-managed worker launch templates, with disk encryption and metadata options being the most glaring. I'll submit a PR today for the metadata options as this PR covers the disk encryption quite nicely (I tested successfully on a fork of this repo). |
|
Would be nice to get this functionality, in other case launch template need to be created separately. |
| | disk\_encrypted | Whether the root disk will be encrypyted. Require `create_launch_template` to be `true` and require `disk_kms_key_id` to be set | bool | false | | ||
| | disk\_kms\_key\_id | KMS Key used to encrypt the root disk. Require both `create_launch_template` and `disk_encrypted` to be `true` | string | "" | |
There was a problem hiding this comment.
| | disk\_encrypted | Whether the root disk will be encrypyted. Require `create_launch_template` to be `true` and require `disk_kms_key_id` to be set | bool | false | | |
| | disk\_kms\_key\_id | KMS Key used to encrypt the root disk. Require both `create_launch_template` and `disk_encrypted` to be `true` | string | "" | | |
| | disk\_encrypted | Whether the root disk will be encrypyted. Requires `create_launch_template` to be `true` and require `disk_kms_key_id` to be set | bool | false | | |
| | disk\_kms\_key\_id | KMS Key used to encrypt the root disk. Requires both `create_launch_template` and `disk_encrypted` to be `true` | string | "" | |
There was a problem hiding this comment.
I can certainly change this. I elected to use the singular as it was the existing style in the other README entries for the sake of consistency, though I agree it's a little stilted.
There was a problem hiding this comment.
It's more than just stilted, it's grammatically incorrect. The verb needs to match in number with the implied subject.
Just noticed there's a second "require" on line 26 that also has incorrect number. It should probably be removed.
There was a problem hiding this comment.
Sorry for the delay. I updated the language in the README
alzabo
force-pushed
the
node_group_encryption
branch
2 times, most recently
from
94ecce8 to
777f29d
Compare
alzabo
force-pushed
the
node_group_encryption
branch
from
777f29d to
68fbcdb
Compare
bryantbiggs
left a comment
bryantbiggs
left a comment
There was a problem hiding this comment.
@antonbabenko / @barryib 👍🏼
|
Looks like this has one approval, any guess when it might be merged/released? Urgently needed for a project we're working on (which already uses the module). |
|
@jweigand Please see my other comment - #1459 (comment) . You will hear about this PR soon. |
|
Here we go! 🎉 v17.2.0 has been just released. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
7 participants
PR o'clock
Description
Add encryption config to node_group launch_group ebs config block.
I'm open to any thoughts about what the keys should be named. I chose to preface with
disk_for consistency with the other related keys.Checklist