Remove os.Umask from overlay.BuildOverlayIndir

Adds tests to ensure that the overlay image contains the correct permissions.

Signed-off-by: Jonathon Anderson <janderson@ciq.com>

CHANGELOG.md

@@ -45,7 +45,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Bump github.com/spf13/cobra from 1.7.0 to 1.8.0 #1166
 - Bump github.com/fatih/color from 1.15.0 to 1.17.0 #1224
 - Bump github.com/coreos/ignition/v2 from 2.15.0 to 2.19.0 #1239
-- Bump github.com/sassoftware/go-rpmutils from 0.2.0 to 0.4.0 #1217
 - Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 #1481
 - Bump google.golang.org/protobuf from 1.34.1 to 1.35.1 #1480
 - Bump golang.org/x/term from 0.20.0 to 0.25.0 #1476

go.mod

@@ -5,6 +5,7 @@ go 1.21.0
 require (
 	dario.cat/mergo v1.0.1
 	github.com/Masterminds/sprig/v3 v3.3.0
+	github.com/cavaliergopher/cpio v1.0.1
 	github.com/cheynewallace/tabby v1.1.1
 	github.com/containers/image/v5 v5.32.2
 	github.com/containers/storage v1.55.2
@@ -21,7 +22,6 @@ require (
 	github.com/opencontainers/image-spec v1.1.0
 	github.com/opencontainers/umoci v0.4.7
 	github.com/pkg/errors v0.9.1
-	github.com/sassoftware/go-rpmutils v0.4.0
 	github.com/spf13/cobra v1.8.1
 	github.com/stretchr/testify v1.9.0
 	github.com/talos-systems/go-smbios v0.1.1

go.sum

@@ -32,6 +32,8 @@ github.com/aws/aws-sdk-go v1.20.6/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN
 github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod h1:q/89r3U2H7sSsE2t6Kca0lfwTK8JdoNGS/yzM/4iH5I=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
+github.com/cavaliergopher/cpio v1.0.1 h1:KQFSeKmZhv0cr+kawA3a0xTQCU4QxXF1vhU7P7av2KM=
+github.com/cavaliergopher/cpio v1.0.1/go.mod h1:pBdaqQjnvXxdS/6CvNDwIANIFSP0xRKI16PX4xejRQc=
 github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=
 github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
@@ -310,8 +312,6 @@ github.com/rootless-containers/proto v0.1.0/go.mod h1:vgkUFZbQd0gcE/K/ZwtE4MYjZP
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
-github.com/sassoftware/go-rpmutils v0.4.0 h1:ojND82NYBxgwrV+mX1CWsd5QJvvEZTKddtCdFLPWhpg=
-github.com/sassoftware/go-rpmutils v0.4.0/go.mod h1:3goNWi7PGAT3/dlql2lv3+MSN5jNYPjT5mVcQcIsYzI=
 github.com/secure-systems-lab/go-securesystemslib v0.8.0 h1:mr5An6X45Kb2nddcFlbmfHkLguCE9laoZCUzEEpIZXA=
 github.com/secure-systems-lab/go-securesystemslib v0.8.0/go.mod h1:UH2VZVuJfCYR8WgMlCU1uFsOUU+KeyrTWcSS73NBOzU=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=

internal/pkg/overlay/overlay.go

@@ -10,7 +10,6 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
-	"syscall"
 	"text/template"
 
 	"github.com/Masterminds/sprig/v3"
@@ -251,9 +250,6 @@ func BuildOverlayIndir(nodeData node.Node, overlayNames []string, outputDir stri
 		return fmt.Errorf("overlay names contains illegal characters: %v", overlayNames)
 	}
 
-	// Temporarily set umask to 0000, so directories in the overlay retain permissions
-	defer syscall.Umask(syscall.Umask(0))
-
 	wwlog.Verbose("Processing node/overlays: %s/%s", nodeData.Id(), strings.Join(overlayNames, ","))
 	for _, overlayName := range overlayNames {
 		wwlog.Verbose("Building overlay %s for node %s in %s", overlayName, nodeData.Id(), outputDir)

internal/pkg/overlay/overlay_test.go

@@ -13,6 +13,8 @@ import (
 	"github.com/warewulf/warewulf/internal/pkg/node"
 	"github.com/warewulf/warewulf/internal/pkg/testenv"
 	"github.com/warewulf/warewulf/internal/pkg/util"
+
+	"github.com/cavaliergopher/cpio"
 )
 
 func Test_OverlayMethods(t *testing.T) {
@@ -259,6 +261,7 @@ func Test_BuildOverlay(t *testing.T) {
 		overlays    []string
 		image       string
 		contents    []string
+		perms       []int
 		hasFiles    bool
 	}{
 		{
@@ -292,6 +295,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1"},
 			image:       "o1.img",
 			contents:    []string{"o1.txt"},
+			perms:       []int{0644},
 		},
 		{
 			description: "if multiple overlays are specified without a node, then the combined overlay is built directly in the overlay directory",
@@ -300,6 +304,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1", "o2"},
 			image:       "o1-o2.img",
 			contents:    []string{"o1.txt", "o2.txt"},
+			perms:       []int{0644, 0644},
 		},
 		{
 			description: "if a single node overlay is specified, then the overlay is built in a node overlay directory",
@@ -308,6 +313,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1"},
 			image:       "node1/o1.img",
 			contents:    []string{"o1.txt"},
+			perms:       []int{0644},
 		},
 		{
 			description: "if multiple node overlays are specified, then the combined overlay is built in a node overlay directory",
@@ -316,6 +322,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1", "o2"},
 			image:       "node1/o1-o2.img",
 			contents:    []string{"o1.txt", "o2.txt"},
+			perms:       []int{0644, 0644},
 		},
 		{
 			description: "if no node system overlays are specified, then context pointed overlay is generated",
@@ -340,6 +347,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1"},
 			image:       "node1/__SYSTEM__.img",
 			contents:    []string{"o1.txt"},
+			perms:       []int{0644},
 		},
 		{
 			description: "if a single node runtime overlay is specified, then a runtime overlay image is generated in a node overlay directory",
@@ -348,6 +356,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1"},
 			image:       "node1/__RUNTIME__.img",
 			contents:    []string{"o1.txt"},
+			perms:       []int{0644},
 		},
 		{
 			description: "if multiple node system overlays are specified, then a system overlay image is generated with the contents of both overlays",
@@ -356,6 +365,7 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1", "o2"},
 			image:       "node1/__SYSTEM__.img",
 			contents:    []string{"o1.txt", "o2.txt"},
+			perms:       []int{0644, 0644},
 		},
 		{
 			description: "if multiple node runtime overlays are specified, then a runtime overlay image is generated with the contents of both overlays",
@@ -364,45 +374,48 @@ func Test_BuildOverlay(t *testing.T) {
 			overlays:    []string{"o1", "o2"},
 			image:       "node1/__RUNTIME__.img",
 			contents:    []string{"o1.txt", "o2.txt"},
+			perms:       []int{0644, 0644},
+		},
+		{
+			description: "validating altered permissions are retained",
+			nodeName:    "node1",
+			context:     "runtime",
+			overlays:    []string{"o3"},
+			image:       "node1/__RUNTIME__.img",
+			contents:    []string{"subdir", "subdir/o3.txt"},
+			perms:       []int{0700, 0600},
 		},
 	}
 
-	conf := warewulfconf.Get()
+	env := testenv.New(t)
-	overlayDir, overlayDirErr := os.MkdirTemp(os.TempDir(), "ww-test-overlay-*")
+	defer env.RemoveAll(t)
-	assert.NoError(t, overlayDirErr)
+
-	defer os.RemoveAll(overlayDir)
+	env.CreateFile(t, "var/lib/warewulf/overlays/o1/rootfs/o1.txt")
-	conf.Paths.WWOverlaydir = overlayDir
+	env.CreateFile(t, "var/lib/warewulf/overlays/o2/rootfs/o2.txt")
-	assert.NoError(t, os.Mkdir(path.Join(overlayDir, "o1"), 0700))
+	env.CreateFile(t, "var/lib/warewulf/overlays/o3/rootfs/subdir/o3.txt.ww")
-	{
+	env.Chmod(t, "var/lib/warewulf/overlays/o3/rootfs/subdir", 0700)
-		_, err := os.Create(path.Join(overlayDir, "o1", "o1.txt"))
+	env.Chmod(t, "var/lib/warewulf/overlays/o3/rootfs/subdir/o3.txt.ww", 0600)
-		assert.NoError(t, err)
-	}
-	assert.NoError(t, os.Mkdir(path.Join(overlayDir, "o2"), 0700))
-	{
-		_, err := os.Create(path.Join(overlayDir, "o2", "o2.txt"))
-		assert.NoError(t, err)
-	}
 
 	for _, tt := range tests {
 		nodeInfo := node.NewNode(tt.nodeName)
 		t.Run(tt.description, func(t *testing.T) {
-			provisionDir, provisionDirErr := os.MkdirTemp(os.TempDir(), "ww-test-provision-*")
-			assert.NoError(t, provisionDirErr)
-			defer os.RemoveAll(provisionDir)
-			conf.Paths.WWProvisiondir = provisionDir
-
 			err := BuildOverlay(nodeInfo, tt.context, tt.overlays)
 			assert.NoError(t, err)
 			if tt.image != "" {
-				image := path.Join(provisionDir, "overlays", tt.image)
+				image := env.GetPath(path.Join("srv/warewulf/overlays", tt.image))
 				assert.FileExists(t, image)
 				sort.Strings(tt.contents)
-				files, err := util.CpioFiles(image)
+				headers, err := readCpio(image)
 				assert.NoError(t, err)
-				sort.Strings(files)
+				assert.Equal(t, len(tt.contents), len(headers))
-				assert.Equal(t, tt.contents, files)
+				for i, file := range tt.contents {
+					assert.Equal(t, file, headers[file].Name)
+					if len(tt.perms) > i {
+						assert.Equal(t, tt.perms[i], int(headers[file].Mode.Perm()))
+					}
+				}
 			} else {
-				dirName := path.Join(provisionDir, "overlays", tt.nodeName)
+				dirName := env.GetPath(path.Join("srv/warewulf/overlays", tt.nodeName))
 				isEmpty := dirIsEmpty(t, dirName)
 				assert.True(t, isEmpty, "%v should be empty, but isn't", dirName)
 			}
@@ -630,3 +643,24 @@ func dirIsEmpty(t *testing.T, name string) bool {
 	t.Log(dirnames)
 	return false
 }
+
+func readCpio(name string) (headers map[string]*cpio.Header, err error) {
+	f, err := os.Open(name)
+	if err != nil {
+		return headers, err
+	}
+	defer f.Close()
+
+	reader := cpio.NewReader(f)
+	headers = make(map[string]*cpio.Header)
+	for {
+		header, err := reader.Next()
+		if err == io.EOF {
+			return headers, nil
+		}
+		if err != nil {
+			return headers, err
+		}
+		headers[header.Name] = header
+	}
+}

internal/pkg/testenv/testenv.go

@@ -112,6 +112,14 @@ func (env *TestEnv) MkdirAll(t *testing.T, dirName string) {
 	assert.NoError(t, err)
 }
 
+// Chmod changes the mode of fileName in the test environment, which must already exist.
+//
+// Asserts no errors occur.
+func (env *TestEnv) Chmod(t *testing.T, fileName string, mode int) {
+	err := os.Chmod(env.GetPath(fileName), os.FileMode(mode))
+	assert.NoError(t, err)
+}
+
 // WriteFile writes content to fileName, creating any necessary
 // intermediate directories relative to the test environment.
 //

internal/pkg/util/cpio.go

@@ -1,31 +0,0 @@
-package util
-
-import (
-	"io"
-	"os"
-
-	"github.com/sassoftware/go-rpmutils/cpio"
-)
-
-/*
-Opens cpio archive and returns the file list
-*/
-func CpioFiles(name string) (files []string, err error) {
-	f, err := os.Open(name)
-	if err != nil {
-		return files, err
-	}
-	defer f.Close()
-
-	reader := cpio.NewReader(f)
-	for {
-		header, err := reader.Next()
-		if err == io.EOF {
-			return files, nil
-		}
-		if err != nil {
-			return files, err
-		}
-		files = append(files, header.Filename())
-	}
-}