Fix image history file. (#356)

The logic to detect if an image is on a read-only mount is broken. This
change fixes it.

Also, add some tests to ensure the image history file is being added to
the image.

Author: cwize1

toolkit/tools/internal/testutils/.gitignore

@@ -0,0 +1 @@
+/testrpms/

toolkit/tools/pkg/imagecustomizerlib/customizepackages_test.go

@@ -62,6 +62,8 @@ func TestCustomizeImagePackagesAddOfflineDir(t *testing.T) {
 		"/usr/bin/jq",
 	)
 
+	verifyImageHistoryFile(t, 1, config, imageConnection.Chroot().RootDir())
+
 	err = imageConnection.CleanClose()
 	if !assert.NoError(t, err) {
 		return
@@ -105,6 +107,8 @@ func TestCustomizeImagePackagesAddOfflineDir(t *testing.T) {
 		"/usr/bin/jq",
 		"/usr/bin/go",
 	)
+
+	verifyImageHistoryFile(t, 2, config, imageConnection.Chroot().RootDir())
 }
 
 func copyRpms(sourceDir string, targetDir string, excludePrefixes []string) error {
@@ -356,6 +360,8 @@ func TestCustomizeImagePackagesSnapshotTime(t *testing.T) {
 		"snapshotTime %s should install jq version %s, but got: %s", snapshotTime, expectedVersion, jqVersionOutput)
 
 	ensureFilesNotExist(t, imageConnection, customTdnfConfRelPath)
+
+	verifyImageHistoryFile(t, 1, config, imageConnection.Chroot().RootDir())
 }
 
 func TestCustomizeImagePackagesCliSnapshotTimeOverridesConfigFile(t *testing.T) {
@@ -405,6 +411,8 @@ func TestCustomizeImagePackagesCliSnapshotTimeOverridesConfigFile(t *testing.T)
 		"snapshotTime %s should install jq version %s, but got: %s", snapshotTimeCLI, expectedVersion, jqVersionOutput)
 
 	ensureFilesNotExist(t, imageConnection, customTdnfConfRelPath)
+
+	verifyImageHistoryFile(t, 1, config, imageConnection.Chroot().RootDir())
 }
 
 func TestCustomizeImagePackagesSnapshotTimeWithoutPreviewFlagFails(t *testing.T) {

toolkit/tools/pkg/imagecustomizerlib/imagehistory.go

@@ -33,8 +33,8 @@ const (
 func addImageHistory(ctx context.Context, imageChroot *safechroot.Chroot, imageUuid string,
 	baseConfigPath string, toolVersion string, buildTime string, config *imagecustomizerapi.Config,
 ) error {
-	canWriteHistoryFile := isPathOnReadOnlyMount(customizerLoggingDir, imageChroot)
-	if !canWriteHistoryFile {
+	cannotWriteHistoryFile := isPathOnReadOnlyMount(customizerLoggingDir, imageChroot)
+	if cannotWriteHistoryFile {
 		return nil
 	}
 

toolkit/tools/pkg/imagecustomizerlib/imagehistory_test.go

@@ -61,7 +61,15 @@ func TestAddImageHistory(t *testing.T) {
 	assert.NotEqual(t, allHistory[0].ImageUuid, allHistory[1].ImageUuid, "imageUuid should be different for each entry")
 }
 
-func verifyHistoryFile(t *testing.T, expectedEntries int, expectedUuid string, expectedVersion string, expectedDate string, config imagecustomizerapi.Config, historyFilePath string) (allHistory []ImageHistory) {
+func verifyImageHistoryFile(t *testing.T, expectedEntries int, config imagecustomizerapi.Config, rootPath string,
+) (allHistory []ImageHistory) {
+	return verifyHistoryFile(t, expectedEntries, "", ToolVersion, "", config,
+		filepath.Join(rootPath, customizerLoggingDir, historyFileName))
+}
+
+func verifyHistoryFile(t *testing.T, expectedEntries int, expectedUuid string, expectedVersion string,
+	expectedDate string, config imagecustomizerapi.Config, historyFilePath string,
+) (allHistory []ImageHistory) {
 	exists, err := file.PathExists(historyFilePath)
 	assert.NoError(t, err, "error checking history file existence")
 	assert.True(t, exists, "history file should exist")
@@ -75,9 +83,13 @@ func verifyHistoryFile(t *testing.T, expectedEntries int, expectedUuid string, e
 
 	// Verify the last entry content
 	entry := allHistory[expectedEntries-1]
-	assert.Equal(t, expectedUuid, entry.ImageUuid, "imageUuid should match")
+	if expectedUuid != "" {
+		assert.Equal(t, expectedUuid, entry.ImageUuid, "imageUuid should match")
+	}
 	assert.Equal(t, expectedVersion, entry.ToolVersion, "toolVersion should match")
-	assert.Equal(t, expectedDate, entry.BuildTime, "buildTime should match")
+	if expectedDate != "" {
+		assert.Equal(t, expectedDate, entry.BuildTime, "buildTime should match")
+	}
 	// Since the config is modified its entirety won't be an exact match; picking one consistent field to verify
 	assert.Equal(t, config.OS.BootLoader.ResetType, entry.Config.OS.BootLoader.ResetType, "config bootloader reset type should match")
 

toolkit/tools/pkg/imagecustomizerlib/readonlymounts.go

@@ -5,6 +5,7 @@ package imagecustomizerlib
 
 import (
 	"path/filepath"
+	"strings"
 
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/safechroot"
 	"golang.org/x/sys/unix"
@@ -15,8 +16,8 @@ func isPathOnReadOnlyMount(chrootPath string, imageChroot *safechroot.Chroot) bo
 
 	mounts := imageChroot.GetMountPoints()
 	for _, mount := range mounts {
-		_, err := filepath.Rel(mount.GetTarget(), chrootPath)
-		if err != nil {
+		relativePath, err := filepath.Rel(mount.GetTarget(), chrootPath)
+		if err != nil || strings.HasPrefix(relativePath, "../") {
 			// Path is not relative to the mount.
 			continue
 		}