Merge pull request #1053 from yastij/thumbprint-support

k8s-ci-robot · web-flow · commit c18ca883bca8 · 2020-12-02T07:04:50.000-08:00
set the thumbprint to enable secure communications
diff --git a/api/v1alpha2/vspherecluster_conversion.go b/api/v1alpha2/vspherecluster_conversion.go
@@ -58,6 +58,9 @@ func (src *VSphereCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint
 	if restored.Spec.ControlPlaneEndpoint.Port != 0 {
 		dst.Spec.ControlPlaneEndpoint.Port = restored.Spec.ControlPlaneEndpoint.Port
 	}
+	if restored.Spec.Thumbprint != "" {
+		dst.Spec.Thumbprint = restored.Spec.Thumbprint
+	}
 
 	dst.Status.Conditions = restored.Status.Conditions
 
diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go
diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -78,6 +78,13 @@ type VirtualMachineCloneSpec struct {
 	// +optional
 	Server string `json:"server,omitempty"`
 
+	// Thumbprint is the colon-separated SHA-1 checksum of the given vCenter server's host certificate
+	// When this is set to empty, this VirtualMachine would be created
+	// without TLS certificate validation of the communication between Cluster API Provider vSphere
+	// and the VMware vCenter server.
+	// +optional
+	Thumbprint string `json:"thumbprint,omitempty"`
+
 	// Datacenter is the name or inventory path of the datacenter in which the
 	// virtual machine is created/located.
 	// +optional
diff --git a/api/v1alpha3/vspherecluster_types.go b/api/v1alpha3/vspherecluster_types.go
@@ -39,6 +39,11 @@ type VSphereClusterSpec struct {
 	// +optional
 	Insecure *bool `json:"insecure,omitempty"`
 
+	// Thumbprint is the colon-separated SHA-1 checksum of the given vCenter server's host certificate
+	// When provided, Insecure should not be set to true
+	// +optional
+	Thumbprint string `json:"thumbprint,omitempty"`
+
 	// CloudProviderConfiguration holds the cluster-wide configuration for the
 	// vSphere cloud provider.
 	CloudProviderConfiguration CPIConfig `json:"cloudProviderConfiguration,omitempty"`
diff --git a/api/v1alpha3/vspherecluster_webhook.go b/api/v1alpha3/vspherecluster_webhook.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1alpha3
 
 import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
@@ -25,3 +27,27 @@ func (r *VSphereCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 		For(r).
 		Complete()
 }
+
+// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha3-vspherecluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vsphereclusters,versions=v1alpha3,name=validation.vspherecluster.infrastructure.x-k8s.io,sideEffects=None
+
+// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
+func (r *VSphereCluster) ValidateCreate() error {
+	var allErrs field.ErrorList
+	spec := r.Spec
+
+	if spec.Thumbprint != "" && spec.Insecure != nil && *spec.Insecure {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "Insecure"), spec.Insecure, "cannot be set to true at the same time as .spec.Thumbprint"))
+	}
+
+	return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
+}
+
+// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
+func (r *VSphereCluster) ValidateUpdate(old runtime.Object) error {
+	return nil
+}
+
+// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
+func (r *VSphereCluster) ValidateDelete() error {
+	return nil
+}
diff --git a/api/v1alpha3/vspherecluster_webhook_test.go b/api/v1alpha3/vspherecluster_webhook_test.go
@@ -0,0 +1,71 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha3
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+//nolint
+func TestVSphereCluster_ValidateCreate(t *testing.T) {
+
+	g := NewWithT(t)
+	tests := []struct {
+		name           string
+		vsphereCluster *VSphereCluster
+		wantErr        bool
+	}{
+		{
+			name:           "insecure true with empty thumbprint",
+			vsphereCluster: createVSphereCluster("foo.com", true, ""),
+			wantErr:        false,
+		},
+		{
+			name:           "insecure false with non-empty thumbprint",
+			vsphereCluster: createVSphereCluster("foo.com", false, "thumprint:foo"),
+			wantErr:        false,
+		},
+		{
+			name:           "insecure true with non-empty thumbprint",
+			vsphereCluster: createVSphereCluster("foo.com", true, "thumprint:foo"),
+			wantErr:        true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.vsphereCluster.ValidateCreate()
+			if tc.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+		})
+	}
+}
+
+func createVSphereCluster(server string, insecure bool, thumbprint string) *VSphereCluster {
+	vsphereCluster := &VSphereCluster{
+		Spec: VSphereClusterSpec{
+			Server:     server,
+			Insecure:   &insecure,
+			Thumbprint: thumbprint,
+		},
+	}
+	return vsphereCluster
+}
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_haproxyloadbalancers.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_haproxyloadbalancers.yaml
@@ -255,6 +255,13 @@ spec:
                       used to clone the virtual machine.
                     minLength: 1
                     type: string
+                  thumbprint:
+                    description: Thumbprint is the colon-separated SHA-1 checksum
+                      of the given vCenter server's host certificate When this is
+                      set to empty, this VirtualMachine would be created without TLS
+                      certificate validation of the communication between Cluster
+                      API Provider vSphere and the VMware vCenter server.
+                    type: string
                 required:
                 - network
                 - template
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml
@@ -527,6 +527,11 @@ spec:
               server:
                 description: Server is the address of the vSphere endpoint.
                 type: string
+              thumbprint:
+                description: Thumbprint is the colon-separated SHA-1 checksum of the
+                  given vCenter server's host certificate When provided, Insecure
+                  should not be set to true
+                type: string
             type: object
           status:
             description: VSphereClusterStatus defines the observed state of VSphereClusterSpec
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachines.yaml
@@ -519,6 +519,13 @@ spec:
                   used to clone the virtual machine.
                 minLength: 1
                 type: string
+              thumbprint:
+                description: Thumbprint is the colon-separated SHA-1 checksum of the
+                  given vCenter server's host certificate When this is set to empty,
+                  this VirtualMachine would be created without TLS certificate validation
+                  of the communication between Cluster API Provider vSphere and the
+                  VMware vCenter server.
+                type: string
             required:
             - network
             - template
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml
@@ -579,6 +579,13 @@ spec:
                           template used to clone the virtual machine.
                         minLength: 1
                         type: string
+                      thumbprint:
+                        description: Thumbprint is the colon-separated SHA-1 checksum
+                          of the given vCenter server's host certificate When this
+                          is set to empty, this VirtualMachine would be created without
+                          TLS certificate validation of the communication between
+                          Cluster API Provider vSphere and the VMware vCenter server.
+                        type: string
                     required:
                     - network
                     - template
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_vspherevms.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_vspherevms.yaml
@@ -277,6 +277,13 @@ spec:
                   used to clone the virtual machine.
                 minLength: 1
                 type: string
+              thumbprint:
+                description: Thumbprint is the colon-separated SHA-1 checksum of the
+                  given vCenter server's host certificate When this is set to empty,
+                  this VirtualMachine would be created without TLS certificate validation
+                  of the communication between Cluster API Provider vSphere and the
+                  VMware vCenter server.
+                type: string
             required:
             - network
             - template
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -6,6 +6,26 @@ metadata:
   creationTimestamp: null
   name: validating-webhook-configuration
 webhooks:
+- clientConfig:
+    caBundle: Cg==
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-vspherecluster
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.vspherecluster.infrastructure.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1alpha3
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - vsphereclusters
+  sideEffects: None
 - clientConfig:
     caBundle: Cg==
     service:
diff --git a/controllers/vspheremachine_controller.go b/controllers/vspheremachine_controller.go
@@ -448,6 +448,9 @@ func (r machineReconciler) reconcileNormalPre7(ctx *context.MachineContext, vsph
 				vm.Spec.Server = ctx.VSphereCluster.Spec.Server
 			}
 		}
+		if vm.Spec.Thumbprint == "" {
+			vm.Spec.Thumbprint = ctx.VSphereCluster.Spec.Thumbprint
+		}
 		if vm.Spec.Datacenter == "" {
 			vm.Spec.Datacenter = vsphereCloudConfig.Datacenter
 		}
diff --git a/controllers/vspherevm_controller.go b/controllers/vspherevm_controller.go
@@ -139,7 +139,7 @@ func (r vmReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr error)
 	// Get or create an authenticated session to the vSphere endpoint.
 	authSession, err := session.GetOrCreate(r.Context,
 		vsphereVM.Spec.Server, vsphereVM.Spec.Datacenter,
-		r.ControllerManagerContext.Username, r.ControllerManagerContext.Password)
+		r.ControllerManagerContext.Username, r.ControllerManagerContext.Password, vsphereVM.Spec.Thumbprint)
 	if err != nil {
 		return reconcile.Result{}, errors.Wrap(err, "failed to create vSphere session")
 	}
diff --git a/packaging/flavorgen/flavors/generators.go b/packaging/flavorgen/flavors/generators.go
@@ -44,6 +44,7 @@ const (
 	machineDeploymentNameSuffix  = "-md-0"
 	namespaceVar                 = "${NAMESPACE}"
 	vSphereDataCenterVar         = "${VSPHERE_DATACENTER}"
+	vSphereThumbprint            = "${VSPHERE_TLS_THUMBPRINT}"
 	vSphereDatastoreVar          = "${VSPHERE_DATASTORE}"
 	vSphereFolderVar             = "${VSPHERE_FOLDER}"
 	vSphereHaproxyTemplateVar    = "${VSPHERE_HAPROXY_TEMPLATE}"
@@ -121,15 +122,19 @@ func newVSphereCluster(lb *infrav1.HAProxyLoadBalancer) infrav1.VSphereCluster {
 			Namespace: namespaceVar,
 		},
 		Spec: infrav1.VSphereClusterSpec{
-			Server: vSphereServerVar,
+			Server:     vSphereServerVar,
+			Thumbprint: vSphereThumbprint,
 			CloudProviderConfiguration: infrav1.CPIConfig{
 				Global: infrav1.CPIGlobalConfig{
 					SecretName:      "cloud-provider-vsphere-credentials",
 					SecretNamespace: metav1.NamespaceSystem,
-					Insecure:        true,
+					Thumbprint:      vSphereThumbprint,
 				},
 				VCenter: map[string]infrav1.CPIVCenterConfig{
-					vSphereServerVar: {Datacenters: vSphereDataCenterVar},
+					vSphereServerVar: {
+						Datacenters: vSphereDataCenterVar,
+						Thumbprint:  vSphereThumbprint,
+					},
 				},
 				Network: infrav1.CPINetworkConfig{
 					Name: vSphereNetworkVar,
@@ -254,6 +259,7 @@ func defaultVirtualMachineCloneSpec() infrav1.VirtualMachineCloneSpec {
 		MemoryMiB:     defaultMemoryMiB,
 		Template:      vSphereTemplateVar,
 		Server:        vSphereServerVar,
+		Thumbprint:    vSphereThumbprint,
 		ResourcePool:  vSphereResourcePoolVar,
 		Datastore:     vSphereDatastoreVar,
 		Folder:        vSphereFolderVar,
diff --git a/pkg/services/govmomi/create_test.go b/pkg/services/govmomi/create_test.go
@@ -49,7 +49,7 @@ func TestCreate(t *testing.T) {
 	authSession, err := session.GetOrCreate(
 		vmContext,
 		vmContext.VSphereVM.Spec.Server, "",
-		s.URL.User.Username(), pass)
+		s.URL.User.Username(), pass, "")
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/services/govmomi/vcenter/clone_test.go b/pkg/services/govmomi/vcenter/clone_test.go
@@ -144,7 +144,7 @@ func initSimulator(t *testing.T) (*simulator.Model, *session.Session, *simulator
 	authSession, err := session.GetOrCreate(
 		ctx.TODO(),
 		server.URL.Host, "",
-		server.URL.User.Username(), pass)
+		server.URL.User.Username(), pass, "")
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/session/session.go b/pkg/session/session.go
@@ -25,6 +25,8 @@ import (
 	"github.com/vmware/govmomi"
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/session"
+	"github.com/vmware/govmomi/vim25"
 	"github.com/vmware/govmomi/vim25/soap"
 
 	"sigs.k8s.io/cluster-api-provider-vsphere/api/v1alpha3"
@@ -44,7 +46,7 @@ type Session struct {
 // already exist.
 func GetOrCreate(
 	ctx context.Context,
-	server, datacenter, username, password string) (*Session, error) {
+	server, datacenter, username, password string, thumbprint string) (*Session, error) {
 
 	sessionMU.Lock()
 	defer sessionMU.Unlock()
@@ -65,12 +67,9 @@ func GetOrCreate(
 	}
 
 	soapURL.User = url.UserPassword(username, password)
-
-	// Temporarily setting the insecure flag True
-	// TODO(ssurana): handle the certs better
-	client, err := govmomi.NewClient(ctx, soapURL, true)
+	client, err := newClient(ctx, soapURL, thumbprint)
 	if err != nil {
-		return nil, errors.Wrapf(err, "error setting up new vSphere SOAP client")
+		return nil, err
 	}
 
 	session := Session{Client: client}
@@ -96,6 +95,28 @@ func GetOrCreate(
 	return &session, nil
 }
 
+func newClient(ctx context.Context, url *url.URL, thumprint string) (*govmomi.Client, error) {
+	insecure := thumprint == ""
+	soapClient := soap.NewClient(url, insecure)
+	if !insecure {
+		soapClient.SetThumbprint(url.Host, thumprint)
+	}
+
+	vimClient, err := vim25.NewClient(ctx, soapClient)
+	if err != nil {
+		return nil, err
+	}
+	c := &govmomi.Client{
+		Client:         vimClient,
+		SessionManager: session.NewManager(vimClient),
+	}
+	if err := c.Login(ctx, url.User); err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}
+
 // FindByBIOSUUID finds an object by its BIOS UUID.
 //
 // To avoid comments about this function's name, please see the Golang
diff --git a/test/e2e/config/vsphere-ci.yaml b/test/e2e/config/vsphere-ci.yaml
@@ -86,6 +86,7 @@ variables:
   EXP_CLUSTER_RESOURCE_SET: "true"
   CONTROL_PLANE_MACHINE_COUNT: 1
   WORKER_MACHINE_COUNT: 1
+  VSPHERE_TLS_THUMBPRINT: "4F:AF:22:BD:C8:B2:AC:DE:0A:04:71:02:D1:62:05:50:2B:A2:54:E9"
   VSPHERE_DATACENTER:  "SDDC-Datacenter"
   VSPHERE_FOLDER: "clusterapi"
   VSPHERE_RESOURCE_POOL: "clusterapi"

Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,9 @@ func (src *VSphereCluster) ConvertTo(dstRaw conversion.Hub) error { // nolint`
`58`	`58`	`if restored.Spec.ControlPlaneEndpoint.Port != 0 {`
`59`	`59`	`dst.Spec.ControlPlaneEndpoint.Port = restored.Spec.ControlPlaneEndpoint.Port`
`60`	`60`	`}`
	`61`	`+ if restored.Spec.Thumbprint != "" {`
	`62`	`+ dst.Spec.Thumbprint = restored.Spec.Thumbprint`
	`63`	`+ }`
`61`	`64`
`62`	`65`	`dst.Status.Conditions = restored.Status.Conditions`
`63`	`66`