Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage: Fix import of instance snapshots with underscores in their names #13899

Merged
merged 3 commits into from
Aug 9, 2024
+32 −19
Merged

Storage: Fix import of instance snapshots with underscores in their names #13899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69bd881
lxd/storage/backend/lxd: Pass full snapshot name when validating impo…
tomponline Aug 9, 2024
a35079f
lxd/instance/instance/utils: Don't allow snapshots named ".." in Vali…
tomponline Aug 9, 2024
8c7d870
test: Update instance backup tests to check for invalid and valid sna…
tomponline Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lxd/instance/instance_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,11 @@ func ValidName(instanceName string, isSnapshot bool) error {
return fmt.Errorf("Invalid instance name %q: %w", parentName, err)
}

// Snapshot part is more flexible, but doesn't allow space or / character.
// Snapshot part is more flexible, but doesn't allow "..", space or / characters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also reject *?

$ lxc snapshot c1 '.*'
Error: Create instance snapshot: Failed to run: zfs snapshot -r default/containers/c1@snapshot-.*: exit status 1 (cannot create snapshot 'default/containers/c1@snapshot-.*': invalid character '*' in name
no snapshots were created)

And also @ and # which have special meaning in ZFS at least.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah maybe - although this might break users who've used snapshots with those chars in on non-zfs drivers.

if snapshotName == ".." {
return fmt.Errorf("Invalid instance snapshot name %q", snapshotName)
}

if strings.ContainsAny(snapshotName, " /") {
return fmt.Errorf("Invalid instance snapshot name %q: Cannot contain spaces or slashes", snapshotName)
}
Expand Down
6 changes: 4 additions & 2 deletions lxd/storage/backend_lxd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,14 +734,16 @@ func (b *lxdBackend) CreateInstanceFromBackup(srcBackup backup.Info, srcData io.
}

for _, snapName := range srcBackup.Snapshots {
err = instance.ValidName(snapName, true)
snapInstName := fmt.Sprintf("%s%s%s", srcBackup.Name, shared.SnapshotDelimiter, snapName)
err = instance.ValidName(snapInstName, true)
if err != nil {
return nil, nil, err
}
}

for _, snap := range srcBackup.Config.Snapshots {
err = instance.ValidName(snap.Name, true)
snapInstName := fmt.Sprintf("%s%s%s", srcBackup.Name, shared.SnapshotDelimiter, snap.Name)
err = instance.ValidName(snapInstName, true)
if err != nil {
return nil, nil, err
}
Expand Down
39 changes: 23 additions & 16 deletions test/suites/backup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,14 @@ _backup_import_with_project() {

lxc launch testimage c1
lxc launch testimage c2
lxc snapshot c2

# Check invalid snapshot names
! lxc snapshot c2 ".." || false
! lxc snapshot c2 "with/slash" || false
! lxc snapshot c2 "with space" || false

# Check valid snapshot name with underscore can be exported + imported
lxc snapshot c2 snap0-with_underscore

lxd_backend=$(storage_backend "$LXD_DIR")

Expand Down Expand Up @@ -398,20 +405,20 @@ _backup_import_with_project() {
fi

old_uuid="$(lxc storage volume get "${pool}" container/c2 volatile.uuid)"
old_snap0_uuid="$(lxc storage volume get "${pool}" container/c2/snap0 volatile.uuid)"
old_snap0_uuid="$(lxc storage volume get "${pool}" container/c2/snap0-with_underscore volatile.uuid)"
lxc export c2 "${LXD_DIR}/c2.tar.gz"
lxc delete --force c2

lxc import "${LXD_DIR}/c2.tar.gz"
lxc import "${LXD_DIR}/c2.tar.gz" c3
lxc info c2 | grep snap0
lxc info c3 | grep snap0
lxc info c2 | grep snap0-with_underscore
lxc info c3 | grep snap0-with_underscore

# Check if the imported instance and its snapshot have a new UUID.
[ -n "$(lxc storage volume get "${pool}" container/c2 volatile.uuid)" ]
[ -n "$(lxc storage volume get "${pool}" container/c2/snap0 volatile.uuid)" ]
[ -n "$(lxc storage volume get "${pool}" container/c2/snap0-with_underscore volatile.uuid)" ]
[ "$(lxc storage volume get "${pool}" container/c2 volatile.uuid)" != "${old_uuid}" ]
[ "$(lxc storage volume get "${pool}" container/c2/snap0 volatile.uuid)" != "${old_snap0_uuid}" ]
[ "$(lxc storage volume get "${pool}" container/c2/snap0-with_underscore volatile.uuid)" != "${old_snap0_uuid}" ]

lxc start c2
lxc start c3
Expand All @@ -422,20 +429,20 @@ _backup_import_with_project() {
# Import into different project (before deleting earlier import).
lxc import "${LXD_DIR}/c2.tar.gz" --project "$project-b"
lxc import "${LXD_DIR}/c2.tar.gz" --project "$project-b" c3
lxc info c2 --project "$project-b" | grep snap0
lxc info c3 --project "$project-b" | grep snap0
lxc info c2 --project "$project-b" | grep snap0-with_underscore
lxc info c3 --project "$project-b" | grep snap0-with_underscore
lxc start c2 --project "$project-b"
lxc start c3 --project "$project-b"
lxc stop c2 --project "$project-b" --force
lxc stop c3 --project "$project-b" --force
lxc restore c2 snap0 --project "$project-b"
lxc restore c3 snap0 --project "$project-b"
lxc restore c2 snap0-with_underscore --project "$project-b"
lxc restore c3 snap0-with_underscore --project "$project-b"
lxc delete --force c2 --project "$project-b"
lxc delete --force c3 --project "$project-b"
fi

lxc restore c2 snap0
lxc restore c3 snap0
lxc restore c2 snap0-with_underscore
lxc restore c3 snap0-with_underscore
lxc start c2
lxc start c3
lxc delete --force c2
Expand All @@ -445,14 +452,14 @@ _backup_import_with_project() {
if storage_backend_optimized_backup "$lxd_backend"; then
lxc import "${LXD_DIR}/c2-optimized.tar.gz"
lxc import "${LXD_DIR}/c2-optimized.tar.gz" c3
lxc info c2 | grep snap0
lxc info c3 | grep snap0
lxc info c2 | grep snap0-with_underscore
lxc info c3 | grep snap0-with_underscore
lxc start c2
lxc start c3
lxc stop c2 --force
lxc stop c3 --force
lxc restore c2 snap0
lxc restore c3 snap0
lxc restore c2 snap0-with_underscore
lxc restore c3 snap0-with_underscore
lxc start c2
lxc start c3
lxc delete --force c2
Expand Down
Loading