Skip to content

Commit

Permalink
Add JSON output by default, allow yaml with Accept: application/yaml
Browse files Browse the repository at this point in the history
Addressed review comments.
- Controller sanitized VM/CM/DV output
- Fixed URIs to include internal/external
- Modified proxy to not add query to show external URI.
- ConfigMap data is now in string format
- Cleared out metadata fields in controller

Signed-off-by: Alexander Wels <[email protected]>
  • Loading branch information
awels committed Jan 17, 2023
1 parent 3c141ee commit 65fe62a
Show file tree
Hide file tree
Showing 16 changed files with 606 additions and 213 deletions.
4 changes: 4 additions & 0 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -19969,6 +19969,10 @@
"cert"
],
"properties": {
"cdiHeaderSecretUrl": {
"description": "CDIHeaderSecretUrl returns a Containerized Data Importer compatible secret",
"type": "string"
},
"cert": {
"description": "Cert is the public CA certificate base64 encoded",
"type": "string"
Expand Down
3 changes: 0 additions & 3 deletions cmd/virt-exportproxy/virt-exportproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,9 @@ func (app *exportProxyApp) proxyHandler(w http.ResponseWriter, r *http.Request)

p := &httputil.ReverseProxy{
Director: func(req *http.Request) {
query := req.URL.Query()
query.Set("externalURI", "true")
req.URL.Scheme = "https"
req.URL.Host = host
req.URL.Path = targetPath
req.URL.RawQuery = query.Encode()
log.Log.Infof("Proxying to %s", req.URL.String())
if _, ok := req.Header["User-Agent"]; !ok {
// explicitly disable User-Agent so it's not set to default value
Expand Down
1 change: 1 addition & 0 deletions cmd/virt-exportserver/virt-exportserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func getVolumeInfo() []exportServer.VolumeInfo {
RawURI: os.Getenv(envPrefix + "_EXPORT_RAW_URI"),
RawGzURI: os.Getenv(envPrefix + "_EXPORT_RAW_GZIP_URI"),
VMURI: os.Getenv("EXPORT_VM_DEF_URI"),
SecretURI: os.Getenv("EXPORT_SECRET_DEF_URI"),
}
result = append(result, vi)
}
Expand Down
32 changes: 20 additions & 12 deletions pkg/storage/export/export/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ const (
vmManifest = "virtualmachine-manifest"
exportNameKey = "export-name"
manifestData = "manifest-data"
exportDefPath = "/export-def/"
manifestsPath = "/manifests/all"
secretManifestPath = "/manifests/secret"
externalHostKey = "external_host"
internalHostKey = "internal_host"
externalCaConfigMapKey = "external_ca_cm"
Expand Down Expand Up @@ -773,7 +774,7 @@ func (ctrl *VMExportController) getExporterPod(vmExport *exportv1.VirtualMachine
}

func (ctrl *VMExportController) createExporterPod(vmExport *exportv1.VirtualMachineExport, service *corev1.Service, pvcs []*corev1.PersistentVolumeClaim) (*corev1.Pod, error) {
log.Log.V(3).Infof("Checking if pod exist: %s/%s", vmExport.Namespace, ctrl.getExportPodName(vmExport))
log.Log.V(3).Infof("Checking if pod exists: %s/%s", vmExport.Namespace, ctrl.getExportPodName(vmExport))
key := controller.NamespacedKey(vmExport.Namespace, ctrl.getExportPodName(vmExport))
if obj, exists, err := ctrl.PodInformer.GetStore().GetByKey(key); err != nil {
log.Log.V(3).Errorf("error %v", err)
Expand Down Expand Up @@ -859,7 +860,10 @@ func (ctrl *VMExportController) createExporterPodManifest(vmExport *exportv1.Vir
Value: getDeadlineValue(deadline, vmExport).Format(time.RFC3339),
}, corev1.EnvVar{
Name: "EXPORT_VM_DEF_URI",
Value: exportDefPath,
Value: manifestsPath,
}, corev1.EnvVar{
Name: "EXPORT_SECRET_DEF_URI",
Value: secretManifestPath,
})

tokenSecretRef := ""
Expand Down Expand Up @@ -936,9 +940,9 @@ func (ctrl *VMExportController) createDataManifestAndAddToPod(vmExport *exportv1
}

func (ctrl *VMExportController) createDataManifestConfigMap(vmExport *exportv1.VirtualMachineExport, vm *virtv1.VirtualMachine, service *corev1.Service) (*corev1.ConfigMap, error) {
data := make(map[string][]byte)
data := make(map[string]string)

data[internalHostKey] = []byte(fmt.Sprintf("%s.%s.svc", service.Name, service.Namespace))
data[internalHostKey] = fmt.Sprintf("%s.%s.svc", service.Name, service.Namespace)
cert, err := ctrl.internalExportCa()
if err != nil {
return nil, err
Expand All @@ -948,24 +952,24 @@ func (ctrl *VMExportController) createDataManifestConfigMap(vmExport *exportv1.V
if err != nil {
return nil, err
}
data[internalCaConfigMapKey] = caCmBytes
data[internalCaConfigMapKey] = string(caCmBytes)

externalUrlPath := fmt.Sprintf(externalUrlLinkFormat, vmExport.Namespace, vmExport.Name)
externalLinkHost, cert := ctrl.getExternalLinkHostAndCert()
if externalLinkHost != "" {
data[externalHostKey] = []byte(path.Join(externalLinkHost, externalUrlPath))
data[externalHostKey] = path.Join(externalLinkHost, externalUrlPath)
externalCaCm := ctrl.createExportCaConfigMap(cert, vmExport.Name)
caCmBytes, err := json.Marshal(externalCaCm)
if err != nil {
return nil, err
}
data[externalCaConfigMapKey] = caCmBytes
data[externalCaConfigMapKey] = string(caCmBytes)
}
vmBytes, err := ctrl.generateVMDefinitionFromVm(vm)
if err != nil {
return nil, err
}
data[vmManifest] = vmBytes
data[vmManifest] = string(vmBytes)

datavolumes := ctrl.generateDataVolumesFromVm(vm)
for _, datavolume := range datavolumes {
Expand All @@ -974,10 +978,10 @@ func (ctrl *VMExportController) createDataManifestConfigMap(vmExport *exportv1.V
if err != nil {
return nil, err
}
data[fmt.Sprintf("dv-%s", datavolume.Name)] = dvBytes
data[fmt.Sprintf("dv-%s", datavolume.Name)] = string(dvBytes)
}
}
data[exportNameKey] = []byte(vmExport.Name)
data[exportNameKey] = vmExport.Name
res := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: vm.Namespace,
Expand All @@ -986,7 +990,7 @@ func (ctrl *VMExportController) createDataManifestConfigMap(vmExport *exportv1.V
*metav1.NewControllerRef(vmExport, exportGVK),
},
},
BinaryData: data,
Data: data,
}
return res, nil
}
Expand Down Expand Up @@ -1325,6 +1329,10 @@ func (ctrl *VMExportController) generateVMDefinitionFromVm(vm *virtv1.VirtualMac
// Clear status
expandedVm.Status = virtv1.VirtualMachineStatus{}
expandedVm.ManagedFields = nil
expandedVm.ObjectMeta.SetCreationTimestamp(metav1.Time{})
expandedVm.ObjectMeta.SetUID("")
expandedVm.ObjectMeta.SetResourceVersion("")

// Update dvTemplates if exists
expandedVm = ctrl.updateHttpSourceDataVolumeTemplate(vm)
vmBytes, err := json.Marshal(expandedVm)
Expand Down
8 changes: 4 additions & 4 deletions pkg/storage/export/export/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var (
expectedPodEnvVars = []k8sv1.EnvVar{
{
Name: "EXPORT_VM_DEF_URI",
Value: exportDefPath,
Value: manifestsPath,
}, {
Name: "CERT_FILE",
Value: "/cert/tls.crt",
Expand Down Expand Up @@ -1131,9 +1131,9 @@ var _ = Describe("Export controller", func() {
Expect(ok).To(BeTrue())
Expect(cm.GetName()).To(Equal(cmName))
Expect(cm.GetNamespace()).To(Equal(testNamespace))
Expect(cm.BinaryData).ToNot(BeEmpty())
Expect(string(cm.BinaryData[internalHostKey])).To(Equal(fmt.Sprintf("%s.%s.svc", controller.getExportServiceName(testVMExport), service.Namespace)))
Expect(cm.BinaryData[vmManifest]).To(Equal(vmBytes))
Expect(cm.Data).ToNot(BeEmpty())
Expect(cm.Data[internalHostKey]).To(Equal(fmt.Sprintf("%s.%s.svc", controller.getExportServiceName(testVMExport), service.Namespace)))
Expect(cm.Data[vmManifest]).To(Equal(string(vmBytes)))
return true, cm, nil
})
err = controller.createDataManifestAndAddToPod(testVMExport, vm, testPod, service)
Expand Down
15 changes: 9 additions & 6 deletions pkg/storage/export/export/links.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ const (
exportResourceName = "virtualmachineexports"
gv = apiGroup + "/" + apiVersion
externalUrlLinkFormat = "/api/" + gv + "/namespaces/%s/" + exportResourceName + "/%s"
internal = "internal"
external = "external"
)

func (ctrl *VMExportController) getInteralLinks(pvcs []*corev1.PersistentVolumeClaim, exporterPod *corev1.Pod, service *corev1.Service) (*exportv1.VirtualMachineExportLink, error) {
Expand All @@ -58,25 +60,26 @@ func (ctrl *VMExportController) getInteralLinks(pvcs []*corev1.PersistentVolumeC
return nil, err
}
host := fmt.Sprintf("%s.%s.svc", service.Name, service.Namespace)
return ctrl.getLinks(pvcs, exporterPod, host, internalCert)
return ctrl.getLinks(pvcs, exporterPod, host, internal, internalCert)
}

func (ctrl *VMExportController) getExternalLinks(pvcs []*corev1.PersistentVolumeClaim, exporterPod *corev1.Pod, export *exportv1.VirtualMachineExport) (*exportv1.VirtualMachineExportLink, error) {
urlPath := fmt.Sprintf(externalUrlLinkFormat, export.Namespace, export.Name)
externalLinkHost, cert := ctrl.getExternalLinkHostAndCert()
if externalLinkHost != "" {
hostAndBase := path.Join(externalLinkHost, urlPath)
return ctrl.getLinks(pvcs, exporterPod, hostAndBase, cert)
return ctrl.getLinks(pvcs, exporterPod, hostAndBase, external, cert)
}
return nil, nil
}

func (ctrl *VMExportController) getLinks(pvcs []*corev1.PersistentVolumeClaim, exporterPod *corev1.Pod, hostAndBase, cert string) (*exportv1.VirtualMachineExportLink, error) {
func (ctrl *VMExportController) getLinks(pvcs []*corev1.PersistentVolumeClaim, exporterPod *corev1.Pod, hostAndBase, linkType, cert string) (*exportv1.VirtualMachineExportLink, error) {
const scheme = "https://"
exportLink := &exportv1.VirtualMachineExportLink{
Volumes: []exportv1.VirtualMachineExportVolume{},
Cert: cert,
DefinitionUrl: scheme + path.Join(hostAndBase, exportDefPath),
Volumes: []exportv1.VirtualMachineExportVolume{},
Cert: cert,
DefinitionUrl: scheme + path.Join(hostAndBase, linkType, manifestsPath),
CDIHeaderSecretUrl: scheme + path.Join(hostAndBase, linkType, secretManifestPath),
}
for _, pvc := range pvcs {
if pvc != nil && exporterPod != nil && exporterPod.Status.Phase == corev1.PodRunning {
Expand Down
6 changes: 6 additions & 0 deletions pkg/storage/export/export/vmsnapshot-source.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package export
import (
"context"
"fmt"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -185,6 +186,11 @@ func (ctrl *VMExportController) getOrCreatePVCFromSnapshot(vmExport *exportv1.Vi
func (ctrl *VMExportController) updateVMExporVMSnapshotStatus(vmExport *exportv1.VirtualMachineExport, exporterPod *corev1.Pod, service *corev1.Service, sourceVolumes *sourceVolumes) (time.Duration, error) {
vmExportCopy := vmExport.DeepCopy()

// Change the names of the returned pvcs by removing the prefix we used to create the PVCs, this matches
// the volumes of the source VM
for _, pvc := range sourceVolumes.volumes {
pvc.Name = strings.TrimPrefix(pvc.Name, fmt.Sprintf("%s-", vmExport.Name))
}
if err := ctrl.updateCommonVMExportStatusFields(vmExport, vmExportCopy, exporterPod, service, sourceVolumes); err != nil {
return 0, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/export/export/vmsnapshot-source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ var _ = Describe("VMSnapshot source", func() {

It("Should update status with correct links from snapshot with kubevirt content type", func() {
testVMExport := createSnapshotVMExport()
restoreName := fmt.Sprintf("%s-%s", testVMExport.Name, testVolumesnapshotName)
restoreName := testVolumesnapshotName
vmExportClient.Fake.PrependReactor("update", "virtualmachineexports", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
update, ok := action.(testing.UpdateAction)
Expect(ok).To(BeTrue())
Expand Down Expand Up @@ -575,7 +575,7 @@ var _ = Describe("VMSnapshot source", func() {

It("Should update status with correct links from snapshot with other content type", func() {
testVMExport := createSnapshotVMExport()
restoreName := fmt.Sprintf("%s-%s", testVMExport.Name, testVolumesnapshotName)
restoreName := testVolumesnapshotName
vmExportClient.Fake.PrependReactor("update", "virtualmachineexports", func(action testing.Action) (handled bool, obj runtime.Object, err error) {
update, ok := action.(testing.UpdateAction)
Expect(ok).To(BeTrue())
Expand Down
2 changes: 2 additions & 0 deletions pkg/storage/export/virt-exportserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
"//vendor/github.com/spf13/pflag:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
Expand All @@ -34,6 +35,7 @@ go_test(
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
],
Expand Down
Loading

0 comments on commit 65fe62a

Please sign in to comment.