Split templating from file IO operations (#45)

* Move WriteFile calls out of Builder helper functions Signed-off-by: Atanas Dinov <[email protected]> * Define ExecutablePerms and NonExecutablePerms defaults Signed-off-by: Atanas Dinov <[email protected]> * Extract WriteTemplate from WriteFile Signed-off-by: Atanas Dinov <[email protected]> * Drop WriteFile Signed-off-by: Atanas Dinov <[email protected]> * Set permissions in CopyFile Signed-off-by: Atanas Dinov <[email protected]> * Extract template package Signed-off-by: Atanas Dinov <[email protected]> * Remove hardcoded permissions in tests Signed-off-by: Atanas Dinov <[email protected]> * Use more verbose error wrapping Signed-off-by: Atanas Dinov <[email protected]> * Resolve conflicts Signed-off-by: Atanas Dinov <[email protected]> --------- Signed-off-by: Atanas Dinov <[email protected]>
suse-edge · Nov 21, 2023 · 9fd31c8 · 9fd31c8
1 parent 89f1468
commit 9fd31c8
Show file tree

Hide file tree

Showing 16 changed files with 171 additions and 181 deletions.
diff --git a/pkg/build/build.go b/pkg/build/build.go
@@ -8,7 +8,6 @@ import (
 	"slices"
 
 	"github.com/suse-edge/edge-image-builder/pkg/config"
-	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 //go:embed scripts/script_base.sh
@@ -94,14 +93,12 @@ func (b *Builder) generateCombustionScript() error {
 	return nil
 }
 
-func (b *Builder) writeBuildDirFile(filename string, contents string, templateData any) (string, error) {
-	destFilename := filepath.Join(b.context.BuildDir, filename)
-	return destFilename, fileio.WriteFile(destFilename, contents, templateData)
+func (b *Builder) generateBuildDirFilename(filename string) string {
+	return filepath.Join(b.context.BuildDir, filename)
 }
 
-func (b *Builder) writeCombustionFile(filename string, contents string, templateData any) (string, error) {
-	destFilename := filepath.Join(b.context.CombustionDir, filename)
-	return destFilename, fileio.WriteFile(destFilename, contents, templateData)
+func (b *Builder) generateCombustionDirFilename(filename string) string {
+	return filepath.Join(b.context.CombustionDir, filename)
 }
 
 func (b *Builder) registerCombustionScript(scriptName string) {

diff --git a/pkg/build/build_test.go b/pkg/build/build_test.go
@@ -42,7 +42,7 @@ func TestGenerateCombustionScript(t *testing.T) {
 	assert.Equal(t, "foo.sh", builder.combustionScripts[1])
 }
 
-func TestWriteCombustionFile(t *testing.T) {
+func TestGenerateCombustionDirFilename(t *testing.T) {
 	// Setup
 	context, err := NewContext("", "", true)
 	require.NoError(t, err)
@@ -54,26 +54,17 @@ func TestWriteCombustionFile(t *testing.T) {
 		context: context,
 	}
 
-	testData := "Edge Image Builder"
 	testFilename := "combustion-file.sh"
 
 	// Test
-	writtenFilename, err := builder.writeCombustionFile(testFilename, testData, nil)
+	filename := builder.generateCombustionDirFilename(testFilename)
 
 	// Verify
-	require.NoError(t, err)
-
 	expectedFilename := filepath.Join(context.CombustionDir, testFilename)
-	foundData, err := os.ReadFile(expectedFilename)
-	require.NoError(t, err)
-	assert.Equal(t, expectedFilename, writtenFilename)
-	assert.Equal(t, testData, string(foundData))
-
-	// Make sure the file isn't automatically added to the combustion scripts list
-	require.Equal(t, 0, len(builder.combustionScripts))
+	assert.Equal(t, expectedFilename, filename)
 }
 
-func TestWriteBuildDirFile(t *testing.T) {
+func TestGenerateBuildDirFilename(t *testing.T) {
 	// Setup
 	context, err := NewContext("", "", true)
 	require.NoError(t, err)
@@ -85,18 +76,12 @@ func TestWriteBuildDirFile(t *testing.T) {
 		context: context,
 	}
 
-	testData := "Edge Image Builder"
 	testFilename := "build-dir-file.sh"
 
 	// Test
-	writtenFilename, err := builder.writeBuildDirFile(testFilename, testData, nil)
+	filename := builder.generateBuildDirFilename(testFilename)
 
 	// Verify
-	require.NoError(t, err)
-
 	expectedFilename := filepath.Join(context.BuildDir, testFilename)
-	require.Equal(t, expectedFilename, writtenFilename)
-	foundData, err := os.ReadFile(expectedFilename)
-	require.NoError(t, err)
-	assert.Equal(t, testData, string(foundData))
+	require.Equal(t, expectedFilename, filename)
 }
diff --git a/pkg/build/grub.go b/pkg/build/grub.go
@@ -1,11 +1,11 @@
 package build
 
 import (
-	"bytes"
 	_ "embed"
 	"fmt"
 	"strings"
-	"text/template"
+
+	"github.com/suse-edge/edge-image-builder/pkg/template"
 )
 
 //go:embed scripts/grub/guestfish-snippet.tpl
@@ -19,24 +19,17 @@ func (b *Builder) generateGRUBGuestfishCommands() (string, error) {
 		return "", nil
 	}
 
-	tmpl, err := template.New("guestfish-snippet").Parse(guestfishSnippet)
-	if err != nil {
-		return "", fmt.Errorf("building template for GRUB guestfish snippet: %w", err)
-	}
-
 	argLine := strings.Join(b.imageConfig.OperatingSystem.KernelArgs, " ")
 	values := struct {
 		KernelArgs string
 	}{
 		KernelArgs: argLine,
 	}
 
-	var buff bytes.Buffer
-	err = tmpl.Execute(&buff, values)
+	snippet, err := template.Parse("guestfish-snippet", guestfishSnippet, values)
 	if err != nil {
-		return "", fmt.Errorf("applying GRUB guestfish snippet: %w", err)
+		return "", fmt.Errorf("parsing GRUB guestfish snippet: %w", err)
 	}
 
-	snippet := buff.String()
 	return snippet, nil
 }
diff --git a/pkg/build/message.go b/pkg/build/message.go
@@ -3,6 +3,9 @@ package build
 import (
 	_ "embed"
 	"fmt"
+	"os"
+
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 const (
@@ -13,8 +16,9 @@ const (
 var messageScript string
 
 func (b *Builder) configureMessage() error {
-	_, err := b.writeCombustionFile(messageScriptName, messageScript, nil)
-	if err != nil {
+	filename := b.generateCombustionDirFilename(messageScriptName)
+
+	if err := os.WriteFile(filename, []byte(messageScript), fileio.ExecutablePerms); err != nil {
 		return fmt.Errorf("copying script %s: %w", messageScriptName, err)
 	}
 	b.registerCombustionScript(messageScriptName)

diff --git a/pkg/build/raw.go b/pkg/build/raw.go
@@ -6,16 +6,18 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
+	"github.com/suse-edge/edge-image-builder/pkg/template"
 )
 
 const (
 	copyExec         = "/bin/cp"
 	modifyScriptName = "modify-raw-image.sh"
-	modifyScriptMode = 0o744
 )
 
 //go:embed scripts/modify-raw-image.sh.tpl
-var modifyRawImageScript string
+var modifyRawImageTemplate string
 
 func (b *Builder) buildRawImage() error {
 	cmd := b.createRawImageCopyCommand()
@@ -66,13 +68,14 @@ func (b *Builder) writeModifyScript() error {
 		ConfigureGRUB: grubConfiguration,
 	}
 
-	writtenFilename, err := b.writeBuildDirFile(modifyScriptName, modifyRawImageScript, &values)
+	data, err := template.Parse(modifyScriptName, modifyRawImageTemplate, &values)
 	if err != nil {
-		return fmt.Errorf("writing modification script %s: %w", modifyScriptName, err)
+		return fmt.Errorf("parsing %s template: %w", modifyScriptName, err)
 	}
-	err = os.Chmod(writtenFilename, modifyScriptMode)
-	if err != nil {
-		return fmt.Errorf("changing permissions on the modification script %s: %w", modifyScriptName, err)
+
+	filename := b.generateBuildDirFilename(modifyScriptName)
+	if err = os.WriteFile(filename, []byte(data), fileio.ExecutablePerms); err != nil {
+		return fmt.Errorf("writing modification script %s: %w", modifyScriptName, err)
 	}
 
 	return nil

diff --git a/pkg/build/raw_test.go b/pkg/build/raw_test.go
@@ -1,14 +1,14 @@
 package build
 
 import (
-	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/suse-edge/edge-image-builder/pkg/config"
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 func TestCreateRawImageCopyCommand(t *testing.T) {
@@ -70,7 +70,7 @@ func TestWriteModifyScript(t *testing.T) {
 
 	stats, err := os.Stat(expectedFilename)
 	require.NoError(t, err)
-	assert.Equal(t, fs.FileMode(modifyScriptMode), stats.Mode())
+	assert.Equal(t, fileio.ExecutablePerms, stats.Mode())
 
 	foundContents := string(foundBytes)
 	assert.Contains(t, foundContents, "guestfish --rw -a config-dir/output-image")

diff --git a/pkg/build/rpm.go b/pkg/build/rpm.go
@@ -5,10 +5,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-
 	"strings"
 
 	"github.com/suse-edge/edge-image-builder/pkg/fileio"
+	"github.com/suse-edge/edge-image-builder/pkg/template"
 )
 
 const (
@@ -75,7 +75,7 @@ func copyRPMs(rpmSourceDir string, rpmDestDir string, rpmFileNames []string) err
 		sourcePath := filepath.Join(rpmSourceDir, rpm)
 		destPath := filepath.Join(rpmDestDir, rpm)
 
-		err := fileio.CopyFile(sourcePath, destPath)
+		err := fileio.CopyFile(sourcePath, destPath, fileio.NonExecutablePerms)
 		if err != nil {
 			return fmt.Errorf("copying file %s: %w", sourcePath, err)
 		}
@@ -91,13 +91,15 @@ func (b *Builder) writeRPMScript(rpmFileNames []string) error {
 		RPMs: strings.Join(rpmFileNames, " "),
 	}
 
-	writtenFilename, err := b.writeCombustionFile(modifyRPMScriptName, modifyRPMScript, &values)
+	data, err := template.Parse(modifyRPMScriptName, modifyRPMScript, &values)
 	if err != nil {
-		return fmt.Errorf("writing RPM script: %w", err)
+		return fmt.Errorf("parsing RPM script template: %w", err)
 	}
-	err = os.Chmod(writtenFilename, modifyScriptMode)
+
+	filename := b.generateCombustionDirFilename(modifyRPMScriptName)
+	err = os.WriteFile(filename, []byte(data), fileio.ExecutablePerms)
 	if err != nil {
-		return fmt.Errorf("adjusting permissions: %w", err)
+		return fmt.Errorf("writing RPM script: %w", err)
 	}
 
 	b.registerCombustionScript(modifyRPMScriptName)

diff --git a/pkg/build/rpm_test.go b/pkg/build/rpm_test.go
@@ -1,13 +1,13 @@
 package build
 
 import (
-	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 func setupRPMSourceDir(t *testing.T, addFiles bool) (tmpDir string, rpmSourceDir string, teardown func()) {
@@ -136,7 +136,7 @@ func TestWriteRPMScript(t *testing.T) {
 
 	stats, err := os.Stat(expectedFilename)
 	require.NoError(t, err)
-	assert.Equal(t, fs.FileMode(modifyScriptMode), stats.Mode())
+	assert.Equal(t, fileio.ExecutablePerms, stats.Mode())
 
 	foundContents := string(foundBytes)
 	assert.Contains(t, foundContents, "rpm1.rpm")

diff --git a/pkg/build/scripts.go b/pkg/build/scripts.go
@@ -10,7 +10,6 @@ import (
 
 const (
 	scriptsDir = "scripts"
-	scriptMode = 0o744
 )
 
 func (b *Builder) configureScripts() error {
@@ -39,14 +38,10 @@ func (b *Builder) configureScripts() error {
 		copyMe := filepath.Join(fullScriptsDir, scriptEntry.Name())
 		copyTo := filepath.Join(b.context.CombustionDir, scriptEntry.Name())
 
-		err = fileio.CopyFile(copyMe, copyTo)
+		err = fileio.CopyFile(copyMe, copyTo, fileio.ExecutablePerms)
 		if err != nil {
 			return fmt.Errorf("copying script to %s: %w", copyTo, err)
 		}
-		err = os.Chmod(copyTo, scriptMode)
-		if err != nil {
-			return fmt.Errorf("modifying permissions for script %s: %w", copyTo, err)
-		}
 
 		// Make sure the combustion main script will execute the newly copied script
 		b.registerCombustionScript(scriptEntry.Name())

diff --git a/pkg/build/scripts_test.go b/pkg/build/scripts_test.go
@@ -1,13 +1,13 @@
 package build
 
 import (
-	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 func TestConfigureScripts(t *testing.T) {
@@ -56,7 +56,7 @@ func TestConfigureScripts(t *testing.T) {
 		fullEntryPath := filepath.Join(builder.context.CombustionDir, entry.Name())
 		stats, err := os.Stat(fullEntryPath)
 		require.NoError(t, err)
-		assert.Equal(t, fs.FileMode(scriptMode), stats.Mode())
+		assert.Equal(t, fileio.ExecutablePerms, stats.Mode())
 	}
 
 	// - make sure entries were added to the combustion scripts list, so they are

diff --git a/pkg/build/users.go b/pkg/build/users.go
@@ -4,11 +4,13 @@ import (
 	_ "embed"
 	"fmt"
 	"os"
+
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
+	"github.com/suse-edge/edge-image-builder/pkg/template"
 )
 
 const (
 	usersScriptName = "add-users.sh"
-	userScriptMode  = 0o744
 )
 
 //go:embed scripts/users/add-users.sh.tpl
@@ -20,13 +22,15 @@ func (b *Builder) configureUsers() error {
 		return nil
 	}
 
-	filename, err := b.writeCombustionFile(usersScriptName, usersScript, b.imageConfig.OperatingSystem.Users)
+	data, err := template.Parse(usersScriptName, usersScript, b.imageConfig.OperatingSystem.Users)
 	if err != nil {
-		return fmt.Errorf("writing %s to the combustion directory: %w", usersScriptName, err)
+		return fmt.Errorf("parsing users script template: %w", err)
 	}
-	err = os.Chmod(filename, userScriptMode)
+
+	filename := b.generateCombustionDirFilename(usersScriptName)
+	err = os.WriteFile(filename, []byte(data), fileio.ExecutablePerms)
 	if err != nil {
-		return fmt.Errorf("modifying permissions for script %s: %w", filename, err)
+		return fmt.Errorf("writing %s to the combustion directory: %w", usersScriptName, err)
 	}
 
 	b.registerCombustionScript(usersScriptName)

diff --git a/pkg/build/users_test.go b/pkg/build/users_test.go
@@ -1,14 +1,14 @@
 package build
 
 import (
-	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/suse-edge/edge-image-builder/pkg/config"
+	"github.com/suse-edge/edge-image-builder/pkg/fileio"
 )
 
 func TestConfigureUsers(t *testing.T) {
@@ -61,7 +61,7 @@ func TestConfigureUsers(t *testing.T) {
 
 	stats, err := os.Stat(expectedFilename)
 	require.NoError(t, err)
-	assert.Equal(t, fs.FileMode(userScriptMode), stats.Mode())
+	assert.Equal(t, fileio.ExecutablePerms, stats.Mode())
 
 	foundContents := string(foundBytes)