From 718c79c7cadcabba0afa77bb529e8b06608159ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Clerget?= Date: Fri, 24 Oct 2025 16:03:02 -0500 Subject: [PATCH] Fix ImageDelete API not returning error when checking if image is used by nodes/profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédric Clerget --- CHANGELOG.md | 1 + internal/pkg/api/image/image.go | 48 ++++++++++++------------ internal/pkg/warewulfd/api/image_test.go | 38 ++++++++++--------- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2efa1b1d..1c343974 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fix "address already in use" in `wwclient` when `secure: true`. #2009 - Use device names in netplan bonds. #2013 +- Fix ImageDelete API not returning error when checking if image is used by nodes/profiles ### Dependencies diff --git a/internal/pkg/api/image/image.go b/internal/pkg/api/image/image.go index d1c6be96..e599c473 100644 --- a/internal/pkg/api/image/image.go +++ b/internal/pkg/api/image/image.go @@ -5,6 +5,7 @@ import ( "os" "path" "path/filepath" + "slices" "strconv" "strings" @@ -58,37 +59,38 @@ func ImageDelete(cdp *wwapiv1.ImageDeleteParameter) (err error) { return fmt.Errorf("could not open nodeDB: %s", err) } -ARG_LOOP: - for i := 0; i < len(cdp.ImageNames); i++ { - //_, arg := range args { - imageName := cdp.ImageNames[i] - for _, n := range nodeDB.Nodes { - if n.ImageName == imageName { - wwlog.Error("image %s is in use by node %s, skipping", imageName, n.Id()) - continue ARG_LOOP - } - } - for _, p := range nodeDB.NodeProfiles { - if p.ImageName == imageName { - wwlog.Error("image %s is in use by profile %s, skipping", imageName, p.Id()) - continue ARG_LOOP - } - } - + // validate image names + for _, imageName := range cdp.ImageNames { if !image.ValidSource(imageName) { - wwlog.Error("image name is not a valid source: %s", imageName) - continue + return fmt.Errorf("image name is not valid source: %s", imageName) } + } + + // check if the deleted images are not used by nodes + for nodeName, node := range nodeDB.Nodes { + if slices.Contains(cdp.ImageNames, node.ImageName) { + return fmt.Errorf("image %s is in use by node %s, cannot delete", node.ImageName, nodeName) + } + } + + // check if the deleted images are not used by profiles + for profileName, profile := range nodeDB.NodeProfiles { + if slices.Contains(cdp.ImageNames, profile.ImageName) { + return fmt.Errorf("image %s is in use by profile %s, cannot delete", profile.ImageName, profileName) + } + } + + // delete images + for _, imageName := range cdp.ImageNames { err := image.DeleteSource(imageName) if err != nil { - wwlog.Error("could not remove source: %s", imageName) + return fmt.Errorf("could not remove source image %s: %w", imageName, err) } err = image.DeleteImage(imageName) if err != nil { - wwlog.Error("could not remove image files %s", imageName) + return fmt.Errorf("could not remove image file %s: %w", imageName, err) } - - fmt.Printf("Image has been deleted: %s\n", imageName) + wwlog.Info("Image %q has been deleted", imageName) } return diff --git a/internal/pkg/warewulfd/api/image_test.go b/internal/pkg/warewulfd/api/image_test.go index 683dcdcb..3e9151bc 100644 --- a/internal/pkg/warewulfd/api/image_test.go +++ b/internal/pkg/warewulfd/api/image_test.go @@ -16,7 +16,8 @@ import ( "github.com/warewulf/warewulf/internal/pkg/warewulfd" ) -var imageTests = map[string]struct { +var imageTests = []struct { + name string initFiles []string request func(serverURL string) (*http.Request, error) response string @@ -25,7 +26,8 @@ var imageTests = map[string]struct { resultAbsentFiles []string authenticate bool }{ - "test no authentication": { + { + name: "test no authentication", initFiles: []string{ "/var/lib/warewulf/chroots/test-image/rootfs/file", }, @@ -36,8 +38,8 @@ var imageTests = map[string]struct { status: http.StatusUnauthorized, authenticate: false, }, - - "test get all images": { + { + name: "test get all images", initFiles: []string{ "/var/lib/warewulf/chroots/test-image/rootfs/file", }, @@ -47,8 +49,8 @@ var imageTests = map[string]struct { response: `{"test-image": {"kernels":[], "size":0, "buildtime":0, "writable":true}}`, authenticate: true, }, - - "test get single image": { + { + name: "test get single image", initFiles: []string{ "/var/lib/warewulf/chroots/test-image/rootfs/file", }, @@ -58,8 +60,8 @@ var imageTests = map[string]struct { response: `{"kernels":[], "size":0, "buildtime":0, "writable":true}`, authenticate: true, }, - - "test build image": { + { + name: "test build image", initFiles: []string{ "/var/lib/warewulf/chroots/test-image/rootfs/file", }, @@ -73,8 +75,8 @@ var imageTests = map[string]struct { }, authenticate: true, }, - - "test rename image": { + { + name: "test rename image", initFiles: []string{ "/var/lib/warewulf/chroots/test-image/rootfs/file", }, @@ -84,15 +86,15 @@ var imageTests = map[string]struct { response: `{"kernels":[], "size":512, "buildtime":"<>", "writable":true}`, authenticate: true, }, - - "test delete image": { + { + name: "test delete image", initFiles: []string{ "/var/lib/warewulf/chroots/new-image/rootfs/file", }, request: func(serverURL string) (*http.Request, error) { return http.NewRequest(http.MethodDelete, serverURL+"/api/images/new-image", nil) }, - response: `{"kernels":[], "size":0, "buildtime":"<>", "writable":true}`, + response: `{"kernels":[], "size":512, "buildtime":"<>", "writable":true}`, resultAbsentFiles: []string{ "/var/lib/warewulf/chroots/new-image", "/srv/warewulf/images/new-image.img", @@ -108,13 +110,13 @@ users: - name: admin password hash: $2b$05$5QVWDpiWE7L4SDL9CYdi3O/l6HnbNOLoXgY2sa1bQQ7aSBKdSqvsC ` + env := testenv.New(t) + defer env.RemoveAll() - for name, tt := range imageTests { - t.Run(name, func(t *testing.T) { - warewulfd.SetNoDaemon() - env := testenv.New(t) - defer env.RemoveAll() + warewulfd.SetNoDaemon() + for _, tt := range imageTests { + t.Run(tt.name, func(t *testing.T) { // Create test files for _, fileName := range tt.initFiles { env.CreateFile(fileName)