GCP: populate encryption values into tf and machines #4397

staebler · 2020-11-19T20:37:00Z

Need to add this to the "bootstrap" module, too.

https://github.com/openshift/installer/pull/4397/files#diff-c9542c57acc94c4d250fd8d806ac9cf57c9ccd52c3c8e282a34527198949ff57R35

Is that not done as the change in data/data/gcp/bootstrap/main.tf, or does that need to go into data/data/bootstrap somewhere?

In /data/data/gcp/main.tf as linked above. It is currently only passing the variable from main to master, but needs to do the same for bootstrap.

patrickdillon · 2020-11-19T20:51:14Z

column alignment is off and causing tf-fmt to fail

column alignment is off and causing tf-fmt to fail

weird, tf-fmt seems to be suggesting different alignment for this file than the other two:
https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/4397/pull-ci-openshift-installer-master-tf-fmt/1329497726143959040/build-log.txt

michaelgugino · 2020-11-19T18:52:57Z

Is there a reason that OSDisk.EncryptionKey can't just be an imported type from the GCP provider? That should allow us to a do a straight assignment here.

Yeah. OSDisk.EncryptionKey is defined in the types package, which represents the installer API, so we try to keep it free of external dependencies considering other projects import the types package.

michaelgugino · 2020-11-19T18:53:31Z

Maybe this belongs in GCP provider in a util package?

michaelgugino · 2020-11-19T18:53:41Z

Maybe this belongs in GCP provider in a util package?

To echo our conversation on slack, I would handle this in terraform:

create a type for the key similar to Auth in TFVars and pass the values in that way

conditionally create the self link variable (or null value therein) with logic similar to BYO networking: https://github.com/openshift/installer/blob/master/data/data/gcp/network/common.tf

I'm also fine with this approach and think it's OK to have this logic live here.

-Original file line number
+Diff line change
@@ Expand Up / @@ -54,6 +54,7 @@ resource "google_compute_instance" "bootstrap" { @@
           size  = var.root_volume_size
           image = var.image
         }
+        kms_key_self_link = var.root_volume_kms_key_link
       }
       network_interface {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -59,6 +59,12 @@ variable "root_volume_type" { @@
       description = "The volume type for the bootstrap node's root volume."
     }
+    variable "root_volume_kms_key_link" {
+      type        = string
+      description = "The GCP self link of KMS key to encrypt the volume."
+      default     = null
+    }
     variable "zone" {
       type        = string
       description = "The zone for the bootstrap node."
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -30,8 +30,9 @@ module "bootstrap" { @@
       zone             = var.gcp_master_availability_zones[0]
       region           = var.gcp_region
-      root_volume_size = var.gcp_master_root_volume_size
-      root_volume_type = var.gcp_master_root_volume_type
+      root_volume_size         = var.gcp_master_root_volume_size
+      root_volume_type         = var.gcp_master_root_volume_type
+      root_volume_kms_key_link = var.gcp_root_volume_kms_key_link
       labels = local.labels
     }
@@ Expand All / @@ -47,8 +48,9 @@ module "master" { @@
       subnet         = module.network.master_subnet
       zones          = distinct(var.gcp_master_availability_zones)
-      root_volume_size = var.gcp_master_root_volume_size
-      root_volume_type = var.gcp_master_root_volume_type
+      root_volume_size         = var.gcp_master_root_volume_size
+      root_volume_type         = var.gcp_master_root_volume_type
+      root_volume_kms_key_link = var.gcp_root_volume_kms_key_link
       labels = local.labels
     }
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -41,6 +41,7 @@ resource "google_compute_instance" "master" { @@
           size  = var.root_volume_size
           image = var.image
         }
+        kms_key_self_link = var.root_volume_kms_key_link
       }
       network_interface {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -44,6 +44,12 @@ variable "root_volume_type" { @@
       description = "The type of volume for the root block device."
     }
+    variable "root_volume_kms_key_link" {
+      type        = string
+      description = "The GCP self link of KMS key to encrypt the volume."
+      default     = null
+    }
     variable "zones" {
       type = list
     }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GCP: populate encryption values into tf and machines #4397

Uh oh!

Diff view

Diff view

There are no files selected for viewing

staebler Nov 19, 2020

Uh oh!

JoelSpeed Nov 23, 2020

Uh oh!

patrickdillon Nov 23, 2020

Uh oh!

patrickdillon Nov 19, 2020

Uh oh!

patrickdillon Nov 19, 2020

Uh oh!

michaelgugino Nov 19, 2020

Uh oh!

patrickdillon Nov 19, 2020

Uh oh!

michaelgugino Nov 19, 2020

Uh oh!

michaelgugino Nov 19, 2020

Uh oh!

patrickdillon Nov 19, 2020

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up @@
     		return nil, err
     	}
+    	var encryptionKey *gcpprovider.GCPEncryptionKeyReference
+    	if mpool.OSDisk.EncryptionKey != nil {
+    		encryptionKey = &gcpprovider.GCPEncryptionKeyReference{
+    			KMSKey: &gcpprovider.GCPKMSKeyReference{
+    				Name:      mpool.OSDisk.EncryptionKey.KMSKey.Name,
+    				KeyRing:   mpool.OSDisk.EncryptionKey.KMSKey.KeyRing,
+    				ProjectID: mpool.OSDisk.EncryptionKey.KMSKey.ProjectID,
+    				Location:  mpool.OSDisk.EncryptionKey.KMSKey.Location,
+    			},
+    			KMSKeyServiceAccount: mpool.OSDisk.EncryptionKey.KMSKeyServiceAccount,
+    		}
+    	}
     	return &gcpprovider.GCPMachineProviderSpec{
     		TypeMeta: metav1.TypeMeta{
     			APIVersion: "gcpprovider.openshift.io/v1beta1",
@@ Expand All @@
     		UserDataSecret:    &corev1.LocalObjectReference{Name: userDataSecret},
     		CredentialsSecret: &corev1.LocalObjectReference{Name: "gcp-cloud-credentials"},
     		Disks: []*gcpprovider.GCPDisk{{
-    			AutoDelete: true,
-    			Boot:       true,
-    			SizeGb:     mpool.OSDisk.DiskSizeGB,
-    			Type:       mpool.OSDisk.DiskType,
-    			Image:      osImage,
+    			AutoDelete:    true,
+    			Boot:          true,
+    			SizeGb:        mpool.OSDisk.DiskSizeGB,
+    			Type:          mpool.OSDisk.DiskType,
+    			Image:         osImage,
+    			EncryptionKey: encryptionKey,
     		}},
     		NetworkInterfaces: []*gcpprovider.GCPNetworkInterface{{
     			Network:    network,
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,12 +2,17 @@ package gcp @@
     import (
     	"encoding/json"
+    	"fmt"
     	gcpprovider "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1"
     	"github.com/openshift/installer/pkg/types"
     )
+    const (
+    	kmsKeyNameFmt = "projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s"
+    )
     // Auth is the collection of credentials that will be used by terrform.
     type Auth struct {
     	ProjectID      string `json:"gcp_project_id,omitempty"`
@@ Expand All / @@ -26,6 +31,7 @@ type config struct { @@
     	ImageLicenses           []string `json:"gcp_image_licenses,omitempty"`
     	VolumeType              string   `json:"gcp_master_root_volume_type"`
     	VolumeSize              int64    `json:"gcp_master_root_volume_size"`
+    	VolumeKMSKeyLink        string   `json:"gcp_root_volume_kms_key_link"`
     	PublicZoneName          string   `json:"gcp_public_dns_zone_name,omitempty"`
     	PublishStrategy         string   `json:"gcp_publish_strategy,omitempty"`
     	PreexistingNetwork      bool     `json:"gcp_preexisting_network,omitempty"`
@@ Expand Down Expand Up / @@ -78,5 +84,17 @@ func TFVars(sources TFVarsSources) ([]byte, error) { @@
     		cfg.PreexistingImage = false
     	}
+    	if masterConfig.Disks[0].EncryptionKey != nil {
+    		cfg.VolumeKMSKeyLink = generateDiskEncryptionKeyLink(masterConfig.Disks[0].EncryptionKey, masterConfig.ProjectID)
+    	}
     	return json.MarshalIndent(cfg, "", "  ")
     }
+    func generateDiskEncryptionKeyLink(keyRef *gcpprovider.GCPEncryptionKeyReference, projectID string) string {
+    	if keyRef.KMSKey.ProjectID != "" {
+    		projectID = keyRef.KMSKey.ProjectID
+    	}
+    	return fmt.Sprintf(kmsKeyNameFmt, projectID, keyRef.KMSKey.Location, keyRef.KMSKey.KeyRing, keyRef.KMSKey.Name)
+    }

GCP: populate encryption values into tf and machines #4397

Uh oh!

GCP: populate encryption values into tf and machines #4397

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!