From 81d72bf5ea40c31e6dcf6e8a56b586443fbf8f53 Mon Sep 17 00:00:00 2001 From: Jason Dobies Date: Thu, 2 Nov 2023 14:04:02 -0400 Subject: [PATCH] Fixed issue where the modify script for raw images could not be executed As part of this, I refactored the writeBuildDirFile and writeCombustionDirFile methods to return the full name of the file written. They are meant to be utility functions to prevent the caller from having to manually assemble the full paths to common locations, and it makes sense that these calls would also tell the user what the filename ended up being. --- pkg/build/build.go | 8 ++++---- pkg/build/build_test.go | 6 ++++-- pkg/build/message.go | 2 +- pkg/build/raw.go | 8 +++++++- pkg/build/raw_test.go | 5 +++++ 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/pkg/build/build.go b/pkg/build/build.go index 1e76cb97..1d030a60 100644 --- a/pkg/build/build.go +++ b/pkg/build/build.go @@ -138,14 +138,14 @@ func (b *Builder) generateCombustionScript() error { return nil } -func (b *Builder) writeBuildDirFile(filename string, contents string, templateData any) error { +func (b *Builder) writeBuildDirFile(filename string, contents string, templateData any) (string, error) { destFilename := filepath.Join(b.eibBuildDir, filename) - return b.writeFile(destFilename, contents, templateData) + return destFilename, b.writeFile(destFilename, contents, templateData) } -func (b *Builder) writeCombustionFile(filename string, contents string, templateData any) error { +func (b *Builder) writeCombustionFile(filename string, contents string, templateData any) (string, error) { destFilename := filepath.Join(b.combustionDir, filename) - return b.writeFile(destFilename, contents, templateData) + return destFilename, b.writeFile(destFilename, contents, templateData) } func (b *Builder) writeFile(filename string, contents string, templateData any) error { diff --git a/pkg/build/build_test.go b/pkg/build/build_test.go index 92c3fc6c..5f515680 100644 --- a/pkg/build/build_test.go +++ b/pkg/build/build_test.go @@ -122,7 +122,7 @@ func TestWriteCombustionFile(t *testing.T) { testFilename := "combustion-file.sh" // Test - err = builder.writeCombustionFile(testFilename, testData, nil) + writtenFilename, err := builder.writeCombustionFile(testFilename, testData, nil) // Verify require.NoError(t, err) @@ -130,6 +130,7 @@ func TestWriteCombustionFile(t *testing.T) { expectedFilename := filepath.Join(builder.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 @@ -147,12 +148,13 @@ func TestWriteBuildDirFile(t *testing.T) { testFilename := "build-dir-file.sh" // Test - err = builder.writeBuildDirFile(testFilename, testData, nil) + writtenFilename, err := builder.writeBuildDirFile(testFilename, testData, nil) // Verify require.NoError(t, err) expectedFilename := filepath.Join(builder.eibBuildDir, testFilename) + require.Equal(t, expectedFilename, writtenFilename) foundData, err := os.ReadFile(expectedFilename) require.NoError(t, err) assert.Equal(t, testData, string(foundData)) diff --git a/pkg/build/message.go b/pkg/build/message.go index 32d586a3..8410d4c3 100644 --- a/pkg/build/message.go +++ b/pkg/build/message.go @@ -13,7 +13,7 @@ const ( var messageScript string func (b *Builder) configureMessage() error { - err := b.writeCombustionFile(messageScriptName, messageScript, nil) + _, err := b.writeCombustionFile(messageScriptName, messageScript, nil) if err != nil { return fmt.Errorf("copying script %s: %w", messageScriptName, err) } diff --git a/pkg/build/raw.go b/pkg/build/raw.go index 4387605a..6355ed10 100644 --- a/pkg/build/raw.go +++ b/pkg/build/raw.go @@ -3,6 +3,7 @@ package build import ( _ "embed" "fmt" + "os" "os/exec" "path/filepath" ) @@ -10,6 +11,7 @@ import ( const ( copyExec = "/bin/cp" modifyScriptName = "modify-raw-image.sh" + modifyScriptMode = 0o744 ) //go:embed scripts/modify-raw-image.sh.tpl @@ -54,10 +56,14 @@ func (b *Builder) writeModifyScript() error { CombustionDir: b.combustionDir, } - err := b.writeBuildDirFile(modifyScriptName, modifyRawImageScript, &values) + writtenFilename, err := b.writeBuildDirFile(modifyScriptName, modifyRawImageScript, &values) if err != nil { return fmt.Errorf("writing modification script %s: %w", modifyScriptName, err) } + err = os.Chmod(writtenFilename, modifyScriptMode) + if err != nil { + return fmt.Errorf("changing permissions on the modification script %s: %w", modifyScriptName, err) + } return nil } diff --git a/pkg/build/raw_test.go b/pkg/build/raw_test.go index 7150ce3b..ca27c9e6 100644 --- a/pkg/build/raw_test.go +++ b/pkg/build/raw_test.go @@ -1,6 +1,7 @@ package build import ( + "io/fs" "os" "path/filepath" "testing" @@ -66,6 +67,10 @@ func TestWriteModifyScript(t *testing.T) { foundBytes, err := os.ReadFile(expectedFilename) require.NoError(t, err) + stats, err := os.Stat(expectedFilename) + require.NoError(t, err) + assert.Equal(t, fs.FileMode(modifyScriptMode), stats.Mode()) + foundContents := string(foundBytes) assert.Contains(t, foundContents, "guestfish --rw -a config-dir/output-image") assert.Contains(t, foundContents, "copy-in "+builder.combustionDir)