Skip to content

Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field#245

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
ingvagabund:wire-ebs-encoded-field-to-ec2-ebs-one
Aug 5, 2019
Merged

Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field#245
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
ingvagabund:wire-ebs-encoded-field-to-ec2-ebs-one

Conversation

@ingvagabund
Copy link
Member

AWS actuator allows to specify encrypted root volumes for compute machines.
Though, it does not wire it to EBS definition which is passed to AWS ec2 service.
The provider should respect the setting and provision encrypted volumes when requested.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1734193

….Encrypted field

AWS actuator allows to specify encrypted root volumes for compute machines.
Though, it does not wire it to EBS definition which is passed to AWS ec2 service.
The provider should respect the setting and provision encrypted volumes when requested.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1734193
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 5, 2019
@enxebre
Copy link
Member

enxebre commented Aug 5, 2019

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 5, 2019
@wking
Copy link
Member

wking commented Aug 5, 2019

nit: you can reference the bug from the PR subject to get links in release-image changelogs with:

/retitle Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field

@ingvagabund
Copy link
Member Author

/retitle Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field

@openshift-ci-robot openshift-ci-robot changed the title Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field Aug 5, 2019
@openshift-ci-robot
Copy link

@ingvagabund: This pull request references a valid Bugzilla bug. The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field

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 openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 5, 2019
Copy link

@michaelgugino michaelgugino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Probably should test this in e2e somehow, but I think we should be able to trust that the cloud-provider does the right thing on the other hand.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2019
@openshift-merge-robot openshift-merge-robot merged commit 73da00f into openshift:master Aug 5, 2019
@openshift-ci-robot
Copy link

@ingvagabund: All pull requests linked via external trackers have merged. The Bugzilla bug has been moved to the MODIFIED state.

Details

In response to this:

Bug 1734193: Wire provider spec EBS volume Encrypted field into ec2.EbsBlockDevice.Encrypted field

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.

@ingvagabund ingvagabund deleted the wire-ebs-encoded-field-to-ec2-ebs-one branch August 5, 2019 17:10
wking added a commit to wking/openshift-installer that referenced this pull request Aug 5, 2019
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.
@wking
Copy link
Member

wking commented Aug 5, 2019

Probably should test this in e2e somehow...

I've filed openshift/installer#2160 to use this approach for all machines except the bootstrap machine. We can ensure it's working just by looking for unencrypted root volumes in the CI account.

@enxebre
Copy link
Member

enxebre commented Aug 6, 2019

validation should be also included here https://github.com/openshift/cluster-api-provider-aws/blob/master/test/machines/machines_test.go#L209-L243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants