The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6791
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Based on suggestion from https://discuss.linuxcontainers.org/t/two-suggestions-to-improve-first-impression-with-lxd-3-19/6645 this PR expands the suggests to improve instance name validation error messages to make them more helpful.
Also validates snapshot names.
From 2c911762537cae00fe6409d994a4c8162072279b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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.parr...@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
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
