-
Notifications
You must be signed in to change notification settings - Fork 516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MGMT-13627: Add ConfidentialVM options to AzureMachineProviderSpec #1403
MGMT-13627: Add ConfidentialVM options to AzureMachineProviderSpec #1403
Conversation
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
Hello @mresvanis! Some important instructions when contributing to openshift/api: For merging purposes, this repository follows the no-Feature-Freeze process which means that in addition to the standard
OR
Who should apply these qe/docs/px labels?
|
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
c8a6882
to
9de5dcf
Compare
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
e787b3c
to
e9a3595
Compare
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
/hold
|
e9a3595
to
e0f1ffd
Compare
Upstream issue: kubernetes-sigs/cluster-api-provider-azure#3264 |
/unhold |
e0f1ffd
to
ad7cb37
Compare
e89f31d
to
092a0cc
Compare
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
Hi @JoelSpeed and @soltysh, now that the respective upstream PR is merged I would very much appreciate your review and feedback for this PR. Many thanks. |
I will try to review next week. A quick scan suggests that the names of some of the fields might need to be changed, since they don't fit the conventions (they don't fit upstream conventions either, eg Could you please review our conventions and make the appropriate changes? The API need not match upstream, we would prefer it matches conventions downstream where possible, so long as the API can be converted to upstream's at a later point, that's ok |
092a0cc
to
3f4e0b1
Compare
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
@JoelSpeed I think I have covered the conformance to our API conventions, just a kind reminder for a review. Thanks in advance. |
325f265
to
0a96906
Compare
29db5be
to
dd9b827
Compare
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
@mresvanis: This pull request references MGMT-13627 which is a valid jira issue. 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. |
/test verify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you can update the required tags to match the correct format, there's one I highlighted and one a few lines below, after that, this LGTM
dd9b827
to
bf194f4
Compare
…ineProviderSpec This change introduces the security profile to the OS disk parameters and adds the SecurityType and UEFISettings sections to the VM's security profile. The SecurityType defines whether the VM is a Confidential or a Trusted Launch VM. This field should be set to one of the two options, in order to enable the UEFISettings section. The UEFISettings fields include the VirtualizedTrustedPlatformModule and SecureBoot fields, which can be set to either Enabled or Disabled. The OS disk security profile includes the SecurityEncryptionType and the DiskEncryptionSet fields. The SecurityEncryptionType can only be used when the VM is a Confidential one. When SecurityEncryptionType is set, vTPM should be enabled and the VM security profile's SecurityType should also be set to ConfidentialVM. Possible values for the SecurityEncryptionType are: - DiskWithVMGuestState, OS disk encryption before VM deployment that uses platform-managed keys (PMK) or a customer-managed key (CMK) - VMGuestStateOnly, no OS disk confidential encryption When the SecurityEncryptionType is set to DiskWithVMGuestState, the DiskEncryptionSet can be specified to allow for customer-managed keys to be used to encrypt the OS disk and the VMGuestState blob. Signed-off-by: Michail Resvanis <mresvani@redhat.com>
bf194f4
to
173c2ae
Compare
/retest-required |
/test verify |
/lgtm The verify failure is in code that's already in master, looks like maybe there's an ordering issue coming out somewhere, will need to be investigated |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, mresvanis 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 |
/test verify |
@mresvanis: all tests passed! Full PR test history. Your PR dashboard. 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. |
This change adds the Confidential VM configuration options to the
AzureMachineProviderSpec
, in order to support Confidential Computing on Azure.Specifically, it introduces the OS disk security profile, which includes the
SecurityEncryptionType
andDiskEncryptionSetID
fields and adds theSecureBoot
andVirtualizedTrustedPlatformModule
fields in the security profile of the VM.The OS disk security profile
SecurityEncryptionType
field defines whether the VM is a Confidential VM. It cannot be defined alongside theEncryptionAtHost
. WhenSecurityEncryptionType
is defined theVirtualizedTrustedPlatformModule
should be set toEnabled
, whileSecureBoot
can be eitherEnabled
orDisabled
.Possible values for the
SecurityEncryptionType
are:DiskWithVMGuestState
, OS disk encryption before VM deployment that uses platform-managed keys (PMK) or a customer-managed key (CMK)VMGuestStateOnly
, no OS disk confidential encryptionWhen the
SecurityEncryptionType
is set toDiskWithVMGuestState
, theDiskEncryptionSetID
can be specified to allow using customer-managed keys to encrypt the OS disk and the VMGuestState blob.Feature link: https://issues.redhat.com/browse/OCPBU-233
Cluster API provider Azure PR: kubernetes-sigs/cluster-api-provider-azure#3265
Related PRs for Confidential Compute support on GCP:
Signed-off-by: Michail Resvanis mresvani@redhat.com