From dd105eeb6ce61d9e63b3b013e29bed7dad6156b2 Mon Sep 17 00:00:00 2001
From: Florent Gluck <florent.gluck@hesge.ch>
Date: Wed, 7 Sep 2022 08:48:20 +0200
Subject: [PATCH] Fixed minor mistake in README.md Reworked VMs code a bit to
have more conistent behavior among methods
---
README.md | 4 +-
src/router/routerTemplates.go | 2 +-
src/router/routerVMs.go | 140 +++++++++++++++++-----------------
src/vms/vms.go | 100 ++++++++++++------------
4 files changed, 126 insertions(+), 120 deletions(-)
diff --git a/README.md b/README.md
index bc76ef9..7246ec8 100644
--- a/README.md
+++ b/README.md
@@ -375,8 +375,8 @@ Legend for the "Done" column:
| Done | Route | Description | Method | Parameters | Req. user cap. |
|--- |--- |--- |--- |--- |--- |
-| x | `/templates/vm` | create a template | POST | vmID,name,descr,access | `TPL_CREATE` |
-| x | `/templates/qcow` | create a template | POST | qcow,name,descr,access | `TPL_CREATE` |
+| x | `/templates/vm` | create a template | POST | vmID,name,access | `TPL_CREATE` |
+| x | `/templates/qcow` | create a template | POST | qcow,name,access | `TPL_CREATE` |
| x | `/templates/{id}` | delete a template | DELETE | | `TPL_DESTROY_ANY|TPL_DESTROY` |
| x | `/templates` | list templates | GET | | `TPL_LIST_ANY|TPL_LIST` |
diff --git a/src/router/routerTemplates.go b/src/router/routerTemplates.go
index 9e0dd5e..fae960a 100644
--- a/src/router/routerTemplates.go
+++ b/src/router/routerTemplates.go
@@ -185,7 +185,7 @@ func (r *RouterTemplates)CreateTemplateFromQCOW(c echo.Context) error {
// CAP_TPL_DESTROY_ANY: any template can be deleted.
// CAP_TPL_DESTROY: only a template owned by the user can be deleted.
// Remark: a template can only be deleted if no VM references it!
-// curl --cacert ca.pem -X DELETE https://localhost:1077/templates/Xubuntu_22.04_+_gcc_+_vscodium -H "Authorization: Bearer <AccessToken>"
+// curl --cacert ca.pem -X DELETE https://localhost:1077/templates/4913a2bb-edfe-4dfe-af53-38197a44523b -H "Authorization: Bearer <AccessToken>"
func (r *RouterTemplates)DeleteTemplateByID(c echo.Context) error {
// Retrieves logged user from context.
user, err := getLoggedUser(r.users, c)
diff --git a/src/router/routerVMs.go b/src/router/routerVMs.go
index 278d7e5..6a30eff 100644
--- a/src/router/routerVMs.go
+++ b/src/router/routerVMs.go
@@ -359,75 +359,6 @@ func (r *RouterVMs)DeleteVMAccessForUser(c echo.Context) error {
return c.JSONPretty(http.StatusOK, jsonMsg("OK"), " ")
}
-// Helper function that returns a list of VMs that match either:
-// - the logged user has the userCapabilityAny capability.
-// - the VM access for the logged user matches the vmAccessCapability capability.
-// Also, VMs for which cond is false are filtered out.
-func (r *RouterVMs)performVMsList(c echo.Context, userCapabilityAny, vmAccessCapability string, cond vms.VMKeeperFn) error {
- // Retrieves logged user from context.
- user, err := getLoggedUser(r.users, c)
- if err != nil {
- return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
- }
-
- // If the logged user has the XX_ANY capability (userCapabilityAny), returns all VMs.
- if user.HasCapability(userCapabilityAny) {
- // Returns all VMs that pass the condition.
- return c.JSONPretty(http.StatusOK, r.vms.GetVMs(cond), " ")
- } else {
- // Returns all VMs for which VM access for the logged user matches the specified the VM Access capability (vmAccessCapability).
- return c.JSONPretty(http.StatusOK, r.vms.GetVMs(
- func(vm vms.VM) bool {
- capabilities, exists := vm.Access[user.Email]
- if exists {
- _, visible := capabilities[vmAccessCapability]
- return visible && cond(vm)
- } else {
- return false
- }
- }), " ")
- }
-}
-
-// Helper function that performs an action on a VM based either on:
-// - the logged user has the userCapabilityAny capability.
-// - the VM access for the logged user matches the vmAccessCapability capability.
-func (r *RouterVMs)performVMAction(c echo.Context, userCapabilityAny, vmAccessCapability string, action vmActionFn) error {
- // Retrieves the VM on which to perform the action.
- id, err := uuid.Parse(c.Param("id"))
- if err != nil {
- return echo.NewHTTPError(http.StatusBadRequest, err.Error())
- }
- vm, err := r.vms.GetVM(id)
- if err != nil {
- return echo.NewHTTPError(http.StatusNotFound, err.Error())
- }
-
- // Retrieves the logged user from context.
- user, err := getLoggedUser(r.users, c)
- if err != nil {
- return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
- }
-
- // First, checks if the user has the XX_ANY capability
- if user.HasCapability(userCapabilityAny) {
- return action(c, &vm)
- } else {
- // Check if the VM access for the logged user matches the required capability
- userCaps, exists := vm.Access[user.Email]
- if !exists {
- return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
- }
-
- _, hasAccess := userCaps[vmAccessCapability]
- if !hasAccess {
- return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
- }
-
- return action(c, &vm)
- }
-}
-
// Exports a VM's directory into a .tar archive.
// Requires either capability:
// User cap: VM_READFS_ANY: any VM can have their filesystem read.
@@ -511,4 +442,73 @@ func (r *RouterVMs)ImportFilesToVM(c echo.Context) error {
return c.JSONPretty(http.StatusCreated, jsonMsg("OK"), " ")
})
-}
\ No newline at end of file
+}
+
+// Helper function that returns a list of VMs that match either:
+// - the logged user has the userCapabilityAny capability.
+// - the VM access for the logged user matches the vmAccessCapability capability.
+// Also, VMs for which cond is false are filtered out.
+func (r *RouterVMs)performVMsList(c echo.Context, userCapabilityAny, vmAccessCapability string, cond vms.VMKeeperFn) error {
+ // Retrieves logged user from context.
+ user, err := getLoggedUser(r.users, c)
+ if err != nil {
+ return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
+ }
+
+ // If the logged user has the XX_ANY capability (userCapabilityAny), returns all VMs.
+ if user.HasCapability(userCapabilityAny) {
+ // Returns all VMs that pass the condition.
+ return c.JSONPretty(http.StatusOK, r.vms.GetVMs(cond), " ")
+ } else {
+ // Returns all VMs for which VM access for the logged user matches the specified the VM Access capability (vmAccessCapability).
+ return c.JSONPretty(http.StatusOK, r.vms.GetVMs(
+ func(vm vms.VM) bool {
+ capabilities, exists := vm.Access[user.Email]
+ if exists {
+ _, visible := capabilities[vmAccessCapability]
+ return visible && cond(vm)
+ } else {
+ return false
+ }
+ }), " ")
+ }
+}
+
+// Helper function that performs an action on a VM based either on:
+// - the logged user has the userCapabilityAny capability.
+// - the VM access for the logged user matches the vmAccessCapability capability.
+func (r *RouterVMs)performVMAction(c echo.Context, userCapabilityAny, vmAccessCapability string, action vmActionFn) error {
+ // Retrieves the VM on which to perform the action.
+ id, err := uuid.Parse(c.Param("id"))
+ if err != nil {
+ return echo.NewHTTPError(http.StatusBadRequest, err.Error())
+ }
+ vm, err := r.vms.GetVM(id)
+ if err != nil {
+ return echo.NewHTTPError(http.StatusNotFound, err.Error())
+ }
+
+ // Retrieves the logged user from context.
+ user, err := getLoggedUser(r.users, c)
+ if err != nil {
+ return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
+ }
+
+ // First, checks if the user has the XX_ANY capability
+ if user.HasCapability(userCapabilityAny) {
+ return action(c, &vm)
+ } else {
+ // Check if the VM access for the logged user matches the required capability
+ userCaps, exists := vm.Access[user.Email]
+ if !exists {
+ return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
+ }
+
+ _, hasAccess := userCaps[vmAccessCapability]
+ if !hasAccess {
+ return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
+ }
+
+ return action(c, &vm)
+ }
+}
diff --git a/src/vms/vms.go b/src/vms/vms.go
index 129a978..0e5277e 100644
--- a/src/vms/vms.go
+++ b/src/vms/vms.go
@@ -94,11 +94,11 @@ func InitVMs() error {
}
// Returns the list of VMs for which VMKeeperFn returns true.
-func (vms *VMs)GetVMs(keep VMKeeperFn) []VM {
+func (vms *VMs)GetVMs(keepFn VMKeeperFn) []VM {
vms.rwlock.RLock()
list := []VM{}
for _, vm := range vms.m {
- if keep(vm) {
+ if keepFn(vm) {
list = append(list, vm)
}
}
@@ -115,6 +115,10 @@ func (vms *VMs)GetVMs(keep VMKeeperFn) []VM {
func (vms *VMs)GetVM(vmID uuid.UUID) (VM, error) {
vms.rwlock.RLock()
defer vms.rwlock.RUnlock()
+ return vms.getVMUnsafe(vmID)
+}
+
+func (vms *VMs)getVMUnsafe(vmID uuid.UUID) (VM, error) {
vm, exists := vms.m[vmID.String()]
if !exists {
return dummyVM, errors.New("VM not found")
@@ -123,24 +127,25 @@ func (vms *VMs)GetVM(vmID uuid.UUID) (VM, error) {
}
// Deletes a VM by its ID and deletes its files.
-func (vms *VMs)DeleteVM(id uuid.UUID) error {
- key := id.String()
+func (vms *VMs)DeleteVM(vmID uuid.UUID) error {
vms.rwlock.Lock()
defer vms.rwlock.Unlock()
- vm, exists := vms.m[key]
- if exists {
- // Deletes the VM's files (and directories).
- vm.mutex.Lock()
- if err := vm.delete(); err != nil {
- vm.mutex.Unlock()
- return err
- }
+
+ vm, err := vms.getVMUnsafe(vmID)
+ if err != nil {
+ return err
+ }
+
+ // Deletes the VM's files (and directories).
+ vm.mutex.Lock()
+ if err := vm.delete(); err != nil {
vm.mutex.Unlock()
- // Removes the VM from the map.
- delete(vms.m, key)
- return nil
+ return err
}
- return errors.New("VM not found")
+ vm.mutex.Unlock()
+ // Removes the VM from the map.
+ delete(vms.m, vm.ID.String())
+ return nil
}
// Adds a VM and writes its files.
@@ -165,13 +170,13 @@ func (vms *VMs)AddVM(vm *VM) error {
// Starts a VM by its ID.
// Returns the port on which the VM is running and the access password.
-func (vms *VMs)StartVM(id uuid.UUID) (int, string, error) {
+func (vms *VMs)StartVM(vmID uuid.UUID) (int, string, error) {
vms.rwlock.Lock()
defer vms.rwlock.Unlock()
- vm, exists := vms.m[id.String()]
- if !exists {
- return -1, "", errors.New("VM not found")
+ vm, err := vms.getVMUnsafe(vmID)
+ if err != nil {
+ return 0, "", err
}
// Check there will be at least 1GB of RAM left after the VM has started,
@@ -215,33 +220,33 @@ func (vms *VMs)StartVM(id uuid.UUID) (int, string, error) {
}
// Stops by force a VM by its ID.
-func (vms *VMs)StopVM(id uuid.UUID) error {
+func (vms *VMs)StopVM(vmID uuid.UUID) error {
vms.rwlock.RLock()
defer vms.rwlock.RUnlock()
- vm, exists := vms.m[id.String()]
- if !exists {
- return errors.New("VM not found")
+ vm, err := vms.getVMUnsafe(vmID)
+ if err != nil {
+ return err
}
vm.mutex.Lock()
- err := vm.stop()
+ err = vm.stop()
vm.mutex.Unlock()
return err
}
// Gracefully stops a VM by its ID.
-func (vms *VMs)ShutdownVM(id uuid.UUID) error {
+func (vms *VMs)ShutdownVM(vmID uuid.UUID) error {
vms.rwlock.Lock()
defer vms.rwlock.Unlock()
- vm, exists := vms.m[id.String()]
- if !exists {
- return errors.New("VM not found")
+ vm, err := vms.getVMUnsafe(vmID)
+ if err != nil {
+ return err
}
vm.mutex.Lock()
- err := vm.shutdown()
+ err = vm.shutdown()
vm.mutex.Unlock()
return err
}
@@ -261,10 +266,9 @@ func (vms *VMs)IsTemplateUsed(templateID string) bool {
// Edit a VM' specs: name, cpus, ram, nic
func (vms *VMs)EditVM(vmID uuid.UUID, name string, cpus, ram int, nic NicType) error {
- // Retrieves the VM to be edited.
- vm, err := vms.GetVM(vmID)
+ vm, err := vms.getVMUnsafe(vmID)
if err != nil {
- return errors.New("VM not found")
+ return err
}
// Only updates fields that have changed.
@@ -305,12 +309,18 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
return errors.New("Invalid capability")
}
+ vms.rwlock.Lock()
+ defer vms.rwlock.Unlock()
+
// Retrieves the VM for which the access caps must be changed.
- vm, err := vms.GetVM(vmID)
+ vm, err := vms.getVMUnsafe(vmID)
if err != nil {
- return errors.New("VM not found")
+ return err
}
+ vm.mutex.Lock()
+ defer vm.mutex.Unlock()
+
// Checks the logged user has VM_SET_ACCESS set in her/his VM access.
userCaps := vm.Access[loggedUserEmail]
_, exists := userCaps[caps.CAP_VM_SET_ACCESS]
@@ -320,11 +330,6 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
vm.Access[userEmail] = newAccess
- vm.mutex.Lock()
- defer vm.mutex.Unlock()
- vms.rwlock.Lock()
- defer vms.rwlock.Unlock()
-
if err = vms.updateVM(&vm); err != nil {
return errors.New("Failed updating VM")
}
@@ -336,12 +341,18 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
// loggedUserEmail is the email of the currently logged user
// userMail is the email of the user for which to remove the access
func (vms *VMs)DeleteVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string) error {
+ vms.rwlock.Lock()
+ defer vms.rwlock.Unlock()
+
// Retrieves the VM for which the access caps must be changed.
- vm, err := vms.GetVM(vmID)
+ vm, err := vms.getVMUnsafe(vmID)
if err != nil {
- return errors.New("VM not found")
+ return err
}
+ vm.mutex.Lock()
+ defer vm.mutex.Unlock()
+
// Checks the user has VM_SET_ACCESS set in her/his VM access.
userCaps := vm.Access[loggedUserEmail]
_, exists := userCaps[caps.CAP_VM_SET_ACCESS]
@@ -352,11 +363,6 @@ func (vms *VMs)DeleteVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string)
// Removes the user from the Access map
delete(vm.Access, userEmail)
- vm.mutex.Lock()
- defer vm.mutex.Unlock()
- vms.rwlock.Lock()
- defer vms.rwlock.Unlock()
-
if err = vms.updateVM(&vm); err != nil {
return errors.New("Failed updating VM")
}
--
GitLab