Merge pull request kubevirt#8418 from alromeros/make-export-secret-op…

…tional

Enable automatic token generation for VirtualMachineExport objects

api/openapi-spec/swagger.json

@@ -20131,15 +20131,14 @@
     "description": "VirtualMachineExportSpec is the spec for a VirtualMachineExport resource",
     "type": "object",
     "required": [
-     "source",
-     "tokenSecretRef"
+     "source"
     ],
     "properties": {
      "source": {
       "$ref": "#/definitions/k8s.io.api.core.v1.TypedLocalObjectReference"
      },
      "tokenSecretRef": {
-      "description": "TokenSecretRef is the name of the secret that contains the token used by the export server pod",
+      "description": "TokenSecretRef is the name of the custom-defined secret that contains the token used by the export server pod",
       "type": "string"
      }
     }
@@ -20165,6 +20164,10 @@
      "serviceName": {
       "description": "ServiceName is the name of the service created associated with the Virtual Machine export. It will be used to create the internal URLs for downloading the images",
       "type": "string"
+     },
+     "tokenSecretRef": {
+      "description": "TokenSecretRef is the name of the secret that contains the token used by the export server pod",
+      "type": "string"
      }
     }
    },

pkg/storage/export/export/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//pkg/controller:go_default_library",
         "//pkg/storage/snapshot:go_default_library",
         "//pkg/storage/types:go_default_library",
+        "//pkg/util:go_default_library",
         "//pkg/virt-config:go_default_library",
         "//pkg/virt-controller/services:go_default_library",
         "//pkg/virt-controller/watch/util:go_default_library",

pkg/storage/export/export/export.go

@@ -55,6 +55,7 @@ import (
 	"kubevirt.io/kubevirt/pkg/controller"
 	"kubevirt.io/kubevirt/pkg/storage/snapshot"
 	"kubevirt.io/kubevirt/pkg/storage/types"
+	kutil "kubevirt.io/kubevirt/pkg/util"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 	"kubevirt.io/kubevirt/pkg/virt-controller/services"
 	watchutil "kubevirt.io/kubevirt/pkg/virt-controller/watch/util"
@@ -105,6 +106,11 @@ const (
 
 	kvm = 107
 
+	// secretTokenLength is the lenght of the randomly generated token
+	secretTokenLength = 20
+	// secretTokenKey is the entry used to store the token in the virtualMachineExport secret
+	secretTokenKey = "token"
+
 	requeueTime = time.Second * 3
 )
 
@@ -420,6 +426,10 @@ func (ctrl *VMExportController) updateVMExport(vmExport *exportv1.VirtualMachine
 		return 0, err
 	}
 
+	if err := ctrl.handleVMExportToken(vmExport); err != nil {
+		return 0, err
+	}
+
 	if ctrl.isSourcePvc(&vmExport.Spec) {
 		return ctrl.handleSource(vmExport, service, ctrl.getPVCFromSourcePVC, ctrl.updateVMExportPvcStatus)
 	}
@@ -599,6 +609,65 @@ func (ctrl *VMExportController) createCertSecretManifest(vmExport *exportv1.Virt
 	}, nil
 }
 
+// handleVMExportToken checks if a secret has been specified for the current export object and, if not, creates one specific to it
+func (ctrl *VMExportController) handleVMExportToken(vmExport *exportv1.VirtualMachineExport) error {
+	if vmExport.Status == nil {
+		vmExport.Status = &exportv1.VirtualMachineExportStatus{
+			Phase: exportv1.Pending,
+			Conditions: []exportv1.Condition{
+				newReadyCondition(corev1.ConditionFalse, initializingReason, ""),
+				newPvcCondition(corev1.ConditionFalse, unknownReason, ""),
+			},
+		}
+	}
+
+	// If a tokenSecretRef has been specified, we assume that the corresponding
+	// secret has already been created and managed appropiately by the user
+	if vmExport.Spec.TokenSecretRef != nil {
+		vmExport.Status.TokenSecretRef = vmExport.Spec.TokenSecretRef
+		return nil
+	}
+
+	// If not, the secret name is constructed so it can be specific to the current vmExport object
+	if vmExport.Status.TokenSecretRef == nil {
+		generatedSecretName := getDefaultTokenSecretName(vmExport)
+		vmExport.Status.TokenSecretRef = &generatedSecretName
+	}
+
+	token, err := kutil.GenerateSecureRandomString(secretTokenLength)
+	if err != nil {
+		return err
+	}
+
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      *vmExport.Status.TokenSecretRef,
+			Namespace: vmExport.Namespace,
+			OwnerReferences: []metav1.OwnerReference{
+				*metav1.NewControllerRef(vmExport, schema.GroupVersionKind{
+					Group:   exportv1.SchemeGroupVersion.Group,
+					Version: exportv1.SchemeGroupVersion.Version,
+					Kind:    "VirtualMachineExport",
+				}),
+			},
+		},
+		Type: corev1.SecretTypeOpaque,
+		Data: map[string][]byte{
+			secretTokenKey: []byte(token),
+		},
+	}
+
+	secret, err = ctrl.Client.CoreV1().Secrets(vmExport.Namespace).Create(context.Background(), secret, metav1.CreateOptions{})
+	if err != nil {
+		if errors.IsAlreadyExists(err) {
+			return nil
+		}
+		return err
+	}
+	ctrl.Recorder.Eventf(vmExport, corev1.EventTypeNormal, secretCreatedEvent, "Created default secret %s/%s", secret.Namespace, secret.Name)
+	return nil
+}
+
 func (ctrl *VMExportController) getExportSecretName(ownerPod *corev1.Pod) string {
 	var certSecretName string
 	for _, volume := range ownerPod.Spec.Volumes {
@@ -609,6 +678,11 @@ func (ctrl *VMExportController) getExportSecretName(ownerPod *corev1.Pod) string
 	return certSecretName
 }
 
+// getDefaultTokenSecretName returns a secret name specifically created for the current export object
+func getDefaultTokenSecretName(vme *exportv1.VirtualMachineExport) string {
+	return naming.GetName("export-token", vme.Name, validation.DNS1035LabelMaxLength)
+}
+
 func (ctrl *VMExportController) getExportServiceName(vmExport *exportv1.VirtualMachineExport) string {
 	return naming.GetName(exportPrefix, vmExport.Name, validation.DNS1035LabelMaxLength)
 }
@@ -696,9 +770,10 @@ func (ctrl *VMExportController) createExporterPod(vmExport *exportv1.VirtualMach
 
 		log.Log.V(3).Infof("Creating new exporter pod %s/%s", manifest.Namespace, manifest.Name)
 		pod, err := ctrl.Client.CoreV1().Pods(vmExport.Namespace).Create(context.Background(), manifest, metav1.CreateOptions{})
-		if err == nil {
-			ctrl.Recorder.Eventf(vmExport, corev1.EventTypeNormal, exporterPodCreatedEvent, "Created exporter pod %s/%s", manifest.Namespace, manifest.Name)
+		if err != nil {
+			return nil, err
 		}
+		ctrl.Recorder.Eventf(vmExport, corev1.EventTypeNormal, exporterPodCreatedEvent, "Created exporter pod %s/%s", manifest.Namespace, manifest.Name)
 		return pod, nil
 	} else {
 		pod := obj.(*corev1.Pod)
@@ -767,6 +842,11 @@ func (ctrl *VMExportController) createExporterPodManifest(vmExport *exportv1.Vir
 		Value: currentTime().Add(deadline).Format(time.RFC3339),
 	})
 
+	tokenSecretRef := ""
+	if vmExport.Status != nil && vmExport.Status.TokenSecretRef != nil {
+		tokenSecretRef = *vmExport.Status.TokenSecretRef
+	}
+
 	secretName := fmt.Sprintf("secret-%s", rand.String(10))
 	podManifest.Spec.Volumes = append(podManifest.Spec.Volumes, corev1.Volume{
 		Name: certificates,
@@ -776,10 +856,10 @@ func (ctrl *VMExportController) createExporterPodManifest(vmExport *exportv1.Vir
 			},
 		},
 	}, corev1.Volume{
-		Name: vmExport.Spec.TokenSecretRef,
+		Name: tokenSecretRef,
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
-				SecretName: vmExport.Spec.TokenSecretRef,
+				SecretName: tokenSecretRef,
 			},
 		},
 	})
@@ -788,7 +868,7 @@ func (ctrl *VMExportController) createExporterPodManifest(vmExport *exportv1.Vir
 		Name:      certificates,
 		MountPath: "/cert",
 	}, corev1.VolumeMount{
-		Name:      vmExport.Spec.TokenSecretRef,
+		Name:      tokenSecretRef,
 		MountPath: "/token",
 	})
 	return podManifest, nil
@@ -857,15 +937,6 @@ func (ctrl *VMExportController) isKubevirtContentType(pvc *corev1.PersistentVolu
 
 func (ctrl *VMExportController) updateCommonVMExportStatusFields(vmExport, vmExportCopy *exportv1.VirtualMachineExport, exporterPod *corev1.Pod, service *corev1.Service, sourceVolumes *sourceVolumes) error {
 	var err error
-	if vmExportCopy.Status == nil {
-		vmExportCopy.Status = &exportv1.VirtualMachineExportStatus{
-			Phase: exportv1.Pending,
-			Conditions: []exportv1.Condition{
-				newReadyCondition(corev1.ConditionFalse, initializingReason, ""),
-				newPvcCondition(corev1.ConditionFalse, unknownReason, ""),
-			},
-		}
-	}
 
 	vmExportCopy.Status.ServiceName = service.Name
 	vmExportCopy.Status.Links = &exportv1.VirtualMachineExportLinks{}

pkg/storage/export/export/export_test.go

@@ -655,6 +655,10 @@ var _ = Describe("Export controller", func() {
 			},
 		}
 		testVMExport := createPVCVMExport()
+		// We call handleVMExportToken to populate the Status field appropiately
+		err := controller.handleVMExportToken(testVMExport)
+		Expect(testVMExport.Status.TokenSecretRef).ToNot(BeNil())
+		Expect(err).ToNot(HaveOccurred())
 		k8sClient.Fake.PrependReactor("create", "pods", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
 			create, ok := action.(testing.CreateAction)
 			Expect(ok).To(BeTrue())
@@ -711,7 +715,7 @@ var _ = Describe("Export controller", func() {
 			MountPath: "/cert",
 		}))
 		Expect(pod.Spec.Containers[0].VolumeMounts).To(ContainElement(k8sv1.VolumeMount{
-			Name:      testVMExport.Spec.TokenSecretRef,
+			Name:      *testVMExport.Status.TokenSecretRef,
 			MountPath: "/token",
 		}))
 		Expect(pod.Annotations[annCertParams]).To(Equal("{\"Duration\":7200000000000,\"RenewBefore\":3600000000000}"))
@@ -722,6 +726,9 @@ var _ = Describe("Export controller", func() {
 		scp, err := serializeCertParams(cp)
 		Expect(err).ToNot(HaveOccurred())
 		testVMExport := createPVCVMExport()
+		// We call handleVMExportToken to populate the Status field appropiately
+		err = controller.handleVMExportToken(testVMExport)
+		Expect(err).ToNot(HaveOccurred())
 		testExportPod := &k8sv1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "test-export-pod",
@@ -779,6 +786,59 @@ var _ = Describe("Export controller", func() {
 		Expect(err).To(HaveOccurred())
 	})
 
+	It("handleVMExportToken should create the export secret if no TokenSecretRef is specified", func() {
+		testVMExport := createPVCVMExportWithoutSecret()
+		expectedName := getDefaultTokenSecretName(testVMExport)
+		k8sClient.Fake.PrependReactor("create", "secrets", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
+			create, ok := action.(testing.CreateAction)
+			Expect(ok).To(BeTrue())
+			secret, ok := create.GetObject().(*k8sv1.Secret)
+			Expect(ok).To(BeTrue())
+			Expect(secret.GetName()).To(Equal(expectedName))
+			Expect(secret.GetNamespace()).To(Equal(testNamespace))
+			return true, secret, nil
+		})
+		err := controller.handleVMExportToken(testVMExport)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(testVMExport.Status.TokenSecretRef).ToNot(BeNil())
+		Expect(*testVMExport.Status.TokenSecretRef).To(Equal(expectedName))
+		testutils.ExpectEvent(recorder, secretCreatedEvent)
+	})
+
+	It("handleVMExportToken should use the already specified secret if the status is already populated", func() {
+		testVMExport := createPVCVMExportWithoutSecret()
+		oldSecretRef := "oldToken"
+		newSecretRef := getDefaultTokenSecretName(testVMExport)
+		testVMExport.Status = &exportv1.VirtualMachineExportStatus{
+			TokenSecretRef: &oldSecretRef,
+		}
+		k8sClient.Fake.PrependReactor("create", "secrets", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
+			create, ok := action.(testing.CreateAction)
+			Expect(ok).To(BeTrue())
+			secret, ok := create.GetObject().(*k8sv1.Secret)
+			Expect(ok).To(BeTrue())
+			Expect(secret.GetName()).To(Equal(oldSecretRef))
+			Expect(secret.GetNamespace()).To(Equal(testNamespace))
+			return true, secret, nil
+		})
+		err := controller.handleVMExportToken(testVMExport)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(testVMExport.Status.TokenSecretRef).ToNot(BeNil())
+		Expect(*testVMExport.Status.TokenSecretRef).ToNot(Equal(newSecretRef))
+		Expect(*testVMExport.Status.TokenSecretRef).To(Equal(oldSecretRef))
+		testutils.ExpectEvent(recorder, secretCreatedEvent)
+	})
+
+	It("handleVMExportToken should use the user-specified secret if TokenSecretRef is specified", func() {
+		testVMExport := createPVCVMExport()
+		Expect(testVMExport.Spec.TokenSecretRef).ToNot(BeNil())
+		expectedName := *testVMExport.Spec.TokenSecretRef
+		err := controller.handleVMExportToken(testVMExport)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(testVMExport.Status.TokenSecretRef).ToNot(BeNil())
+		Expect(*testVMExport.Status.TokenSecretRef).To(Equal(expectedName))
+	})
+
 	DescribeTable("Should ignore invalid VMExports kind/api combinations", func(kind, apigroup string) {
 		testVMExport := createPVCVMExport()
 		testVMExport.Spec.Source.Kind = kind
@@ -953,7 +1013,23 @@ func createPVCVMExport() *exportv1.VirtualMachineExport {
 				Kind:     "PersistentVolumeClaim",
 				Name:     testPVCName,
 			},
-			TokenSecretRef: "token",
+			TokenSecretRef: pointer.StringPtr("token"),
+		},
+	}
+}
+
+func createPVCVMExportWithoutSecret() *exportv1.VirtualMachineExport {
+	return &exportv1.VirtualMachineExport{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-no-secret",
+			Namespace: testNamespace,
+		},
+		Spec: exportv1.VirtualMachineExportSpec{
+			Source: k8sv1.TypedLocalObjectReference{
+				APIGroup: &k8sv1.SchemeGroupVersion.Group,
+				Kind:     "PersistentVolumeClaim",
+				Name:     testPVCName,
+			},
 		},
 	}
 }
@@ -971,7 +1047,7 @@ func createSnapshotVMExport() *exportv1.VirtualMachineExport {
 				Kind:     "VirtualMachineSnapshot",
 				Name:     testVmsnapshotName,
 			},
-			TokenSecretRef: "token",
+			TokenSecretRef: pointer.StringPtr("token"),
 		},
 	}
 }
@@ -989,7 +1065,7 @@ func createVMVMExport() *exportv1.VirtualMachineExport {
 				Kind:     "VirtualMachine",
 				Name:     testVmName,
 			},
-			TokenSecretRef: "token",
+			TokenSecretRef: pointer.StringPtr("token"),
 		},
 	}
 }

pkg/virt-operator/resource/generate/components/validations_generated.go

@@ -7738,12 +7738,11 @@ var CRDsValidation map[string]string = map[string]string{
           - name
           type: object
         tokenSecretRef:
-          description: TokenSecretRef is the name of the secret that contains the
-            token used by the export server pod
+          description: TokenSecretRef is the name of the custom-defined secret that
+            contains the token used by the export server pod
           type: string
       required:
       - source
-      - tokenSecretRef
       type: object
     status:
       description: VirtualMachineExportStatus is the status for a VirtualMachineExport
@@ -7883,6 +7882,10 @@ var CRDsValidation map[string]string = map[string]string{
             the Virtual Machine export. It will be used to create the internal URLs
             for downloading the images
           type: string
+        tokenSecretRef:
+          description: TokenSecretRef is the name of the secret that contains the
+            token used by the export server pod
+          type: string
       type: object
   required:
   - spec

pkg/virtctl/vmexport/vmexport.go

@@ -284,13 +284,14 @@ func CreateVirtualMachineExport(client kubecli.KubevirtClient, vmeInfo *VMExport
 		return fmt.Errorf("VirtualMachineExport '%s/%s' already exists", vmeInfo.Namespace, vmeInfo.Name)
 	}
 
+	secretRef := getExportSecretName(vmeInfo.Name)
 	vmexport = &exportv1.VirtualMachineExport{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      vmeInfo.Name,
 			Namespace: vmeInfo.Namespace,
 		},
 		Spec: exportv1.VirtualMachineExportSpec{
-			TokenSecretRef: getExportSecretName(vmeInfo.Name),
+			TokenSecretRef: &secretRef,
 			Source:         vmeInfo.ExportSource,
 		},
 	}
@@ -548,7 +549,12 @@ func getOrCreateTokenSecret(client kubecli.KubevirtClient, vmexport *exportv1.Vi
 
 // getTokenFromSecret extracts the token from the secret specified on the virtualMachineExport
 func getTokenFromSecret(client kubecli.KubevirtClient, vmexport *exportv1.VirtualMachineExport) (string, error) {
-	secret, err := client.CoreV1().Secrets(vmexport.Namespace).Get(context.Background(), vmexport.Spec.TokenSecretRef, metav1.GetOptions{})
+	secretName := ""
+	if vmexport.Status != nil && vmexport.Status.TokenSecretRef != nil {
+		secretName = *vmexport.Status.TokenSecretRef
+	}
+
+	secret, err := client.CoreV1().Secrets(vmexport.Namespace).Get(context.Background(), secretName, metav1.GetOptions{})
 	if err != nil {
 		return "", err
 	}