[lxc-devel] [lxd/master] Instance name validation improvements

tomponline on Github lxc-bot at linuxcontainers.org
Tue Jan 28 09:28:04 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 567 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200128/8804946a/attachment.bin>
-------------- next part --------------
From 2c911762537cae00fe6409d994a4c8162072279b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Jan 2020 09:23:29 +0000
Subject: [PATCH 1/4] lxd/container: Removes containerValidName function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index e86439c2df..abf949c450 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -53,20 +53,6 @@ func init() {
 
 // Helper functions
 
-func containerValidName(name string) error {
-	if strings.Contains(name, shared.SnapshotDelimiter) {
-		return fmt.Errorf(
-			"The character '%s' is reserved for snapshots.",
-			shared.SnapshotDelimiter)
-	}
-
-	if !shared.ValidHostname(name) {
-		return fmt.Errorf("Container name isn't a valid hostname")
-	}
-
-	return nil
-}
-
 // instanceCreateAsEmpty creates an empty instance.
 func instanceCreateAsEmpty(d *Daemon, args db.InstanceArgs) (instance.Instance, error) {
 	// Create the instance record.

From a8a2612c9f15843e5c8835bbfddfaaa00afbee29 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Jan 2020 09:23:48 +0000
Subject: [PATCH 2/4] lxd/container: Switches to instance.ValidName

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container.go                    | 15 +++++++--------
 lxd/container_logs.go               | 10 +++++++---
 lxd/container_lxc.go                |  5 +++--
 lxd/containers_post.go              |  5 +++++
 lxd/instance/drivers/driver_qemu.go |  5 +++--
 5 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index abf949c450..844fe79b75 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -672,19 +672,18 @@ func instanceCreateInternal(s *state.State, args db.InstanceArgs) (instance.Inst
 		args.Architecture = s.OS.Architectures[0]
 	}
 
-	// Validate container name if not snapshot (as snapshots use disallowed / char in names).
-	if !args.Snapshot {
-		err := containerValidName(args.Name)
-		if err != nil {
-			return nil, err
-		}
+	err := instance.ValidName(args.Name, args.Snapshot)
+	if err != nil {
+		return nil, err
+	}
 
-		// Unset expiry date since containers don't expire.
+	if !args.Snapshot {
+		// Unset expiry date since instances don't expire.
 		args.ExpiryDate = time.Time{}
 	}
 
 	// Validate container config.
-	err := instance.ValidConfig(s.OS, args.Config, false, false)
+	err = instance.ValidConfig(s.OS, args.Config, false, false)
 	if err != nil {
 		return nil, err
 	}
diff --git a/lxd/container_logs.go b/lxd/container_logs.go
index dc1355a05c..3c7aef4b24 100644
--- a/lxd/container_logs.go
+++ b/lxd/container_logs.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/gorilla/mux"
 
+	"github.com/lxc/lxd/lxd/instance"
 	"github.com/lxc/lxd/lxd/project"
 	"github.com/lxc/lxd/lxd/response"
 	"github.com/lxc/lxd/shared"
@@ -64,7 +65,8 @@ func containerLogsGet(d *Daemon, r *http.Request) response.Response {
 		return resp
 	}
 
-	if err := containerValidName(name); err != nil {
+	err = instance.ValidName(name, false)
+	if err != nil {
 		return response.BadRequest(err)
 	}
 
@@ -118,7 +120,8 @@ func containerLogGet(d *Daemon, r *http.Request) response.Response {
 
 	file := mux.Vars(r)["file"]
 
-	if err := containerValidName(name); err != nil {
+	err = instance.ValidName(name, false)
+	if err != nil {
 		return response.BadRequest(err)
 	}
 
@@ -154,7 +157,8 @@ func containerLogDelete(d *Daemon, r *http.Request) response.Response {
 
 	file := mux.Vars(r)["file"]
 
-	if err := containerValidName(name); err != nil {
+	err = instance.ValidName(name, false)
+	if err != nil {
 		return response.BadRequest(err)
 	}
 
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 0a4497f47e..6e162807b4 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3625,8 +3625,9 @@ func (c *containerLXC) Rename(newName string) error {
 	logger.Info("Renaming container", ctxMap)
 
 	// Sanity checks.
-	if !c.IsSnapshot() && !shared.ValidHostname(newName) {
-		return fmt.Errorf("Invalid container name")
+	err := instance.ValidName(newName, c.IsSnapshot())
+	if err != nil {
+		return err
 	}
 
 	if c.IsRunning() {
diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index f78781b903..376261d49e 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -58,6 +58,11 @@ func createFromImage(d *Daemon, project string, req *api.InstancesPost) response
 			Profiles:    req.Profiles,
 		}
 
+		err := instance.ValidName(args.Name, args.Snapshot)
+		if err != nil {
+			return err
+		}
+
 		var info *api.Image
 		if req.Source.Server != "" {
 			autoUpdate, err := cluster.ConfigGetBool(d.cluster, "images.auto_update_cached")
diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index fb13696a57..33cd43f8c6 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -1794,8 +1794,9 @@ func (vm *qemu) Rename(newName string) error {
 	logger.Info("Renaming instance", ctxMap)
 
 	// Sanity checks.
-	if !vm.IsSnapshot() && !shared.ValidHostname(newName) {
-		return fmt.Errorf("Invalid instance name")
+	err := instance.ValidName(newName, vm.IsSnapshot())
+	if err != nil {
+		return err
 	}
 
 	if vm.IsRunning() {

From 853816a166c049e6a6d40a536314b3f5543c43f9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Jan 2020 09:24:40 +0000
Subject: [PATCH 3/4] lxd/instance/instance/utils: Adds ValidName function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/instance_utils.go | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lxd/instance/instance_utils.go b/lxd/instance/instance_utils.go
index 4fa6984ef6..3293e01478 100644
--- a/lxd/instance/instance_utils.go
+++ b/lxd/instance/instance_utils.go
@@ -872,3 +872,18 @@ func SuitableArchitectures(s *state.State, project string, req api.InstancesPost
 	// No other known types
 	return nil, fmt.Errorf("Unknown instance source type: %s", req.Source.Type)
 }
+
+// ValidName validates an instance name. There are different validation rules for instance snapshot names
+// so it takes an argument indicating whether the name is to be used for a snapshot or not.
+func ValidName(instanceName string, isSnapshot bool) error {
+	if !isSnapshot && strings.Contains(instanceName, shared.SnapshotDelimiter) {
+		return fmt.Errorf("The character %q is reserved for snapshots", shared.SnapshotDelimiter)
+	}
+
+	err := shared.ValidHostname(instanceName)
+	if err != nil {
+		return errors.Wrap(err, "Invalid instance name")
+	}
+
+	return nil
+}

From 06cb1e7edb19614bdd01d6d7b11489cabdebec80 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 28 Jan 2020 09:25:43 +0000
Subject: [PATCH 4/4] shared/util: Modifies ValidHostname to return specific
 error

This makes the error messages more helpful to users.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 shared/util.go | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/shared/util.go b/shared/util.go
index 0a80f5dd7e..43b11b495c 100644
--- a/shared/util.go
+++ b/shared/util.go
@@ -647,33 +647,34 @@ func RunningInUserNS() bool {
 	return true
 }
 
-func ValidHostname(name string) bool {
+// ValidHostname checks the string is valid DNS hostname.
+func ValidHostname(name string) error {
 	// Validate length
 	if len(name) < 1 || len(name) > 63 {
-		return false
+		return fmt.Errorf("Name must be 1-63 characters long")
 	}
 
 	// Validate first character
 	if strings.HasPrefix(name, "-") {
-		return false
+		return fmt.Errorf(`Name must not start with "-" character`)
 	}
 
 	if _, err := strconv.Atoi(string(name[0])); err == nil {
-		return false
+		return fmt.Errorf("Name must not be a number")
 	}
 
 	// Validate last character
 	if strings.HasSuffix(name, "-") {
-		return false
+		return fmt.Errorf(`Name must not end with "-" character`)
 	}
 
 	// Validate the character set
 	match, _ := regexp.MatchString("^[-a-zA-Z0-9]*$", name)
 	if !match {
-		return false
+		return fmt.Errorf("Name can only contain alphanumeric and hyphen characters")
 	}
 
-	return true
+	return nil
 }
 
 // Spawn the editor with a temporary YAML file for editing configs


More information about the lxc-devel mailing list