Merge pull request #3281 from bryan-cox/HOSTEDCP-1328

HOSTEDCP-1328: Add Ability to Encrypt VM OS Disks

api/hypershift/v1alpha1/nodepool_types.go

@@ -883,6 +883,9 @@ type AzureNodePoolPlatform struct {
 	// in a location that does not support AvailabilityZone.
 	// +optional
 	AvailabilityZone string `json:"availabilityZone,omitempty"`
+	// DiskEncryptionSetID is the ID of the DiskEncryptionSet resource to use to encrypt the OS disks for the VMs.
+	// +optional
+	DiskEncryptionSetID string `json:"diskEncryptionSetID,omitempty"`
 }
 
 // We define our own condition type since metav1.Condition has validation

api/hypershift/v1beta1/nodepool_types.go

@@ -876,6 +876,9 @@ type AzureNodePoolPlatform struct {
 	// in a location that does not support AvailabilityZone.
 	// +optional
 	AvailabilityZone string `json:"availabilityZone,omitempty"`
+	// DiskEncryptionSetID is the ID of the DiskEncryptionSet resource to use to encrypt the OS disks for the VMs.
+	// +optional
+	DiskEncryptionSetID string `json:"diskEncryptionSetID,omitempty"`
 }
 
 // We define our own condition type since metav1.Condition has validation

cmd/cluster/azure/create.go

@@ -5,9 +5,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
-	"os/signal"
 	"strings"
-	"syscall"
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 	"github.com/openshift/hypershift/cmd/cluster/core"
@@ -30,35 +28,37 @@ func NewCreateCommand(opts *core.CreateOptions) *cobra.Command {
 	opts.AzurePlatform.Location = "eastus"
 	opts.AzurePlatform.InstanceType = "Standard_D4s_v4"
 	opts.AzurePlatform.DiskSizeGB = 120
+
 	cmd.Flags().StringVar(&opts.AzurePlatform.CredentialsFile, "azure-creds", opts.AzurePlatform.CredentialsFile, "Path to an Azure credentials file (required)")
 	cmd.Flags().StringVar(&opts.AzurePlatform.Location, "location", opts.AzurePlatform.Location, "Location for the cluster")
 	cmd.Flags().StringVar(&opts.AzurePlatform.EncryptionKeyID, "encryption-key-id", opts.AzurePlatform.EncryptionKeyID, "etcd encryption key identifier in the form of https://<vaultName>.vault.azure.net/keys/<keyName>/<keyVersion>")
 	cmd.Flags().StringVar(&opts.AzurePlatform.InstanceType, "instance-type", opts.AzurePlatform.InstanceType, "The instance type to use for nodes")
 	cmd.Flags().Int32Var(&opts.AzurePlatform.DiskSizeGB, "root-disk-size", opts.AzurePlatform.DiskSizeGB, "The size of the root disk for machines in the NodePool (minimum 16)")
-	cmd.Flags().StringSliceVar(&opts.AzurePlatform.AvailabilityZones, "availablity-zones", opts.AzurePlatform.AvailabilityZones, "The availablity zones in which NodePools will be created. Must be left unspecified if the region does not support AZs. If set, one nodepool per zone will be created.")
+	cmd.Flags().StringSliceVar(&opts.AzurePlatform.AvailabilityZones, "availability-zones", opts.AzurePlatform.AvailabilityZones, "The availability zones in which NodePools will be created. Must be left unspecified if the region does not support AZs. If set, one nodepool per zone will be created.")
 	cmd.Flags().StringVar(&opts.AzurePlatform.ResourceGroupName, "resource-group-name", opts.AzurePlatform.ResourceGroupName, "A resource group name to create the HostedCluster infrastructure resources under.")
+	cmd.Flags().StringVar(&opts.AzurePlatform.DiskEncryptionSetID, "disk-encryption-set-id", opts.AzurePlatform.DiskEncryptionSetID, "The Disk Encryption Set ID to use to encrypt the OS disks for the VMs.")
 
 	_ = cmd.MarkFlagRequired("azure-creds")
 	_ = cmd.MarkPersistentFlagRequired("pull-secret")
 
-	cmd.Run = func(cmd *cobra.Command, args []string) {
-		ctx, cancel := context.WithCancel(context.Background())
+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
+		ctx := cmd.Context()
 		if opts.Timeout > 0 {
-			ctx, cancel = context.WithTimeout(context.Background(), opts.Timeout)
+			var cancel context.CancelFunc
+			ctx, cancel = context.WithTimeout(ctx, opts.Timeout)
+			defer cancel()
 		}
-		defer cancel()
 
-		sigs := make(chan os.Signal, 1)
-		signal.Notify(sigs, syscall.SIGINT)
-		go func() {
-			<-sigs
-			cancel()
-		}()
+		err := validate(opts)
+		if err != nil {
+			return err
+		}
 
-		if err := CreateCluster(ctx, opts); err != nil {
+		if err = CreateCluster(ctx, opts); err != nil {
 			opts.Log.Error(err, "Failed to create cluster")
-			os.Exit(1)
+			return err
 		}
+		return nil
 	}
 
 	return cmd
@@ -109,17 +109,18 @@ func applyPlatformSpecificsValues(ctx context.Context, exampleOptions *apifixtur
 	exampleOptions.InfraID = infra.InfraID
 	exampleOptions.ExternalDNSDomain = opts.ExternalDNSDomain
 	exampleOptions.Azure = &apifixtures.ExampleAzureOptions{
-		Location:          infra.Location,
-		ResourceGroupName: infra.ResourceGroupName,
-		VnetName:          infra.VnetName,
-		VnetID:            infra.VNetID,
-		SubnetName:        infra.SubnetName,
-		BootImageID:       infra.BootImageID,
-		MachineIdentityID: infra.MachineIdentityID,
-		InstanceType:      opts.AzurePlatform.InstanceType,
-		SecurityGroupName: infra.SecurityGroupName,
-		DiskSizeGB:        opts.AzurePlatform.DiskSizeGB,
-		AvailabilityZones: opts.AzurePlatform.AvailabilityZones,
+		Location:            infra.Location,
+		ResourceGroupName:   infra.ResourceGroupName,
+		VnetName:            infra.VnetName,
+		VnetID:              infra.VNetID,
+		SubnetName:          infra.SubnetName,
+		BootImageID:         infra.BootImageID,
+		MachineIdentityID:   infra.MachineIdentityID,
+		InstanceType:        opts.AzurePlatform.InstanceType,
+		SecurityGroupName:   infra.SecurityGroupName,
+		DiskSizeGB:          opts.AzurePlatform.DiskSizeGB,
+		AvailabilityZones:   opts.AzurePlatform.AvailabilityZones,
+		DiskEncryptionSetID: opts.AzurePlatform.DiskEncryptionSetID,
 	}
 
 	if opts.AzurePlatform.EncryptionKeyID != "" {
@@ -178,3 +179,12 @@ func lookupRHCOSImage(ctx context.Context, arch string, image string, pullSecret
 
 	return rhcosImage, nil
 }
+
+// validate validates the core create options passed in by the user
+func validate(opts *core.CreateOptions) error {
+	// Resource group name is required when using DiskEncryptionSetID
+	if opts.AzurePlatform.DiskEncryptionSetID != "" && opts.AzurePlatform.ResourceGroupName == "" {
+		return fmt.Errorf("validate: resource-group-name is required when using disk-encryption-set-id")
+	}
+	return nil
+}

cmd/cluster/core/create.go

@@ -162,17 +162,17 @@ type AWSPlatformOptions struct {
 }
 
 type AzurePlatformOptions struct {
-	CredentialsFile   string
-	Location          string
-	EncryptionKeyID   string
-	InstanceType      string
-	DiskSizeGB        int32
-	AvailabilityZones []string
-	ResourceGroupName string
+	CredentialsFile     string
+	Location            string
+	EncryptionKeyID     string
+	InstanceType        string
+	DiskSizeGB          int32
+	AvailabilityZones   []string
+	ResourceGroupName   string
+	DiskEncryptionSetID string
 }
 
 func createCommonFixture(ctx context.Context, opts *CreateOptions) (*apifixtures.ExampleOptions, error) {
-
 	// allow client side defaulting when release image is empty but release stream is set.
 	if len(opts.ReleaseImage) == 0 && len(opts.ReleaseStream) != 0 {
 		defaultVersion, err := version.LookupDefaultOCPVersion(opts.ReleaseStream)

cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml

@@ -484,6 +484,10 @@ spec:
                           specified for clusters in a location that does not support
                           AvailabilityZone.
                         type: string
+                      diskEncryptionSetID:
+                        description: DiskEncryptionSetID is the ID of the DiskEncryptionSet
+                          resource to use to encrypt the OS disks for the VMs.
+                        type: string
                       diskSizeGB:
                         default: 120
                         format: int32
@@ -1463,6 +1467,10 @@ spec:
                           specified for clusters in a location that does not support
                           AvailabilityZone.
                         type: string
+                      diskEncryptionSetID:
+                        description: DiskEncryptionSetID is the ID of the DiskEncryptionSet
+                          resource to use to encrypt the OS disks for the VMs.
+                        type: string
                       diskSizeGB:
                         default: 120
                         format: int32

cmd/nodepool/azure/create.go

@@ -2,65 +2,62 @@ package azure
 
 import (
 	"context"
-	"os"
-	"os/signal"
-	"syscall"
+	"fmt"
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
-	"github.com/openshift/hypershift/cmd/log"
 	"github.com/openshift/hypershift/cmd/nodepool/core"
+
 	"github.com/spf13/cobra"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+type AzurePlatformCreateOptions struct {
+	InstanceType        string
+	DiskSize            int32
+	AvailabilityZone    string
+	ResourceGroupName   string
+	DiskEncryptionSetID string
+}
+
 func NewCreateCommand(coreOpts *core.CreateNodePoolOptions) *cobra.Command {
+	platformOpts := &AzurePlatformCreateOptions{
+		InstanceType: "Standard_D4s_v4",
+		DiskSize:     120,
+	}
+
 	cmd := &cobra.Command{
 		Use:          "azure",
-		Short:        "Creates an Azure nodepool",
+		Short:        "Creates basic functional NodePool resources for Azure platform",
 		SilenceUsage: true,
 	}
-	o := &opts{
-		instanceType: "Standard_D4s_v4",
-		diskSize:     120,
-	}
-	cmd.Flags().StringVar(&o.instanceType, "instance-type", o.instanceType, "The instance type to use for the nodepool")
-	cmd.Flags().Int32Var(&o.diskSize, "root-disk-size", o.diskSize, "The size of the root disk for machines in the NodePool (minimum 16)")
-	cmd.Flags().StringVar(&o.availabilityZone, "availability-zone", o.availabilityZone, "The availabilityZone for the nodepool. Must be left unspecified if in a region that doesn't support AZs")
 
-	cmd.Run = func(cmd *cobra.Command, args []string) {
-		ctx, cancel := context.WithCancel(context.Background())
-		sigs := make(chan os.Signal, 1)
-		signal.Notify(sigs, syscall.SIGINT)
-		go func() {
-			<-sigs
-			cancel()
-		}()
+	cmd.Flags().StringVar(&platformOpts.InstanceType, "instance-type", platformOpts.InstanceType, "The instance type to use for the nodepool")
+	cmd.Flags().Int32Var(&platformOpts.DiskSize, "root-disk-size", platformOpts.DiskSize, "The size of the root disk for machines in the NodePool (minimum 16)")
+	cmd.Flags().StringVar(&platformOpts.AvailabilityZone, "availability-zone", platformOpts.AvailabilityZone, "The availabilityZone for the nodepool. Must be left unspecified if in a region that doesn't support AZs")
+	cmd.Flags().StringVar(&platformOpts.ResourceGroupName, "resource-group-name", platformOpts.ResourceGroupName, "A resource group name to create the HostedCluster infrastructure resources under.")
+	cmd.Flags().StringVar(&platformOpts.DiskEncryptionSetID, "disk-encryption-set-id", platformOpts.DiskEncryptionSetID, "The Disk Encryption Set ID to use to encrypt the OS disks for the VMs.")
 
-		if err := coreOpts.CreateNodePool(ctx, o); err != nil {
-			log.Log.Error(err, "Failed to create nodepool")
-			os.Exit(1)
-		}
-	}
+	cmd.RunE = coreOpts.CreateRunFunc(platformOpts)
 
 	return cmd
 }
 
-type opts struct {
-	instanceType     string
-	diskSize         int32
-	availabilityZone string
-}
+func (o *AzurePlatformCreateOptions) UpdateNodePool(_ context.Context, nodePool *hyperv1.NodePool, _ *hyperv1.HostedCluster, _ crclient.Client) error {
+	// Resource group name is required when using DiskEncryptionSetID
+	if o.DiskEncryptionSetID != "" && o.ResourceGroupName == "" {
+		return fmt.Errorf("UpdateNodePool: resource-group-name is required when using disk-encryption-set-id")
+	}
 
-func (o *opts) UpdateNodePool(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, client crclient.Client) error {
 	nodePool.Spec.Platform.Type = hyperv1.AzurePlatform
 	nodePool.Spec.Platform.Azure = &hyperv1.AzureNodePoolPlatform{
-		VMSize:           o.instanceType,
-		DiskSizeGB:       o.diskSize,
-		AvailabilityZone: o.availabilityZone,
+		VMSize:              o.InstanceType,
+		DiskSizeGB:          o.DiskSize,
+		AvailabilityZone:    o.AvailabilityZone,
+		DiskEncryptionSetID: o.DiskEncryptionSetID,
 	}
 	return nil
 }
 
-func (o opts) Type() hyperv1.PlatformType {
+func (o *AzurePlatformCreateOptions) Type() hyperv1.PlatformType {
 	return hyperv1.AzurePlatform
 }