Bug 1738354: pkg/asset/machines/aws/machines: Request encrypted root volumes #2160
Conversation
The AWS cluster-API provider just started respecting this property in openshift/cluster-api-provider-aws@99de8f2015 (Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field, 2019-08-05, openshift/cluster-api-provider-aws#245). By asking for it, we'll get encrypted root volumes for compute machines and remove the need for copy-and-encrypting the control-plane machines.
openshift-ci-robot
added
the
size/S
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
wking
force-pushed
the
request-encrypted-root-volumes
branch
from
441eb9f to
dc74ef7
Compare
|
The control-plane is still created by the installer right, so this change is a regression that control-plane is not encrypted anymore... /hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
e2e-aws failed on: Seems unrelated (and like rhbz#1713135). /retest |
|
Also looking at https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSEncryption.html#encryption-by-default and other docs it looks like the setting needs to be enabled region level before the API can use this?? |
Ah, right :p. I'll roll that part back and keep passing the encrypted AMI to Terraform for those machines.
Yeah, and that's obviously not great. But this PR isn't about that setting (although I've mentioned it in the past in this space); this PR is about explicitly setting a given root volume encrypted. Which Terraform doesn't support yet (hashicorp/terraform-provider-aws#8624), but which the cluster-API provider now does support. |
Partially reverts 0c370dd (data/aws: Encrypt the AMI used by the bootstrap and master machines, 2019-02-22, openshift#1296). This isn't a clean revert; for example, I left the ability to destroy images which are tagged as owned by the cluster. And we're still copy-and-encrypting for the bootstrap machine and control-plane machines until the AWS Terraform provider supports requesting encrypted root volumes [1]. But with this commit, we're now documenting the encryption in a way that covers both the previous AMI-based encryption used for bootstrap/control-plane and the new root-volume-based encryption used for the compute machines, because they come down to encrypted root volumes regardless of their approach. [1]: hashicorp/terraform-provider-aws#8624
wking
force-pushed
the
request-encrypted-root-volumes
branch
from
dc74ef7 to
58c6413
Compare
openshift-ci-robot
added
size/XS
|
Back to copy-and-encrypt AMI IDs for control-plane machines with dc74ef7c5 -> 58c6413. |
|
doesn't capture the reality that we are still copying AMIs. |
|
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
wking
changed the title
|
@wking: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
|
/close |
|
@sdodson: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Terraform 2.23.0 (which we're pulling in with #2676) is getting us hashicorp/terraform-provider-aws#7757 which might also let us drop the copy-and-encrypt. Worth reopening this now that we have that on the table? |
The AWS cluster-API provider just started respecting this property in openshift/cluster-api-provider-aws@99de8f2015 (openshift/cluster-api-provider-aws#245). By asking for it, we'll get encrypted root volumes for compute machines and remove the need for copy-and-encrypting the control-plane machines.
This PR also partially reverts 0c370dd (#1296). This isn't a clean revert; for example, I left the ability to destroy images which are tagged as owned by the cluster. And we're still copy-and-encrypting for the bootstrap machine until the AWS Terraform provider supports requesting encrypted root volumes (hashicorp/terraform-provider-aws#8624). But with this commit, we no longer use the copied AMI for control-plane nodes, which gets us one step closer to having the cluster API provision them without installer intervention.