From f2d68a41a534ed6df1a722767954097b841321fa Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Mon, 17 Nov 2025 10:40:35 +0000 Subject: [PATCH 1/7] init update --- .../imagecustomizerlib/baseconfigs_test.go | 101 +++++++++++++----- .../imagecustomizerlib/configvalidation.go | 64 +++++++++++ .../pkg/imagecustomizerlib/customizeos.go | 2 +- .../pkg/imagecustomizerlib/imagecustomizer.go | 4 +- .../pkg/imagecustomizerlib/resolvedconfig.go | 3 + ...erarchical-config-storage-verity-base.yaml | 46 ++++++++ .../hierarchical-config-storage-verity.yaml | 52 +++++++++ 7 files changed, 245 insertions(+), 27 deletions(-) create mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml create mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go index 5aaba7de93..be766b56be 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go @@ -57,36 +57,13 @@ func TestBaseConfigsInputAndOutput(t *testing.T) { } func TestBaseConfigsFullRun(t *testing.T) { - baseImage, baseImageInfo := checkSkipForCustomizeDefaultImage(t) - if baseImageInfo.Version == baseImageVersionAzl2 { - t.Skip("'systemd-boot' is not available on Azure Linux 2.0") - } - - ukifyExists, err := file.CommandExists("ukify") - assert.NoError(t, err) - if !ukifyExists { - t.Skip("The 'ukify' command is not available") - } - - if runtime.GOARCH == "arm64" { - t.Skip("systemd-boot not available on AZL3 ARM64 yet") - } - testTmpDir := filepath.Join(tmpDir, "TestBaseConfigsFullRun") defer os.RemoveAll(testTmpDir) - buildDir := filepath.Join(testTmpDir, "build") outImageFilePath := filepath.Join(testTmpDir, "image.raw") - currentConfigFile := filepath.Join(testDir, "hierarchical-config.yaml") - err = CustomizeImageWithConfigFile(t.Context(), buildDir, currentConfigFile, baseImage, nil, - outImageFilePath, "raw", true, "") - if !assert.NoError(t, err) { - return - } - - assert.FileExists(t, outImageFilePath) + customizeImageOrSkip(t, buildDir, currentConfigFile, outImageFilePath) mountPoints := []testutils.MountPoint{ { @@ -245,3 +222,79 @@ func TestBaseConfigsFullRun(t *testing.T) { assert.NotContains(t, grubCfgContents, "rd.info") assert.Contains(t, grubCfgContents, "console=tty0 console=ttyS0") } + +func TestStorageVerityOverride(t *testing.T) { + testTempDir := filepath.Join(tmpDir, "TestStorageVerityOverride") + defer os.RemoveAll(testTempDir) + + buildDir := filepath.Join(testTempDir, "build") + outImageFilePath := filepath.Join(testTempDir, "image.raw") + currentConfigFile := filepath.Join(testDir, "hierarchical-config-storage-verity.yaml") + + baseImageInfo := customizeImageOrSkip(t, buildDir, currentConfigFile, outImageFilePath) + + mountPoints := []testutils.MountPoint{ + { + PartitionNum: 1, + Path: "/boot/efi", + FileSystemType: "vfat", + }, + { + PartitionNum: 2, + Path: "/boot", + FileSystemType: "ext4", + }, + } + + imageConnection, err := testutils.ConnectToImage(buildDir, outImageFilePath, true, mountPoints) + if !assert.NoError(t, err) { + return + } + defer imageConnection.Close() + + partitions, err := getDiskPartitionsMap(imageConnection.Loopback().DevicePath()) + assert.NoError(t, err, "get disk partitions") + + bootPath := filepath.Join(imageConnection.Chroot().RootDir(), "/boot") + rootDevice := testutils.PartitionDevPath(imageConnection, 3) + hashDevice := testutils.PartitionDevPath(imageConnection, 4) + + verifyVerityGrub(t, + bootPath, + rootDevice, + hashDevice, + "PARTUUID="+partitions[3].PartUuid, + "PARTUUID="+partitions[4].PartUuid, + "root", + "rd.info", + baseImageInfo, + "panic-on-corruption", + ) +} + +func customizeImageOrSkip(t *testing.T, buildDir, configFile, outImageFilePath string) testBaseImageInfo { + baseImage, baseImageInfo := checkSkipForCustomizeDefaultImage(t) + if baseImageInfo.Version == baseImageVersionAzl2 { + t.Skip("'systemd-boot' is not available on Azure Linux 2.0") + } + + ukifyExists, err := file.CommandExists("ukify") + assert.NoError(t, err) + if !ukifyExists { + t.Skip("The 'ukify' command is not available") + } + + if runtime.GOARCH == "arm64" { + t.Skip("systemd-boot not available on AZL3 ARM64 yet") + } + + err = CustomizeImageWithConfigFile(t.Context(), buildDir, configFile, baseImage, nil, + outImageFilePath, "raw", true, "") + if !assert.NoError(t, err) { + t.FailNow() + } + + assert.FileExists(t, outImageFilePath) + + return baseImageInfo +} diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 3cce8d8436..6d928f4217 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -120,8 +120,15 @@ func ValidateConfig(ctx context.Context, baseConfigPath string, config *imagecus return nil, err } + // Validate cross-config storage rules + err = validateStorage(rc.ConfigChain) + if err != nil { + return nil, fmt.Errorf("invalid storage inheritance:\n%w", err) + } + rc.Hostname = resolveHostname(rc.ConfigChain) rc.SELinux = resolveSeLinux(rc.ConfigChain) + rc.Storage = resolveStorage(rc.ConfigChain) rc.BootLoader.ResetType = resolveBootLoaderResetType(rc.ConfigChain) rc.Uki = resolveUki(rc.ConfigChain) rc.KernelCommandLine = resolveKernelCommandLine(rc.ConfigChain) @@ -511,6 +518,34 @@ func validateIsoPxeCustomization(rc *ResolvedConfig) error { return nil } +func validateStorage(configChain []*ConfigWithBasePath) error { + var baseHasDisks bool + + for i, configWithBase := range configChain { + storage := configWithBase.Config.Storage + + hasDisks := len(storage.Disks) > 0 + hasResetUUID := storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault + hasReinitVerity := storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault + + if baseHasDisks { + if hasResetUUID { + return fmt.Errorf("invalid storage inheritance at layer %d: current config sets .storage.resetPartitionsUuidsType after a base config defined .storage.disks", i) + } + if hasReinitVerity { + return fmt.Errorf("invalid storage inheritance at layer %d: current config sets .storage.reinitializeVerity after a base config defined .storage.disks", i) + } + } + + // For next iteration + if hasDisks { + baseHasDisks = true + } + } + + return nil +} + func resolveOutputArtifacts(configChain []*ConfigWithBasePath) *imagecustomizerapi.Artifacts { var artifacts *imagecustomizerapi.Artifacts @@ -625,3 +660,32 @@ func resolveKernelCommandLine(configChain []*ConfigWithBasePath) imagecustomizer ExtraCommandLine: mergedArgs, } } + +func resolveStorage(configChain []*ConfigWithBasePath) imagecustomizerapi.Storage { + var resolvedStorage imagecustomizerapi.Storage + + for _, configWithBase := range slices.Backward(configChain) { + storage := configWithBase.Config.Storage + + // .storage.disks - override + if len(storage.Disks) > 0 { + resolvedStorage.Disks = storage.Disks + resolvedStorage.BootType = storage.BootType + resolvedStorage.FileSystems = storage.FileSystems + resolvedStorage.Verity = storage.Verity + return resolvedStorage + } + + // .storage.resetPartitionsUuidsType - override + if storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault { + resolvedStorage.ResetPartitionsUuidsType = storage.ResetPartitionsUuidsType + } + + // .storage.reinitializeVerity - override + if storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault { + resolvedStorage.ReinitializeVerity = storage.ReinitializeVerity + } + } + + return resolvedStorage +} diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizeos.go b/toolkit/tools/pkg/imagecustomizerlib/customizeos.go index fd4a9d035f..3d414af93b 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizeos.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizeos.go @@ -121,7 +121,7 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection overlayUpdated = overlayUpdated || updated } - verityUpdated, err := enableVerityPartition(ctx, rc.Config.Storage.Verity, imageChroot, distroHandler) + verityUpdated, err := enableVerityPartition(ctx, rc.Storage.Verity, imageChroot, distroHandler) if err != nil { return err } diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go index 877f58c987..1871d9a7c6 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go @@ -524,7 +524,7 @@ func customizeOSContents(ctx context.Context, rc *ResolvedConfig) (imageMetadata } } - if len(rc.Config.Storage.Verity) > 0 || len(im.baseImageVerityMetadata) > 0 { + if len(rc.Storage.Verity) > 0 || len(im.baseImageVerityMetadata) > 0 { // Customize image for dm-verity, setting up verity metadata and security features. verityMetadata, err := customizeVerityImageHelper(ctx, rc.BuildDirAbs, rc.Config, rc.RawImageFile, partIdToPartUuid, shrinkPartitions, im.baseImageVerityMetadata, readonlyPartUuids, partUuidToFstabEntry) @@ -678,7 +678,7 @@ func customizeImageHelper(ctx context.Context, rc *ResolvedConfig, partitionsCus ) (map[string]diskutils.FstabEntry, []verityDeviceMetadata, []string, string, error) { logger.Log.Debugf("Customizing OS") - readOnlyVerity := rc.Config.Storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeAll + readOnlyVerity := rc.Storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeAll imageConnection, partUuidToFstabEntry, baseImageVerityMetadata, readonlyPartUuids, err := connectToExistingImage( ctx, rc.RawImageFile, rc.BuildDirAbs, "imageroot", true, false, readOnlyVerity, false) diff --git a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go index c1cb3eaa39..c60456dd0a 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go +++ b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go @@ -48,6 +48,9 @@ type ResolvedConfig struct { // SELinux SELinux imagecustomizerapi.SELinux + // Storage + Storage imagecustomizerapi.Storage + // Bootloader BootLoader imagecustomizerapi.BootLoader diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml new file mode 100644 index 0000000000..4f81ef2a27 --- /dev/null +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml @@ -0,0 +1,46 @@ +storage: + bootType: efi + disks: + - partitionTableType: gpt + partitions: + - id: esp + type: esp + size: 8M + + - id: usr + size: 2G + + - id: usrhash + size: 100M + + filesystems: + - deviceId: esp + type: fat32 + mountPoint: + path: /boot/efi + options: umask=0077 + + - deviceId: verityusr + type: ext4 + mountPoint: + path: /usr + options: ro + + verity: + - id: verityusr + name: usr + dataDeviceId: usr + hashDeviceId: usrhash + +input: + image: + path: testimages/empty-base.vhdx + +output: + image: + path: ./out/storage-reinit-verity.vhdx + format: vhdx + +os: + bootloader: + resetType: hard-reset diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml new file mode 100644 index 0000000000..417b74ae3a --- /dev/null +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml @@ -0,0 +1,52 @@ +storage: + bootType: efi + disks: + - partitionTableType: gpt + partitions: + - id: esp + type: esp + size: 8M + + - id: boot + size: 1G + + - id: root + size: 2G + + - id: roothash + size: 100M + + filesystems: + - deviceId: esp + type: fat32 + mountPoint: + path: /boot/efi + options: umask=0077 + + - deviceId: boot + type: ext4 + mountPoint: + path: /boot + + - deviceId: verityroot + type: ext4 + mountPoint: + path: / + options: ro + + verity: + - id: verityroot + name: root + dataDeviceId: root + hashDeviceId: roothash + corruptionOption: panic + +previewFeatures: +- base-configs + +baseConfigs: + - path: hierarchical-config-storage-verity-base.yaml + +os: + bootloader: + resetType: hard-reset From 573055455d89edc58a6ad12f12699dcc13bdb35e Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Mon, 17 Nov 2025 23:35:59 +0000 Subject: [PATCH 2/7] update --- .../imagecustomizerlib/baseconfigs_test.go | 118 +++++++++++------- .../imagecustomizerlib/configvalidation.go | 6 +- .../testdata/hierarchical-config-base.yaml | 36 ++++++ ...rarchical-config-storage-reinitverity.yaml | 10 ++ ...hierarchical-config-storage-resetuuid.yaml | 12 ++ ...erarchical-config-storage-verity-base.yaml | 46 ------- .../hierarchical-config-storage-verity.yaml | 52 -------- .../testdata/hierarchical-config.yaml | 15 ++- 8 files changed, 145 insertions(+), 150 deletions(-) create mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-reinitverity.yaml create mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-resetuuid.yaml delete mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml delete mode 100644 toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go index be766b56be..e31f74caf7 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go @@ -13,6 +13,7 @@ import ( "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/testutils" "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/userutils" "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" ) func TestBaseConfigsInputAndOutput(t *testing.T) { @@ -57,19 +58,40 @@ func TestBaseConfigsInputAndOutput(t *testing.T) { } func TestBaseConfigsFullRun(t *testing.T) { + baseImage, baseImageInfo := checkSkipForCustomizeDefaultImage(t) + if baseImageInfo.Version == baseImageVersionAzl2 { + t.Skip("'systemd-boot' is not available on Azure Linux 2.0") + } + + ukifyExists, err := file.CommandExists("ukify") + assert.NoError(t, err) + if !ukifyExists { + t.Skip("The 'ukify' command is not available") + } + + if runtime.GOARCH == "arm64" { + t.Skip("systemd-boot not available on AZL3 ARM64 yet") + } + testTmpDir := filepath.Join(tmpDir, "TestBaseConfigsFullRun") defer os.RemoveAll(testTmpDir) buildDir := filepath.Join(testTmpDir, "build") outImageFilePath := filepath.Join(testTmpDir, "image.raw") currentConfigFile := filepath.Join(testDir, "hierarchical-config.yaml") - customizeImageOrSkip(t, buildDir, currentConfigFile, outImageFilePath) + err = CustomizeImageWithConfigFile(t.Context(), buildDir, currentConfigFile, baseImage, nil, + outImageFilePath, "raw", true, "") + if !assert.NoError(t, err) { + t.FailNow() + } + assert.FileExists(t, outImageFilePath) mountPoints := []testutils.MountPoint{ { PartitionNum: 3, Path: "/", FileSystemType: "ext4", + Flags: unix.MS_RDONLY, }, { PartitionNum: 2, @@ -221,40 +243,11 @@ func TestBaseConfigsFullRun(t *testing.T) { assert.NoError(t, err) assert.NotContains(t, grubCfgContents, "rd.info") assert.Contains(t, grubCfgContents, "console=tty0 console=ttyS0") -} - -func TestStorageVerityOverride(t *testing.T) { - testTempDir := filepath.Join(tmpDir, "TestStorageVerityOverride") - defer os.RemoveAll(testTempDir) - - buildDir := filepath.Join(testTempDir, "build") - outImageFilePath := filepath.Join(testTempDir, "image.raw") - currentConfigFile := filepath.Join(testDir, "hierarchical-config-storage-verity.yaml") - - baseImageInfo := customizeImageOrSkip(t, buildDir, currentConfigFile, outImageFilePath) - - mountPoints := []testutils.MountPoint{ - { - PartitionNum: 1, - Path: "/boot/efi", - FileSystemType: "vfat", - }, - { - PartitionNum: 2, - Path: "/boot", - FileSystemType: "ext4", - }, - } - - imageConnection, err := testutils.ConnectToImage(buildDir, outImageFilePath, true, mountPoints) - if !assert.NoError(t, err) { - return - } - defer imageConnection.Close() partitions, err := getDiskPartitionsMap(imageConnection.Loopback().DevicePath()) assert.NoError(t, err, "get disk partitions") + // Verify verity bootPath := filepath.Join(imageConnection.Chroot().RootDir(), "/boot") rootDevice := testutils.PartitionDevPath(imageConnection, 3) hashDevice := testutils.PartitionDevPath(imageConnection, 4) @@ -270,31 +263,60 @@ func TestStorageVerityOverride(t *testing.T) { baseImageInfo, "panic-on-corruption", ) -} -func customizeImageOrSkip(t *testing.T, buildDir, configFile, outImageFilePath string) testBaseImageInfo { - baseImage, baseImageInfo := checkSkipForCustomizeDefaultImage(t) - if baseImageInfo.Version == baseImageVersionAzl2 { - t.Skip("'systemd-boot' is not available on Azure Linux 2.0") + err = imageConnection.CleanClose() + if !assert.NoError(t, err) { + return } - ukifyExists, err := file.CommandExists("ukify") +} + +func TestStorageErrorDisksWithResetUUID(t *testing.T) { + testTempDir := filepath.Join(tmpDir, "TestStorageErrorDisksWithResetUUID") + defer os.RemoveAll(testTempDir) + + buildDir := filepath.Join(testTempDir, "build") + currentConfigFile := filepath.Join(testDir, "hierarchical-config-storage-resetuuid.yaml") + + var config imagecustomizerapi.Config + err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) assert.NoError(t, err) - if !ukifyExists { - t.Skip("The 'ukify' command is not available") - } - if runtime.GOARCH == "arm64" { - t.Skip("systemd-boot not available on AZL3 ARM64 yet") + baseConfigPath, _ := filepath.Split(currentConfigFile) + absBaseConfigPath, err := filepath.Abs(baseConfigPath) + assert.NoError(t, err) + + options := ImageCustomizerOptions{ + BuildDir: buildDir, } - err = CustomizeImageWithConfigFile(t.Context(), buildDir, configFile, baseImage, nil, - outImageFilePath, "raw", true, "") - if !assert.NoError(t, err) { - t.FailNow() + _, err = ValidateConfig(t.Context(), absBaseConfigPath, &config, false, options) + + assert.Error(t, err) + assert.ErrorContains(t, err, "cannot specify 'resetPartitionsUuidsType'") +} + +func TestStorageErrorDisksWithReinitVerity(t *testing.T) { + testTempDir := filepath.Join(tmpDir, "TestStorageErrorDisksWithReinitVerity") + defer os.RemoveAll(testTempDir) + + buildDir := filepath.Join(testTempDir, "build") + currentConfigFile := filepath.Join(testDir, "hierarchical-config-storage-reinitverity.yaml") + + var config imagecustomizerapi.Config + err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) + assert.NoError(t, err) + + baseConfigPath, _ := filepath.Split(currentConfigFile) + absBaseConfigPath, err := filepath.Abs(baseConfigPath) + assert.NoError(t, err) + + options := ImageCustomizerOptions{ + BuildDir: buildDir, } - assert.FileExists(t, outImageFilePath) + _, err = ValidateConfig(t.Context(), absBaseConfigPath, &config, false, options) - return baseImageInfo + assert.Error(t, err) + assert.ErrorContains(t, err, "cannot specify 'reinitializeVerity'") } diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 6d928f4217..1bbe3e5e64 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -530,10 +530,12 @@ func validateStorage(configChain []*ConfigWithBasePath) error { if baseHasDisks { if hasResetUUID { - return fmt.Errorf("invalid storage inheritance at layer %d: current config sets .storage.resetPartitionsUuidsType after a base config defined .storage.disks", i) + return fmt.Errorf( + "cannot specify 'resetPartitionsUuidsType' in config at layer %d when a base config specifies '.storage.disks'", i) } if hasReinitVerity { - return fmt.Errorf("invalid storage inheritance at layer %d: current config sets .storage.reinitializeVerity after a base config defined .storage.disks", i) + return fmt.Errorf( + "cannot specify 'reinitializeVerity' in config at layer %d when a base config specifies '.storage.disks'", i) } } diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-base.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-base.yaml index 713dbbc423..9d0a1ecd11 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-base.yaml +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-base.yaml @@ -15,7 +15,43 @@ output: - ukis path: ./artifacts-1 +storage: + bootType: efi + disks: + - partitionTableType: gpt + partitions: + - id: esp + type: esp + size: 8M + + - id: usr + size: 2G + + - id: usrhash + size: 100M + + filesystems: + - deviceId: esp + type: fat32 + mountPoint: + path: /boot/efi + options: umask=0077 + + - deviceId: verityusr + type: ext4 + mountPoint: + path: /usr + options: ro + + verity: + - id: verityusr + name: usr + dataDeviceId: usr + hashDeviceId: usrhash + os: + bootloader: + resetType: hard-reset hostname: test-hostname-base users: - name: test-user-base diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-reinitverity.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-reinitverity.yaml new file mode 100644 index 0000000000..c30ff08686 --- /dev/null +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-reinitverity.yaml @@ -0,0 +1,10 @@ +previewFeatures: +- base-configs +- reinitialize-verity + +storage: + reinitializeVerity: all + +baseConfigs: + - path: hierarchical-config.yaml + diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-resetuuid.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-resetuuid.yaml new file mode 100644 index 0000000000..acda0e6b55 --- /dev/null +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-resetuuid.yaml @@ -0,0 +1,12 @@ +previewFeatures: +- base-configs + +storage: + resetPartitionsUuidsType: reset-all + +baseConfigs: + - path: hierarchical-config.yaml + +os: + bootloader: + resetType: hard-reset diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml deleted file mode 100644 index 4f81ef2a27..0000000000 --- a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity-base.yaml +++ /dev/null @@ -1,46 +0,0 @@ -storage: - bootType: efi - disks: - - partitionTableType: gpt - partitions: - - id: esp - type: esp - size: 8M - - - id: usr - size: 2G - - - id: usrhash - size: 100M - - filesystems: - - deviceId: esp - type: fat32 - mountPoint: - path: /boot/efi - options: umask=0077 - - - deviceId: verityusr - type: ext4 - mountPoint: - path: /usr - options: ro - - verity: - - id: verityusr - name: usr - dataDeviceId: usr - hashDeviceId: usrhash - -input: - image: - path: testimages/empty-base.vhdx - -output: - image: - path: ./out/storage-reinit-verity.vhdx - format: vhdx - -os: - bootloader: - resetType: hard-reset diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml deleted file mode 100644 index 417b74ae3a..0000000000 --- a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config-storage-verity.yaml +++ /dev/null @@ -1,52 +0,0 @@ -storage: - bootType: efi - disks: - - partitionTableType: gpt - partitions: - - id: esp - type: esp - size: 8M - - - id: boot - size: 1G - - - id: root - size: 2G - - - id: roothash - size: 100M - - filesystems: - - deviceId: esp - type: fat32 - mountPoint: - path: /boot/efi - options: umask=0077 - - - deviceId: boot - type: ext4 - mountPoint: - path: /boot - - - deviceId: verityroot - type: ext4 - mountPoint: - path: / - options: ro - - verity: - - id: verityroot - name: root - dataDeviceId: root - hashDeviceId: roothash - corruptionOption: panic - -previewFeatures: -- base-configs - -baseConfigs: - - path: hierarchical-config-storage-verity-base.yaml - -os: - bootloader: - resetType: hard-reset diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml index 4040e7cb23..c320508d64 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml @@ -9,9 +9,12 @@ storage: - id: boot size: 100M - - id: rootfs + - id: root size: 2G + - id: roothash + size: 100M + bootType: efi filesystems: @@ -26,10 +29,18 @@ storage: mountPoint: path: /boot - - deviceId: rootfs + - deviceId: verityroot type: ext4 mountPoint: path: / + options: ro + + verity: + - id: verityroot + name: root + dataDeviceId: root + hashDeviceId: roothash + corruptionOption: panic previewFeatures: - output-artifacts From 0f0cdd860dd6dde1fdaf5ebbe17180eb52c3286a Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Tue, 18 Nov 2025 16:33:12 +0000 Subject: [PATCH 3/7] update --- .../imagecustomizerlib/baseconfigs_test.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go index e31f74caf7..23959a7ee0 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go @@ -82,7 +82,7 @@ func TestBaseConfigsFullRun(t *testing.T) { err = CustomizeImageWithConfigFile(t.Context(), buildDir, currentConfigFile, baseImage, nil, outImageFilePath, "raw", true, "") if !assert.NoError(t, err) { - t.FailNow() + return } assert.FileExists(t, outImageFilePath) @@ -247,6 +247,13 @@ func TestBaseConfigsFullRun(t *testing.T) { partitions, err := getDiskPartitionsMap(imageConnection.Loopback().DevicePath()) assert.NoError(t, err, "get disk partitions") + // Verify partitions + assert.Len(t, partitions, 4) + assert.Equal(t, "esp", partitions[1].PartLabel) + assert.Equal(t, "", partitions[2].PartLabel) + assert.Equal(t, "", partitions[3].PartLabel) + assert.Equal(t, "", partitions[4].PartLabel) + // Verify verity bootPath := filepath.Join(imageConnection.Chroot().RootDir(), "/boot") rootDevice := testutils.PartitionDevPath(imageConnection, 3) @@ -259,7 +266,7 @@ func TestBaseConfigsFullRun(t *testing.T) { "PARTUUID="+partitions[3].PartUuid, "PARTUUID="+partitions[4].PartUuid, "root", - "rd.info", + "console=tty0", baseImageInfo, "panic-on-corruption", ) @@ -271,8 +278,8 @@ func TestBaseConfigsFullRun(t *testing.T) { } -func TestStorageErrorDisksWithResetUUID(t *testing.T) { - testTempDir := filepath.Join(tmpDir, "TestStorageErrorDisksWithResetUUID") +func TestValidateDisksThenResetUUID(t *testing.T) { + testTempDir := filepath.Join(tmpDir, "TestValidateDisksThenResetUUID") defer os.RemoveAll(testTempDir) buildDir := filepath.Join(testTempDir, "build") @@ -296,8 +303,8 @@ func TestStorageErrorDisksWithResetUUID(t *testing.T) { assert.ErrorContains(t, err, "cannot specify 'resetPartitionsUuidsType'") } -func TestStorageErrorDisksWithReinitVerity(t *testing.T) { - testTempDir := filepath.Join(tmpDir, "TestStorageErrorDisksWithReinitVerity") +func TestValidateDisksThenReinitVerity(t *testing.T) { + testTempDir := filepath.Join(tmpDir, "TestValidateDisksThenReinitVerity") defer os.RemoveAll(testTempDir) buildDir := filepath.Join(testTempDir, "build") From fc462ed6f83dd82f919338afd446a9fb38e7e1eb Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Tue, 18 Nov 2025 17:14:03 +0000 Subject: [PATCH 4/7] add doc --- .../api/configuration/baseConfig.md | 23 +++++++++++++++++++ .../imagecustomizerlib/baseconfigs_test.go | 12 ++-------- .../imagecustomizerlib/configvalidation.go | 2 +- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/docs/imagecustomizer/api/configuration/baseConfig.md b/docs/imagecustomizer/api/configuration/baseConfig.md index 1fd46b7d6c..fe36bcf49e 100644 --- a/docs/imagecustomizer/api/configuration/baseConfig.md +++ b/docs/imagecustomizer/api/configuration/baseConfig.md @@ -51,6 +51,29 @@ or are processed sequentially. - `"hard-reset"` - `""` (i.e. no reset) +**Storage Override Fields:** + +Current value (if specified) fully overrides base's value +- `.storage.disks` +- `.storage.verity` +- `.storage.bootType` +- `.storage.filesystems` + +**Storage configuration also follows specific rules based on field combinations:** + +| Base\Current | None | `.storage.disks` | `.storage.resetPartitionsUuidsType` | `.storage.reinitializeVerity` | +|--------------|------|------------------|-------------------------------------|------------------------------| +| **None** | None | Current's `.storage.disks` | Current's `.storage.resetPartitionsUuidsType` | Current's `.storage.reinitializeVerity` | +| **`.storage.disks`** | Base's `.storage.disks` | Current's `.storage.disks` | Error¹ | Error² | +| **`.resetPartitionsUuidsType`** | Base's `.storage.resetPartitionsUuidsType` | Current's `.storage.disks` | Current's `.storage.resetPartitionsUuidsType` | Base's reset + Current's reinit³ | +| **`.storage.reinitializeVerity`** | Base's `.storage.reinitializeVerity` | Current's `.storage.disks` | Base's reinit + Current's reset³ | Current's `.storage.reinitializeVerity` | + +¹ The current config requesting the partition UUIDs to be reset when the base config customized the partition table is somewhat pointless. Hence we error out. + +² Cannot reinitialize verity when base config destroys original verity setup with partition layout changes. + +³ Base's `.resetPartitionsUuidsType` and current's `.reinitializeVerity` are compatible and both are preserved together since UUID reset updates partition identifiers while verity reinit updates verity mapping to use new identifiers and rebuilds hash trees. + ## path [string] Required. diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go index 23959a7ee0..85c0a45edc 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go @@ -259,16 +259,8 @@ func TestBaseConfigsFullRun(t *testing.T) { rootDevice := testutils.PartitionDevPath(imageConnection, 3) hashDevice := testutils.PartitionDevPath(imageConnection, 4) - verifyVerityGrub(t, - bootPath, - rootDevice, - hashDevice, - "PARTUUID="+partitions[3].PartUuid, - "PARTUUID="+partitions[4].PartUuid, - "root", - "console=tty0", - baseImageInfo, - "panic-on-corruption", + verifyVerityGrub(t, bootPath, rootDevice, hashDevice, "PARTUUID="+partitions[3].PartUuid, + "PARTUUID="+partitions[4].PartUuid, "root", "console=tty0", baseImageInfo, "panic-on-corruption", ) err = imageConnection.CleanClose() diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 1bbe3e5e64..6c51565c03 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -669,8 +669,8 @@ func resolveStorage(configChain []*ConfigWithBasePath) imagecustomizerapi.Storag for _, configWithBase := range slices.Backward(configChain) { storage := configWithBase.Config.Storage - // .storage.disks - override if len(storage.Disks) > 0 { + // Override resolvedStorage.Disks = storage.Disks resolvedStorage.BootType = storage.BootType resolvedStorage.FileSystems = storage.FileSystems From dbe5e0c14599a30eb25f5ddb2227083e1b2ef56f Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Thu, 20 Nov 2025 13:29:46 +0000 Subject: [PATCH 5/7] update storage refs --- toolkit/tools/imagecustomizerapi/config.go | 6 +-- toolkit/tools/imagecustomizerapi/storage.go | 4 -- .../tools/pkg/imagecreatorlib/imagecreator.go | 4 +- .../imagecustomizerlib/configvalidation.go | 41 ++++++++++++++++--- .../imagecustomizerlib/customizebootloader.go | 18 ++++---- .../imagecustomizerlib/customizepartitions.go | 21 +++++----- .../customizepartitionsfilecopy.go | 13 +++--- .../pkg/imagecustomizerlib/customizeverity.go | 25 +++++------ .../pkg/imagecustomizerlib/imagecustomizer.go | 9 ++-- 9 files changed, 82 insertions(+), 59 deletions(-) diff --git a/toolkit/tools/imagecustomizerapi/config.go b/toolkit/tools/imagecustomizerapi/config.go index b7efe4ee51..a4533e16ee 100644 --- a/toolkit/tools/imagecustomizerapi/config.go +++ b/toolkit/tools/imagecustomizerapi/config.go @@ -88,7 +88,7 @@ func (c *Config) IsValid() (err error) { return err } - if c.CustomizePartitions() && !hasResetBootLoader { + if len(c.Storage.Disks) != 0 && !hasResetBootLoader { return fmt.Errorf("'os.bootloader.reset' must be specified if 'storage.disks' is specified") } @@ -150,7 +150,3 @@ func (c *Config) IsValid() (err error) { return nil } - -func (c *Config) CustomizePartitions() bool { - return c.Storage.CustomizePartitions() -} diff --git a/toolkit/tools/imagecustomizerapi/storage.go b/toolkit/tools/imagecustomizerapi/storage.go index d6fb2b5789..61130ee13a 100644 --- a/toolkit/tools/imagecustomizerapi/storage.go +++ b/toolkit/tools/imagecustomizerapi/storage.go @@ -256,10 +256,6 @@ func ValidateVerityMounts(verityDevices []Verity, verityDeviceMountPoint map[*Ve return nil } -func (s *Storage) CustomizePartitions() bool { - return len(s.Disks) > 0 -} - func (s *Storage) buildDeviceMap() (map[string]any, map[string]int, error) { deviceMap := make(map[string]any) partitionLabelCounts := make(map[string]int) diff --git a/toolkit/tools/pkg/imagecreatorlib/imagecreator.go b/toolkit/tools/pkg/imagecreatorlib/imagecreator.go index b07343464a..292a94d7b0 100644 --- a/toolkit/tools/pkg/imagecreatorlib/imagecreator.go +++ b/toolkit/tools/pkg/imagecreatorlib/imagecreator.go @@ -73,7 +73,7 @@ func createNewImage(ctx context.Context, buildDir string, baseConfigPath string, return err } - disks := rc.Config.Storage.Disks + disks := rc.Storage.Disks diskConfig := disks[0] installOSFunc := func(imageChroot *safechroot.Chroot) error { return nil @@ -86,7 +86,7 @@ func createNewImage(ctx context.Context, buildDir string, baseConfigPath string, partIdToPartUuid, err := imagecustomizerlib.CreateNewImage( distroHandler.GetTargetOs(), rc.RawImageFile, - diskConfig, rc.Config.Storage.FileSystems, + diskConfig, rc.Storage.FileSystems, rc.BuildDirAbs, setupRoot, installOSFunc) if err != nil { return err diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 6c51565c03..1f03f1ab60 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -78,10 +78,17 @@ func ValidateConfig(ctx context.Context, baseConfigPath string, config *imagecus return nil, err } - rc.CustomizeOSPartitions = config.CustomizePartitions() || - config.OS != nil || - len(config.Scripts.PostCustomization) > 0 || - len(config.Scripts.FinalizeCustomization) > 0 + rc.CustomizeOSPartitions = false + + for _, configWithBase := range rc.ConfigChain { + if len(configWithBase.Config.Storage.Disks) > 0 || + configWithBase.Config.OS != nil || + len(configWithBase.Config.Scripts.PostCustomization) > 0 || + len(configWithBase.Config.Scripts.FinalizeCustomization) > 0 { + rc.CustomizeOSPartitions = true + break + } + } // Create a UUID for the image. rc.ImageUuid, rc.ImageUuidStr, err = randomization.CreateUuid() @@ -510,7 +517,7 @@ func validateIsoPxeCustomization(rc *ResolvedConfig) error { // While defining a storage configuration can work when the input image is // an iso, there is no obvious point of moving content between partitions // where all partitions get collapsed into the squashfs at the end. - if rc.Config.CustomizePartitions() { + if len(rc.Storage.Disks) > 0 { return ErrCannotCustomizePartitionsOnIso } } @@ -687,6 +694,30 @@ func resolveStorage(configChain []*ConfigWithBasePath) imagecustomizerapi.Storag if storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault { resolvedStorage.ReinitializeVerity = storage.ReinitializeVerity } + + // Recalculate VerityPartitionsType based on resolved storage + verityUsesConfigPartitions := false + verityUsesExistingPartitions := false + + for _, verity := range resolvedStorage.Verity { + if verity.DataDeviceId != "" || verity.HashDeviceId != "" { + verityUsesConfigPartitions = true + } + if verity.DataDevice != nil || verity.HashDevice != nil { + verityUsesExistingPartitions = true + } + } + + switch { + case verityUsesConfigPartitions && verityUsesExistingPartitions: + return imagecustomizerapi.Storage{} + case verityUsesConfigPartitions: + resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsUsesConfig + case verityUsesExistingPartitions: + resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsUsesExisting + default: + resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsNone + } } return resolvedStorage diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go b/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go index 0c6b7d43df..916dfdd879 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go @@ -35,8 +35,7 @@ func handleBootLoader(ctx context.Context, rc *ResolvedConfig, imageConnection * ) error { switch { case rc.BootLoader.ResetType == imagecustomizerapi.ResetBootLoaderTypeHard || newImage: - err := hardResetBootLoader(ctx, rc.BaseConfigPath, rc.Config, imageConnection, partUuidToFstabEntry, - newImage, rc.SELinux) + err := hardResetBootLoader(ctx, rc, imageConnection, partUuidToFstabEntry, newImage) if err != nil { return fmt.Errorf("%w:\n%w", ErrBootloaderHardReset, err) } @@ -52,8 +51,8 @@ func handleBootLoader(ctx context.Context, rc *ResolvedConfig, imageConnection * return nil } -func hardResetBootLoader(ctx context.Context, baseConfigPath string, config *imagecustomizerapi.Config, imageConnection *imageconnection.ImageConnection, - partUuidToFstabEntry map[string]diskutils.FstabEntry, newImage bool, selinuxConfig imagecustomizerapi.SELinux, +func hardResetBootLoader(ctx context.Context, rc *ResolvedConfig, imageConnection *imageconnection.ImageConnection, + partUuidToFstabEntry map[string]diskutils.FstabEntry, newImage bool, ) error { var err error logger.Log.Infof("Hard reset bootloader config") @@ -78,8 +77,9 @@ func hardResetBootLoader(ctx context.Context, baseConfigPath string, config *ima var rootMountIdType imagecustomizerapi.MountIdentifierType var bootType imagecustomizerapi.BootType - if config.CustomizePartitions() { - rootFileSystem, foundRootFileSystem := sliceutils.FindValueFunc(config.Storage.FileSystems, + + if len(rc.Storage.Disks) > 0 { + rootFileSystem, foundRootFileSystem := sliceutils.FindValueFunc(rc.Storage.FileSystems, func(fileSystem imagecustomizerapi.FileSystem) bool { return fileSystem.MountPoint != nil && fileSystem.MountPoint.Path == "/" @@ -90,7 +90,7 @@ func hardResetBootLoader(ctx context.Context, baseConfigPath string, config *ima } rootMountIdType = rootFileSystem.MountPoint.IdType - bootType = config.Storage.BootType + bootType = rc.Storage.BootType } else { rootMountIdType, err = findRootMountIdType(partUuidToFstabEntry) if err != nil { @@ -104,8 +104,8 @@ func hardResetBootLoader(ctx context.Context, baseConfigPath string, config *ima } // Hard-reset the grub config. - err = configureDiskBootLoader(imageConnection, rootMountIdType, bootType, selinuxConfig, - config.OS.KernelCommandLine, currentSelinuxMode, newImage) + err = configureDiskBootLoader(imageConnection, rootMountIdType, bootType, rc.SELinux, + rc.Config.OS.KernelCommandLine, currentSelinuxMode, newImage) if err != nil { return fmt.Errorf("%w:\n%w", ErrBootloaderDiskConfigure, err) } diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go index 2ebf9f9cac..bc6a59d5ec 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go @@ -20,22 +20,23 @@ var ( ErrPartitionsResetUuids = NewImageCustomizerError("Partitions:ResetUuids", "failed to reset partition UUIDs") ) -func customizePartitions(ctx context.Context, buildDir string, baseConfigPath string, config *imagecustomizerapi.Config, - buildImageFile string, targetOS targetos.TargetOs, +func customizePartitions(ctx context.Context, rc *ResolvedConfig, targetOS targetos.TargetOs, ) (bool, string, map[string]string, error) { + + storage := rc.Storage + switch { - case config.CustomizePartitions(): + case len(storage.Disks) > 0: logger.Log.Infof("Customizing partitions") _, span := otel.GetTracerProvider().Tracer(OtelTracerName).Start(ctx, "customize_partitions") defer span.End() - newBuildImageFile := filepath.Join(buildDir, PartitionCustomizedImageName) + newBuildImageFile := filepath.Join(rc.BuildDirAbs, PartitionCustomizedImageName) // If there is no known way to create the new partition layout from the old one, // then fallback to creating the new partitions from scratch and doing a file copy. - partIdToPartUuid, err := customizePartitionsUsingFileCopy(ctx, buildDir, baseConfigPath, config, - buildImageFile, newBuildImageFile, targetOS) + partIdToPartUuid, err := customizePartitionsUsingFileCopy(ctx, rc, newBuildImageFile, targetOS) if err != nil { os.Remove(newBuildImageFile) return false, "", nil, fmt.Errorf("%w:\n%w", ErrPartitionsCustomize, err) @@ -43,17 +44,17 @@ func customizePartitions(ctx context.Context, buildDir string, baseConfigPath st return true, newBuildImageFile, partIdToPartUuid, nil - case config.Storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault: - err := resetPartitionsUuids(ctx, buildImageFile, buildDir) + case storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault: + err := resetPartitionsUuids(ctx, rc.RawImageFile, rc.BuildDirAbs) if err != nil { return false, "", nil, fmt.Errorf("%w:\n%w", ErrPartitionsResetUuids, err) } - return true, buildImageFile, nil, nil + return true, rc.RawImageFile, nil, nil default: // No changes to make to the partitions. // So, just use the original disk. - return false, buildImageFile, nil, nil + return false, rc.RawImageFile, nil, nil } } diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizepartitionsfilecopy.go b/toolkit/tools/pkg/imagecustomizerlib/customizepartitionsfilecopy.go index 6d28226034..e5e076b146 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizepartitionsfilecopy.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizepartitionsfilecopy.go @@ -7,7 +7,6 @@ import ( "context" "fmt" - "github.com/microsoft/azure-linux-image-tools/toolkit/tools/imagecustomizerapi" "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/safechroot" "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/shell" "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/targetos" @@ -21,24 +20,24 @@ var ( ErrPartitionCopyFiles = NewImageCustomizerError("PartitionCopy:Files", "failed to copy partition files") ) -func customizePartitionsUsingFileCopy(ctx context.Context, buildDir string, baseConfigPath string, config *imagecustomizerapi.Config, - buildImageFile string, newBuildImageFile string, targetOS targetos.TargetOs, +func customizePartitionsUsingFileCopy(ctx context.Context, rc *ResolvedConfig, + newBuildImageFile string, targetOS targetos.TargetOs, ) (map[string]string, error) { - existingImageConnection, _, _, _, err := connectToExistingImage(ctx, buildImageFile, buildDir, "imageroot", false, + existingImageConnection, _, _, _, err := connectToExistingImage(ctx, rc.RawImageFile, rc.BuildDirAbs, "imageroot", false, true, false, false) if err != nil { return nil, err } defer existingImageConnection.Close() - diskConfig := config.Storage.Disks[0] + diskConfig := rc.Storage.Disks[0] installOSFunc := func(imageChroot *safechroot.Chroot) error { return copyFilesIntoNewDisk(existingImageConnection.Chroot(), imageChroot) } - partIdToPartUuid, err := CreateNewImage(targetOS, newBuildImageFile, diskConfig, config.Storage.FileSystems, - buildDir, "newimageroot", installOSFunc) + partIdToPartUuid, err := CreateNewImage(targetOS, newBuildImageFile, diskConfig, rc.Storage.FileSystems, + rc.BuildDirAbs, "newimageroot", installOSFunc) if err != nil { return nil, err } diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizeverity.go b/toolkit/tools/pkg/imagecustomizerlib/customizeverity.go index 56f393f6a3..25b12a5b35 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizeverity.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizeverity.go @@ -469,10 +469,10 @@ func updateUkiKernelArgsForVerity(verityMetadata []verityDeviceMetadata, return nil } -func validateVerityMountPaths(imageConnection *imageconnection.ImageConnection, config *imagecustomizerapi.Config, +func validateVerityMountPaths(imageConnection *imageconnection.ImageConnection, storage imagecustomizerapi.Storage, partUuidToFstabEntry map[string]diskutils.FstabEntry, baseImageVerityMetadata []verityDeviceMetadata, ) error { - if config.Storage.VerityPartitionsType != imagecustomizerapi.VerityPartitionsUsesExisting { + if storage.VerityPartitionsType != imagecustomizerapi.VerityPartitionsUsesExisting { // Either: // - Verity is not being used OR // - Partitions were customized and the verity checks were done during the API validity checks. @@ -486,8 +486,8 @@ func validateVerityMountPaths(imageConnection *imageconnection.ImageConnection, } verityDeviceMountPoint := make(map[*imagecustomizerapi.Verity]*imagecustomizerapi.MountPoint) - for i := range config.Storage.Verity { - verity := &config.Storage.Verity[i] + for i := range storage.Verity { + verity := &storage.Verity[i] dataPartition, err := findIdentifiedPartition(partitions, *verity.DataDevice) if err != nil { @@ -530,7 +530,7 @@ func validateVerityMountPaths(imageConnection *imageconnection.ImageConnection, verityDeviceMountPoint[verity] = mountPoint } - err = imagecustomizerapi.ValidateVerityMounts(config.Storage.Verity, verityDeviceMountPoint) + err = imagecustomizerapi.ValidateVerityMounts(storage.Verity, verityDeviceMountPoint) if err != nil { return err } @@ -570,9 +570,8 @@ func findIdentifiedPartition(partitions []diskutils.PartitionInfo, ref imagecust return partition, nil } -func customizeVerityImageHelper(ctx context.Context, buildDir string, config *imagecustomizerapi.Config, - buildImageFile string, partIdToPartUuid map[string]string, shrinkHashPartition bool, - baseImageVerity []verityDeviceMetadata, readonlyPartUuids []string, +func customizeVerityImageHelper(ctx context.Context, rc *ResolvedConfig, partIdToPartUuid map[string]string, + shrinkHashPartition bool, baseImageVerity []verityDeviceMetadata, readonlyPartUuids []string, partUuidToFstabEntry map[string]diskutils.FstabEntry, ) ([]verityDeviceMetadata, error) { logger.Log.Infof("Provisioning verity") @@ -582,7 +581,10 @@ func customizeVerityImageHelper(ctx context.Context, buildDir string, config *im verityMetadata := []verityDeviceMetadata(nil) - loopback, err := safeloopback.NewLoopback(buildImageFile) + isUki := rc.Config.OS.Uki != nil + storage := rc.Storage + + loopback, err := safeloopback.NewLoopback(rc.RawImageFile) if err != nil { return nil, fmt.Errorf("%w:\n%w", ErrVerityImageConnection, err) } @@ -632,7 +634,7 @@ func customizeVerityImageHelper(ctx context.Context, buildDir string, config *im verityMetadata = append(verityMetadata, newMetadata) } - for _, verityConfig := range config.Storage.Verity { + for _, verityConfig := range storage.Verity { // Extract the partition block device path. dataPartition, err := verityIdToPartition(verityConfig.DataDeviceId, verityConfig.DataDevice, partIdToPartUuid, diskPartitions) @@ -679,8 +681,7 @@ func customizeVerityImageHelper(ctx context.Context, buildDir string, config *im } // Update kernel args. - isUki := config.OS.Uki != nil - err = updateKernelArgsForVerity(buildDir, diskPartitions, verityMetadata, isUki, partUuidToFstabEntry) + err = updateKernelArgsForVerity(rc.BuildDirAbs, diskPartitions, verityMetadata, isUki, partUuidToFstabEntry) if err != nil { return nil, err } diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go index 1871d9a7c6..65279a4402 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go @@ -485,8 +485,7 @@ func customizeOSContents(ctx context.Context, rc *ResolvedConfig) (imageMetadata im.targetOS = targetOS // Customize the partitions. - partitionsCustomized, newRawImageFile, partIdToPartUuid, err := customizePartitions(ctx, rc.BuildDirAbs, - rc.BaseConfigPath, rc.Config, rc.RawImageFile, im.targetOS) + partitionsCustomized, newRawImageFile, partIdToPartUuid, err := customizePartitions(ctx, rc, im.targetOS) if err != nil { return im, err } @@ -526,8 +525,8 @@ func customizeOSContents(ctx context.Context, rc *ResolvedConfig) (imageMetadata if len(rc.Storage.Verity) > 0 || len(im.baseImageVerityMetadata) > 0 { // Customize image for dm-verity, setting up verity metadata and security features. - verityMetadata, err := customizeVerityImageHelper(ctx, rc.BuildDirAbs, rc.Config, rc.RawImageFile, - partIdToPartUuid, shrinkPartitions, im.baseImageVerityMetadata, readonlyPartUuids, partUuidToFstabEntry) + verityMetadata, err := customizeVerityImageHelper(ctx, rc, partIdToPartUuid, shrinkPartitions, + im.baseImageVerityMetadata, readonlyPartUuids, partUuidToFstabEntry) if err != nil { return im, fmt.Errorf("%w:\n%w", ErrCustomizeProvisionVerity, err) } @@ -702,7 +701,7 @@ func customizeImageHelper(ctx context.Context, rc *ResolvedConfig, partitionsCus return nil }) - err = validateVerityMountPaths(imageConnection, rc.Config, partUuidToFstabEntry, baseImageVerityMetadata) + err = validateVerityMountPaths(imageConnection, rc.Storage, partUuidToFstabEntry, baseImageVerityMetadata) if err != nil { return nil, nil, nil, "", fmt.Errorf("%w:\n%w", ErrVerityValidation, err) } From 60732a037c5b321d06f57ec6ee18624d8f17654f Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Tue, 25 Nov 2025 13:34:06 +0000 Subject: [PATCH 6/7] resolve comments --- toolkit/tools/imagecustomizerapi/storage.go | 4 + .../tools/pkg/imagecreatorlib/imagecreator.go | 12 +- .../tools/pkg/imagecreatorlib/validation.go | 12 +- .../pkg/imagecreatorlib/validation_test.go | 33 +++--- .../pkg/imagecustomizerlib/baseconfigs.go | 9 +- .../imagecustomizerlib/baseconfigs_test.go | 48 +++++--- .../imagecustomizerlib/configvalidation.go | 110 ++++++++---------- .../imagecustomizerlib/customizebootloader.go | 2 +- .../imagecustomizerlib/customizepartitions.go | 2 +- .../pkg/imagecustomizerlib/imagecustomizer.go | 13 ++- .../imagecustomizer_test.go | 84 +++++++------ .../pkg/imagecustomizerlib/resolvedconfig.go | 1 + .../testdata/hierarchical-config.yaml | 4 + 13 files changed, 175 insertions(+), 159 deletions(-) diff --git a/toolkit/tools/imagecustomizerapi/storage.go b/toolkit/tools/imagecustomizerapi/storage.go index 61130ee13a..d6fb2b5789 100644 --- a/toolkit/tools/imagecustomizerapi/storage.go +++ b/toolkit/tools/imagecustomizerapi/storage.go @@ -256,6 +256,10 @@ func ValidateVerityMounts(verityDevices []Verity, verityDeviceMountPoint map[*Ve return nil } +func (s *Storage) CustomizePartitions() bool { + return len(s.Disks) > 0 +} + func (s *Storage) buildDeviceMap() (map[string]any, map[string]int, error) { deviceMap := make(map[string]any) partitionLabelCounts := make(map[string]int) diff --git a/toolkit/tools/pkg/imagecreatorlib/imagecreator.go b/toolkit/tools/pkg/imagecreatorlib/imagecreator.go index 292a94d7b0..b7127d9337 100644 --- a/toolkit/tools/pkg/imagecreatorlib/imagecreator.go +++ b/toolkit/tools/pkg/imagecreatorlib/imagecreator.go @@ -26,15 +26,13 @@ func CreateImageWithConfigFile(ctx context.Context, buildDir string, configFile return fmt.Errorf("failed to unmarshal config file %s:\n%w", configFile, err) } - baseConfigPath, _ := filepath.Split(configFile) - - absBaseConfigPath, err := filepath.Abs(baseConfigPath) + absConfigFile, err := filepath.Abs(configFile) if err != nil { - return fmt.Errorf("failed to get absolute path of config file directory:\n%w", err) + return fmt.Errorf("failed to get absolute path of config file:\n%w", err) } err = createNewImage( - ctx, buildDir, absBaseConfigPath, config, rpmsSources, outputImageFile, + ctx, buildDir, absConfigFile, config, rpmsSources, outputImageFile, outputImageFormat, toolsTar, distro, distroVersion, packageSnapshotTime) if err != nil { return err @@ -43,12 +41,12 @@ func CreateImageWithConfigFile(ctx context.Context, buildDir string, configFile return nil } -func createNewImage(ctx context.Context, buildDir string, baseConfigPath string, config imagecustomizerapi.Config, +func createNewImage(ctx context.Context, buildDir string, configFile string, config imagecustomizerapi.Config, rpmsSources []string, outputImageFile string, outputImageFormat string, toolsTar string, distro string, distroVersion string, packageSnapshotTime string, ) error { rc, err := validateConfig( - ctx, baseConfigPath, &config, rpmsSources, toolsTar, outputImageFile, + ctx, configFile, &config, rpmsSources, toolsTar, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) if err != nil { return err diff --git a/toolkit/tools/pkg/imagecreatorlib/validation.go b/toolkit/tools/pkg/imagecreatorlib/validation.go index ec7e4c07df..a43c7470b9 100644 --- a/toolkit/tools/pkg/imagecreatorlib/validation.go +++ b/toolkit/tools/pkg/imagecreatorlib/validation.go @@ -66,22 +66,22 @@ func validateSupportedOsFields(osConfig *imagecustomizerapi.OS) error { return nil } -func validateConfig(ctx context.Context, baseConfigPath string, config *imagecustomizerapi.Config, rpmsSources []string, +func validateConfig(ctx context.Context, configFile string, config *imagecustomizerapi.Config, rpmsSources []string, toolsTar string, outputImageFile, outputImageFormat string, packageSnapshotTime string, buildDir string, ) (*imagecustomizerlib.ResolvedConfig, error) { err := validateSupportedFields(config) if err != nil { - return nil, fmt.Errorf("invalid config file (%s):\n%w", baseConfigPath, err) + return nil, fmt.Errorf("invalid config file (%s):\n%w", configFile, err) } // Validate mandatory fields for creating a seed image - err = validateMandatoryFields(baseConfigPath, config, rpmsSources, toolsTar) + err = validateMandatoryFields(configFile, config, rpmsSources, toolsTar) if err != nil { return nil, err } // TODO: Validate for distro and release - rc, err := imagecustomizerlib.ValidateConfig(ctx, baseConfigPath, config, true, + rc, err := imagecustomizerlib.ValidateConfig(ctx, configFile, config, true, imagecustomizerlib.ImageCustomizerOptions{ RpmsSources: rpmsSources, OutputImageFile: outputImageFile, @@ -100,10 +100,10 @@ func validateConfig(ctx context.Context, baseConfigPath string, config *imagecus return rc, nil } -func validateMandatoryFields(baseConfigPath string, config *imagecustomizerapi.Config, rpmsSources []string, toolsTar string) error { +func validateMandatoryFields(configFile string, config *imagecustomizerapi.Config, rpmsSources []string, toolsTar string) error { // check if storage disks is not empty for creating a seed image if len(config.Storage.Disks) == 0 { - return fmt.Errorf("storage.disks field is required in the config file (%s)", baseConfigPath) + return fmt.Errorf("storage.disks field is required in the config file (%s)", configFile) } // rpmSources must not be empty for creating a seed image diff --git a/toolkit/tools/pkg/imagecreatorlib/validation_test.go b/toolkit/tools/pkg/imagecreatorlib/validation_test.go index 894b0d99c8..75b8712e2e 100644 --- a/toolkit/tools/pkg/imagecreatorlib/validation_test.go +++ b/toolkit/tools/pkg/imagecreatorlib/validation_test.go @@ -97,21 +97,21 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { packageSnapshotTime := "" // The output image file can be sepcified as an argument without being in specified the config. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) outputImageFile = outputImageFileNewRelativeCwd // The output image file can be specified as an argument relative to the current working directory. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) outputImageFile = outputImageDir // The output image file, specified as an argument, must not be a directory. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") @@ -119,7 +119,7 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { outputImageFile = outputImageDirRelativeCwd // The above is also true for relative paths. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") @@ -127,14 +127,14 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { outputImageFile = outputImageFileExists // The output image file, specified as an argument, may be a file that already exists. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) outputImageFile = outputImageFileExistsRelativeCwd // The above is also true for relative paths. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) @@ -142,21 +142,21 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { config.Output.Image.Path = outputImageFileNew // The output image file cab be specified in the config without being specified as an argument. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) config.Output.Image.Path = outputImageFileNewRelativeConfig // The output image file can be specified in the config relative to the base config path. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file, specified in the config, must not be a directory. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") @@ -164,7 +164,7 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { config.Output.Image.Path = outputImageDirRelativeConfig // The above is also true for relative paths. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") @@ -172,14 +172,14 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { config.Output.Image.Path = outputImageFileExists // The output image file, specified in the config, may be a file that already exists. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) config.Output.Image.Path = outputImageFileExistsRelativeConfig // The above is also true for relative paths. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) @@ -187,20 +187,20 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { config.Output.Image.Path = outputImageFileNew // The output image file can be specified both as an argument and in the config. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file can even be invalid in the config if it is specified as an argument. - _, err = validateConfig(t.Context(), baseConfigPath, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.NoError(t, err) } func TestValidateConfig_EmptyConfig(t *testing.T) { - baseConfigPath := testDir + configFile := filepath.Join(testDir, "empty-config.yaml") config := &imagecustomizerapi.Config{} rpmSources := []string{} @@ -209,7 +209,7 @@ func TestValidateConfig_EmptyConfig(t *testing.T) { packageSnapshotTime := "" buildDir := "./" - _, err := validateConfig(t.Context(), baseConfigPath, config, rpmSources, "", outputImageFile, outputImageFormat, + _, err := validateConfig(t.Context(), configFile, config, rpmSources, "", outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.ErrorContains(t, err, "storage.disks field is required in the config file") } @@ -236,6 +236,7 @@ func TestValidateConfig_EmptyPackagestoInstall(t *testing.T) { assert.NoError(t, err) // Set the packages to install to an empty slice config.OS.Packages.Install = []string{} + _, err = validateConfig(t.Context(), configFile, &config, rpmSources, toolsFile, outputImageFile, outputImageFormat, packageSnapshotTime, buildDir) assert.ErrorContains(t, err, "no packages to install specified, please specify at least one package to install for a new image") diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go index e0dde4c140..7a631a2d5d 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go @@ -12,13 +12,14 @@ import ( type ConfigWithBasePath struct { Config *imagecustomizerapi.Config BaseConfigPath string + ConfigFileName string } func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithBasePath, error) { visited := make(map[string]bool) pathStack := []string{} - configChain, err := buildConfigChainHelper(ctx, rc.Config, rc.BaseConfigPath, visited, pathStack) + configChain, err := buildConfigChainHelper(ctx, rc.Config, rc.BaseConfigPath, rc.ConfigFileName, visited, pathStack) if err != nil { return nil, err } @@ -27,7 +28,7 @@ func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithBas } func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, configDir string, - visited map[string]bool, pathStack []string) ([]*ConfigWithBasePath, error) { + configFileName string, visited map[string]bool, pathStack []string) ([]*ConfigWithBasePath, error) { var chain []*ConfigWithBasePath @@ -55,7 +56,8 @@ func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, // Recurse into base config baseConfigDir := filepath.Dir(absPath) - subChain, err := buildConfigChainHelper(ctx, &baseCfg, baseConfigDir, visited, pathStack) + baseFileName := filepath.Base(absPath) + subChain, err := buildConfigChainHelper(ctx, &baseCfg, baseConfigDir, baseFileName, visited, pathStack) if err != nil { return nil, err } @@ -67,6 +69,7 @@ func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, chain = append(chain, &ConfigWithBasePath{ Config: cfg, BaseConfigPath: configDir, + ConfigFileName: configFileName, }) return chain, nil diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go index 85c0a45edc..3efddb20d9 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs_test.go @@ -32,7 +32,7 @@ func TestBaseConfigsInputAndOutput(t *testing.T) { err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) assert.NoError(t, err) - rc, err := ValidateConfig(t.Context(), testDir, &config, false, options) + rc, err := ValidateConfig(t.Context(), currentConfigFile, &config, false, options) assert.NoError(t, err) // Verify resolved values @@ -79,6 +79,22 @@ func TestBaseConfigsFullRun(t *testing.T) { outImageFilePath := filepath.Join(testTmpDir, "image.raw") currentConfigFile := filepath.Join(testDir, "hierarchical-config.yaml") + options := ImageCustomizerOptions{ + BuildDir: buildDir, + UseBaseImageRpmRepos: true, + } + + var config imagecustomizerapi.Config + err = imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) + assert.NoError(t, err) + + rc, err := ValidateConfig(t.Context(), currentConfigFile, &config, false, options) + assert.NoError(t, err) + + // Verify VerityPartitionsType is correctly resolved + assert.Equal(t, imagecustomizerapi.VerityPartitionsUsesConfig, rc.Storage.VerityPartitionsType, + "VerityPartitionsType should be VerityPartitionsUsesConfig since the top-level config defines verity partitions") + err = CustomizeImageWithConfigFile(t.Context(), buildDir, currentConfigFile, baseImage, nil, outImageFilePath, "raw", true, "") if !assert.NoError(t, err) { @@ -248,11 +264,12 @@ func TestBaseConfigsFullRun(t *testing.T) { assert.NoError(t, err, "get disk partitions") // Verify partitions - assert.Len(t, partitions, 4) - assert.Equal(t, "esp", partitions[1].PartLabel) - assert.Equal(t, "", partitions[2].PartLabel) - assert.Equal(t, "", partitions[3].PartLabel) - assert.Equal(t, "", partitions[4].PartLabel) + assert.Len(t, partitions, 4, "should have 4 partitions from top-level config") + expectedLabels := []string{"esp", "boot", "root", "roothash"} + actualLabels := []string{partitions[1].PartLabel, partitions[2].PartLabel, + partitions[3].PartLabel, partitions[4].PartLabel} + assert.ElementsMatch(t, expectedLabels, actualLabels, + "partition labels should match top-level config (esp, boot, root, roothash)") // Verify verity bootPath := filepath.Join(imageConnection.Chroot().RootDir(), "/boot") @@ -263,11 +280,6 @@ func TestBaseConfigsFullRun(t *testing.T) { "PARTUUID="+partitions[4].PartUuid, "root", "console=tty0", baseImageInfo, "panic-on-corruption", ) - err = imageConnection.CleanClose() - if !assert.NoError(t, err) { - return - } - } func TestValidateDisksThenResetUUID(t *testing.T) { @@ -281,18 +293,19 @@ func TestValidateDisksThenResetUUID(t *testing.T) { err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) assert.NoError(t, err) - baseConfigPath, _ := filepath.Split(currentConfigFile) - absBaseConfigPath, err := filepath.Abs(baseConfigPath) + absCurrentConfigFile, err := filepath.Abs(currentConfigFile) assert.NoError(t, err) options := ImageCustomizerOptions{ BuildDir: buildDir, } - _, err = ValidateConfig(t.Context(), absBaseConfigPath, &config, false, options) + _, err = ValidateConfig(t.Context(), absCurrentConfigFile, &config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "cannot specify 'resetPartitionsUuidsType'") + assert.ErrorContains(t, err, "hierarchical-config-storage-resetuuid.yaml") + assert.ErrorContains(t, err, "when a base config specifies '.storage.disks'") } func TestValidateDisksThenReinitVerity(t *testing.T) { @@ -306,16 +319,17 @@ func TestValidateDisksThenReinitVerity(t *testing.T) { err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) assert.NoError(t, err) - baseConfigPath, _ := filepath.Split(currentConfigFile) - absBaseConfigPath, err := filepath.Abs(baseConfigPath) + absCurrentConfigFile, err := filepath.Abs(currentConfigFile) assert.NoError(t, err) options := ImageCustomizerOptions{ BuildDir: buildDir, } - _, err = ValidateConfig(t.Context(), absBaseConfigPath, &config, false, options) + _, err = ValidateConfig(t.Context(), absCurrentConfigFile, &config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "cannot specify 'reinitializeVerity'") + assert.ErrorContains(t, err, "hierarchical-config-storage-reinitverity.yaml") + assert.ErrorContains(t, err, "when a base config specifies '.storage.disks'") } diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 1f03f1ab60..585dc77b33 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -46,14 +46,22 @@ var ( ErrOutputSelinuxPolicyPathIsFileConfig = NewImageCustomizerError("Validation:OutputSelinuxPolicyPathIsFileConfig", "path exists but is not a directory") ) -func ValidateConfig(ctx context.Context, baseConfigPath string, config *imagecustomizerapi.Config, +func ValidateConfig(ctx context.Context, configFilePath string, config *imagecustomizerapi.Config, newImage bool, options ImageCustomizerOptions, ) (*ResolvedConfig, error) { _, span := otel.GetTracerProvider().Tracer(OtelTracerName).Start(ctx, "validate_config") defer span.End() + // Derive base config path from config file path + baseConfigPath := filepath.Dir(configFilePath) + if configFilePath == "" { + // For programmatic configs without a file, use current directory + baseConfigPath = "." + } + rc := &ResolvedConfig{ BaseConfigPath: baseConfigPath, + ConfigFileName: filepath.Base(configFilePath), Config: config, Options: options, } @@ -135,7 +143,12 @@ func ValidateConfig(ctx context.Context, baseConfigPath string, config *imagecus rc.Hostname = resolveHostname(rc.ConfigChain) rc.SELinux = resolveSeLinux(rc.ConfigChain) - rc.Storage = resolveStorage(rc.ConfigChain) + + rc.Storage, err = resolveStorage(rc.ConfigChain) + if err != nil { + return nil, err + } + rc.BootLoader.ResetType = resolveBootLoaderResetType(rc.ConfigChain) rc.Uki = resolveUki(rc.ConfigChain) rc.KernelCommandLine = resolveKernelCommandLine(rc.ConfigChain) @@ -517,7 +530,7 @@ func validateIsoPxeCustomization(rc *ResolvedConfig) error { // While defining a storage configuration can work when the input image is // an iso, there is no obvious point of moving content between partitions // where all partitions get collapsed into the squashfs at the end. - if len(rc.Storage.Disks) > 0 { + if rc.Storage.CustomizePartitions() { return ErrCannotCustomizePartitionsOnIso } } @@ -528,21 +541,22 @@ func validateIsoPxeCustomization(rc *ResolvedConfig) error { func validateStorage(configChain []*ConfigWithBasePath) error { var baseHasDisks bool - for i, configWithBase := range configChain { + for _, configWithBase := range configChain { storage := configWithBase.Config.Storage - hasDisks := len(storage.Disks) > 0 + hasDisks := storage.CustomizePartitions() hasResetUUID := storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault hasReinitVerity := storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault if baseHasDisks { + configFullPath := filepath.Join(configWithBase.BaseConfigPath, configWithBase.ConfigFileName) if hasResetUUID { return fmt.Errorf( - "cannot specify 'resetPartitionsUuidsType' in config at layer %d when a base config specifies '.storage.disks'", i) + "cannot specify 'resetPartitionsUuidsType' in %s when a base config specifies '.storage.disks'", configFullPath) } if hasReinitVerity { return fmt.Errorf( - "cannot specify 'reinitializeVerity' in config at layer %d when a base config specifies '.storage.disks'", i) + "cannot specify 'reinitializeVerity' in %s when a base config specifies '.storage.disks'", configFullPath) } } @@ -555,6 +569,35 @@ func validateStorage(configChain []*ConfigWithBasePath) error { return nil } +func resolveStorage(configChain []*ConfigWithBasePath) (imagecustomizerapi.Storage, error) { + var resolvedStorage imagecustomizerapi.Storage + + for _, configWithBase := range slices.Backward(configChain) { + storage := configWithBase.Config.Storage + + // If current config has disks, it overrides all base configs + if storage.CustomizePartitions() { + return storage, nil + } + + // Otherwise, accumulate individual override fields + if storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault { + resolvedStorage.ResetPartitionsUuidsType = storage.ResetPartitionsUuidsType + } + + if storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault { + resolvedStorage.ReinitializeVerity = storage.ReinitializeVerity + } + + if len(storage.Verity) > 0 { + resolvedStorage.Verity = storage.Verity + resolvedStorage.VerityPartitionsType = storage.VerityPartitionsType + } + } + + return resolvedStorage, nil +} + func resolveOutputArtifacts(configChain []*ConfigWithBasePath) *imagecustomizerapi.Artifacts { var artifacts *imagecustomizerapi.Artifacts @@ -669,56 +712,3 @@ func resolveKernelCommandLine(configChain []*ConfigWithBasePath) imagecustomizer ExtraCommandLine: mergedArgs, } } - -func resolveStorage(configChain []*ConfigWithBasePath) imagecustomizerapi.Storage { - var resolvedStorage imagecustomizerapi.Storage - - for _, configWithBase := range slices.Backward(configChain) { - storage := configWithBase.Config.Storage - - if len(storage.Disks) > 0 { - // Override - resolvedStorage.Disks = storage.Disks - resolvedStorage.BootType = storage.BootType - resolvedStorage.FileSystems = storage.FileSystems - resolvedStorage.Verity = storage.Verity - return resolvedStorage - } - - // .storage.resetPartitionsUuidsType - override - if storage.ResetPartitionsUuidsType != imagecustomizerapi.ResetPartitionsUuidsTypeDefault { - resolvedStorage.ResetPartitionsUuidsType = storage.ResetPartitionsUuidsType - } - - // .storage.reinitializeVerity - override - if storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault { - resolvedStorage.ReinitializeVerity = storage.ReinitializeVerity - } - - // Recalculate VerityPartitionsType based on resolved storage - verityUsesConfigPartitions := false - verityUsesExistingPartitions := false - - for _, verity := range resolvedStorage.Verity { - if verity.DataDeviceId != "" || verity.HashDeviceId != "" { - verityUsesConfigPartitions = true - } - if verity.DataDevice != nil || verity.HashDevice != nil { - verityUsesExistingPartitions = true - } - } - - switch { - case verityUsesConfigPartitions && verityUsesExistingPartitions: - return imagecustomizerapi.Storage{} - case verityUsesConfigPartitions: - resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsUsesConfig - case verityUsesExistingPartitions: - resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsUsesExisting - default: - resolvedStorage.VerityPartitionsType = imagecustomizerapi.VerityPartitionsNone - } - } - - return resolvedStorage -} diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go b/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go index 916dfdd879..7d4ccadb1e 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizebootloader.go @@ -78,7 +78,7 @@ func hardResetBootLoader(ctx context.Context, rc *ResolvedConfig, imageConnectio var rootMountIdType imagecustomizerapi.MountIdentifierType var bootType imagecustomizerapi.BootType - if len(rc.Storage.Disks) > 0 { + if rc.Storage.CustomizePartitions() { rootFileSystem, foundRootFileSystem := sliceutils.FindValueFunc(rc.Storage.FileSystems, func(fileSystem imagecustomizerapi.FileSystem) bool { return fileSystem.MountPoint != nil && diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go index bc6a59d5ec..6f44b6e6a1 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizepartitions.go @@ -26,7 +26,7 @@ func customizePartitions(ctx context.Context, rc *ResolvedConfig, targetOS targe storage := rc.Storage switch { - case len(storage.Disks) > 0: + case storage.CustomizePartitions(): logger.Log.Infof("Customizing partitions") _, span := otel.GetTracerProvider().Tracer(OtelTracerName).Start(ctx, "customize_partitions") diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go index 65279a4402..abb20640be 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go @@ -153,7 +153,12 @@ func CustomizeImageWithConfigFileOptions(ctx context.Context, configFile string, return fmt.Errorf("%w:\n%w", ErrGetAbsoluteConfigPath, err) } - err = CustomizeImageOptions(ctx, absBaseConfigPath, &config, options) + absConfigFile, err := filepath.Abs(configFile) + if err != nil { + return fmt.Errorf("%w:\n%w", ErrGetAbsoluteConfigPath, err) + } + + err = CustomizeImageOptions(ctx, absBaseConfigPath, absConfigFile, &config, options) if err != nil { return err } @@ -174,7 +179,7 @@ func CustomizeImage(ctx context.Context, buildDir string, baseConfigPath string, inputImageFile string, rpmsSources []string, outputImageFile string, outputImageFormat string, useBaseImageRpmRepos bool, packageSnapshotTime string, ) (err error) { - return CustomizeImageOptions(ctx, baseConfigPath, config, ImageCustomizerOptions{ + return CustomizeImageOptions(ctx, baseConfigPath, "", config, ImageCustomizerOptions{ BuildDir: buildDir, InputImageFile: inputImageFile, RpmsSources: rpmsSources, @@ -185,7 +190,7 @@ func CustomizeImage(ctx context.Context, buildDir string, baseConfigPath string, }) } -func CustomizeImageOptions(ctx context.Context, baseConfigPath string, config *imagecustomizerapi.Config, +func CustomizeImageOptions(ctx context.Context, baseConfigPath string, configFilePath string, config *imagecustomizerapi.Config, options ImageCustomizerOptions, ) (err error) { ctx, span := otel.GetTracerProvider().Tracer(OtelTracerName).Start(ctx, "customize_image") @@ -209,7 +214,7 @@ func CustomizeImageOptions(ctx context.Context, baseConfigPath string, config *i span.End() }() - rc, err := ValidateConfig(ctx, baseConfigPath, config, false, options) + rc, err := ValidateConfig(ctx, configFilePath, config, false, options) if err != nil { return fmt.Errorf("%w:\n%w", ErrInvalidImageConfig, err) } diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go index 2c7a6dea23..a5e40e1001 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go @@ -138,7 +138,7 @@ func TestValidateConfig_CallsValidateInput(t *testing.T) { // Test that the input is being validated in validateConfig by // triggering an error in validateInput. - _, err := ValidateConfig(t.Context(), testDir, config, false, + _, err := ValidateConfig(t.Context(), "", config, false, ImageCustomizerOptions{ OutputImageFile: "./out/image.vhdx", OutputImageFormat: "vhdx", @@ -152,7 +152,7 @@ func TestValidateConfig_CallsValidateInput_NewImage(t *testing.T) { // Test that the input is being validated in validateConfig by // triggering an error in validateInput. - _, err := ValidateConfig(t.Context(), testDir, config, true, + _, err := ValidateConfig(t.Context(), "", config, true, ImageCustomizerOptions{ OutputImageFile: "./out/image.raw", OutputImageFormat: "raw", @@ -184,19 +184,19 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { } // The input image file can be specified as an argument without being specified in the config. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.InputImageFile = inputImageFileRealRelativeCwd // The input image file specified as an argument can be relative to the current working directory. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.InputImageFile = inputImageFileFake // The input image file, specified as an argument, must be a file. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "doesnotexist.xxx: no such file or directory") @@ -204,19 +204,19 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { config.Input.Image.Path = inputImageFileReal // The input image file can be specified in the config without being specified as an argument. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileRealRelativeConfig // The input image file specified in the config can be relative to the bash config path. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileFake // The input image file, specified in the config, must be a file. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "doesnotexist.xxx: no such file or directory") @@ -224,18 +224,18 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { config.Input.Image.Path = inputImageFileReal // The input image file can be specified both as an argument and in the config. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileFake // The input image file can even be invalid in the config if it is specified as an argument. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) } func TestValidateConfigValidAdditionalFiles(t *testing.T) { - _, err := ValidateConfig(t.Context(), testDir, + _, err := ValidateConfig(t.Context(), "", &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -260,7 +260,7 @@ func TestValidateConfigValidAdditionalFiles(t *testing.T) { } func TestValidateConfigMissingAdditionalFiles(t *testing.T) { - _, err := ValidateConfig(t.Context(), testDir, + _, err := ValidateConfig(t.Context(), "", &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -284,7 +284,7 @@ func TestValidateConfigMissingAdditionalFiles(t *testing.T) { } func TestValidateConfigdditionalFilesIsDir(t *testing.T) { - _, err := ValidateConfig(t.Context(), testDir, + _, err := ValidateConfig(t.Context(), "", &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -335,7 +335,6 @@ func TestValidateConfigScriptNonLocalFile(t *testing.T) { } func TestValidateConfig_CallsValidateOutput(t *testing.T) { - baseConfigPath := testDir config := &imagecustomizerapi.Config{ Input: imagecustomizerapi.Input{ Image: imagecustomizerapi.InputImage{ @@ -348,7 +347,7 @@ func TestValidateConfig_CallsValidateOutput(t *testing.T) { } // Test that the output is being validated in validateConfig by triggering an error in validateOutput. - _, err := ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err := ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "output image file must be specified") } @@ -401,91 +400,91 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { options.OutputImageFormat = imagecustomizerapi.ImageFormatType(filepath.Ext(options.OutputImageFile)[1:]) // The output image file can be sepcified as an argument without being in specified the config. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileNewRelativeCwd // The output image file can be specified as an argument relative to the current working directory. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageDir // The output image file, specified as an argument, must not be a directory. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") options.OutputImageFile = outputImageDirRelativeCwd // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") options.OutputImageFile = outputImageFileExists // The output image file, specified as an argument, may be a file that already exists. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileExistsRelativeCwd // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.OutputImageFile = "" config.Output.Image.Path = outputImageFileNew // The output image file cab be specified in the config without being specified as an argument. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageFileNewRelativeConfig // The output image file can be specified in the config relative to the base config path. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file, specified in the config, must not be a directory. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") config.Output.Image.Path = outputImageDirRelativeConfig // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") config.Output.Image.Path = outputImageFileExists // The output image file, specified in the config, may be a file that already exists. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageFileExistsRelativeConfig // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileNew config.Output.Image.Path = outputImageFileNew // The output image file can be specified both as an argument and in the config. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file can even be invalid in the config if it is specified as an argument. - _, err = ValidateConfig(t.Context(), baseConfigPath, config, false, options) + _, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) } @@ -833,7 +832,6 @@ func TestValidateConfig_InputImageFileSelection(t *testing.T) { assert.NoError(t, err) // Pass the input image file only in the config. - configPath := "config.yaml" config := &imagecustomizerapi.Config{ Input: imagecustomizerapi.Input{ Image: imagecustomizerapi.InputImage{ @@ -848,7 +846,7 @@ func TestValidateConfig_InputImageFileSelection(t *testing.T) { } // The input image file should be set to the value in the config. - rc, err := ValidateConfig(t.Context(), configPath, config, false, options) + rc, err := ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.InputImage.Path, inputImageFileAsConfig) assert.Equal(t, rc.InputFileExt(), "vhdx") @@ -859,7 +857,7 @@ func TestValidateConfig_InputImageFileSelection(t *testing.T) { options.InputImageFile = inputImageFileAsArg // The input image file should be set to the value passed as an argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.InputImage.Path, inputImageFileAsArg) assert.Equal(t, rc.InputFileExt(), "vhdx") @@ -869,7 +867,7 @@ func TestValidateConfig_InputImageFileSelection(t *testing.T) { config.Input.Image.Path = inputImageFileAsConfig // The input image file should be set to the value passed as an argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.InputImage.Path, inputImageFileAsArg) assert.Equal(t, rc.InputFileExt(), "vhdx") @@ -879,7 +877,7 @@ func TestValidateConfig_InputImageFileSelection(t *testing.T) { options.InputImageFile = inputImageFileIsoAsArg options.OutputImageFormat = "iso" options.OutputImageFile = "out/image.iso" - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.InputImage.Path, inputImageFileIsoAsArg) assert.Equal(t, rc.InputFileExt(), "iso") @@ -901,7 +899,6 @@ func TestValidateConfig_OutputImageFileSelection(t *testing.T) { err = file.Write("", inputImageFile) assert.NoError(t, err) - configPath := "config.yaml" config := &imagecustomizerapi.Config{} options := ImageCustomizerOptions{ @@ -911,14 +908,14 @@ func TestValidateConfig_OutputImageFileSelection(t *testing.T) { } // The output image file is not specified in the config or as an argument, so the output image file will be empty. - rc, err := ValidateConfig(t.Context(), configPath, config, false, options) + rc, err := ValidateConfig(t.Context(), "", config, false, options) assert.ErrorContains(t, err, "output image file must be specified") // Pass the output image file only in the config. config.Output.Image.Path = outputImageFilePathAsConfig // The output image file should be set to the value in the config. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFile, outputImageFilePathAsConfig) @@ -927,7 +924,7 @@ func TestValidateConfig_OutputImageFileSelection(t *testing.T) { options.OutputImageFile = outputImageFilePathAsArg // The output image file should be set to the value passed as an argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFile, outputImageFilePathAsArg) @@ -936,7 +933,7 @@ func TestValidateConfig_OutputImageFileSelection(t *testing.T) { // The output image file should be set to the value passed as an // argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFile, outputImageFilePathAsArg) } @@ -956,7 +953,6 @@ func TestValidateConfig_OutputImageFormatSelection(t *testing.T) { err = file.Write("", inputImageFile) assert.NoError(t, err) - configPath := "config.yaml" config := &imagecustomizerapi.Config{} options := ImageCustomizerOptions{ @@ -967,14 +963,14 @@ func TestValidateConfig_OutputImageFormatSelection(t *testing.T) { // The output image format is not specified in the config or as an // argument, so an error will be reported. - rc, err := ValidateConfig(t.Context(), configPath, config, false, options) + rc, err := ValidateConfig(t.Context(), "", config, false, options) assert.ErrorContains(t, err, "output image format must be specified") // Pass the output image format only in the config. config.Output.Image.Format = outputImageFormatAsConfig // The output image file should be set to the value in the config. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFormat, outputImageFormatAsConfig) @@ -984,7 +980,7 @@ func TestValidateConfig_OutputImageFormatSelection(t *testing.T) { // The output image file should be set to the value passed as an // argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFormat, outputImageFormatAsArg) @@ -993,7 +989,7 @@ func TestValidateConfig_OutputImageFormatSelection(t *testing.T) { // The output image file should be set to the value passed as an // argument. - rc, err = ValidateConfig(t.Context(), configPath, config, false, options) + rc, err = ValidateConfig(t.Context(), "", config, false, options) assert.NoError(t, err) assert.Equal(t, rc.OutputImageFormat, outputImageFormatAsArg) } diff --git a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go index c60456dd0a..28281acd94 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go +++ b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go @@ -15,6 +15,7 @@ import ( type ResolvedConfig struct { // Configurations BaseConfigPath string + ConfigFileName string Config *imagecustomizerapi.Config Options ImageCustomizerOptions CustomizeOSPartitions bool diff --git a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml index c320508d64..82f3bbc724 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml +++ b/toolkit/tools/pkg/imagecustomizerlib/testdata/hierarchical-config.yaml @@ -3,16 +3,20 @@ storage: - partitionTableType: gpt partitions: - id: esp + label: esp type: esp size: 100M - id: boot + label: boot size: 100M - id: root + label: root size: 2G - id: roothash + label: roothash size: 100M bootType: efi From 45cc8acb91905741a63912f9f6410da7abb3d72f Mon Sep 17 00:00:00 2001 From: Elaine Zhao Date: Thu, 27 Nov 2025 00:34:28 +0000 Subject: [PATCH 7/7] updates --- toolkit/tools/imagecustomizerapi/config.go | 2 +- .../pkg/imagecustomizerlib/baseconfigs.go | 31 +++++----- .../imagecustomizerlib/configvalidation.go | 46 +++++++-------- .../pkg/imagecustomizerlib/customizeos.go | 14 ++--- .../pkg/imagecustomizerlib/imagecreator.go | 8 +-- .../pkg/imagecustomizerlib/imagecustomizer.go | 6 +- .../imagecustomizer_test.go | 58 +++++++++++-------- .../pkg/imagecustomizerlib/resolvedconfig.go | 10 +++- 8 files changed, 94 insertions(+), 81 deletions(-) diff --git a/toolkit/tools/imagecustomizerapi/config.go b/toolkit/tools/imagecustomizerapi/config.go index a4533e16ee..a68b112e1b 100644 --- a/toolkit/tools/imagecustomizerapi/config.go +++ b/toolkit/tools/imagecustomizerapi/config.go @@ -88,7 +88,7 @@ func (c *Config) IsValid() (err error) { return err } - if len(c.Storage.Disks) != 0 && !hasResetBootLoader { + if c.Storage.CustomizePartitions() && !hasResetBootLoader { return fmt.Errorf("'os.bootloader.reset' must be specified if 'storage.disks' is specified") } diff --git a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go index 7a631a2d5d..20ce4b1991 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go +++ b/toolkit/tools/pkg/imagecustomizerlib/baseconfigs.go @@ -9,17 +9,21 @@ import ( "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/file" ) -type ConfigWithBasePath struct { +type ConfigWithPath struct { Config *imagecustomizerapi.Config - BaseConfigPath string - ConfigFileName string + ConfigFilePath string } -func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithBasePath, error) { +// ConfigDir returns the directory containing the config file +func (c *ConfigWithPath) ConfigDir() string { + return filepath.Dir(c.ConfigFilePath) +} + +func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithPath, error) { visited := make(map[string]bool) pathStack := []string{} - configChain, err := buildConfigChainHelper(ctx, rc.Config, rc.BaseConfigPath, rc.ConfigFileName, visited, pathStack) + configChain, err := buildConfigChainHelper(ctx, rc.Config, rc.ConfigFilePath, visited, pathStack) if err != nil { return nil, err } @@ -27,10 +31,12 @@ func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithBas return configChain, nil } -func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, configDir string, - configFileName string, visited map[string]bool, pathStack []string) ([]*ConfigWithBasePath, error) { +func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, configFilePath string, + visited map[string]bool, pathStack []string) ([]*ConfigWithPath, error) { + + var chain []*ConfigWithPath - var chain []*ConfigWithBasePath + configDir := filepath.Dir(configFilePath) for _, base := range cfg.BaseConfigs { // Resolve base config path relative to current config's directory @@ -55,9 +61,7 @@ func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, } // Recurse into base config - baseConfigDir := filepath.Dir(absPath) - baseFileName := filepath.Base(absPath) - subChain, err := buildConfigChainHelper(ctx, &baseCfg, baseConfigDir, baseFileName, visited, pathStack) + subChain, err := buildConfigChainHelper(ctx, &baseCfg, absPath, visited, pathStack) if err != nil { return nil, err } @@ -66,10 +70,9 @@ func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, } // Add the current config last - chain = append(chain, &ConfigWithBasePath{ + chain = append(chain, &ConfigWithPath{ Config: cfg, - BaseConfigPath: configDir, - ConfigFileName: configFileName, + ConfigFilePath: configFilePath, }) return chain, nil diff --git a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go index 585dc77b33..ea2a8497d2 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go +++ b/toolkit/tools/pkg/imagecustomizerlib/configvalidation.go @@ -52,16 +52,8 @@ func ValidateConfig(ctx context.Context, configFilePath string, config *imagecus _, span := otel.GetTracerProvider().Tracer(OtelTracerName).Start(ctx, "validate_config") defer span.End() - // Derive base config path from config file path - baseConfigPath := filepath.Dir(configFilePath) - if configFilePath == "" { - // For programmatic configs without a file, use current directory - baseConfigPath = "." - } - rc := &ResolvedConfig{ - BaseConfigPath: baseConfigPath, - ConfigFileName: filepath.Base(configFilePath), + ConfigFilePath: configFilePath, Config: config, Options: options, } @@ -118,6 +110,8 @@ func ValidateConfig(ctx context.Context, configFilePath string, config *imagecus return nil, err } + baseConfigPath := rc.ConfigDir() + if !newImage { rc.InputImage, err = validateInput(rc.ConfigChain, options.InputImageFile, options.InputImage) if err != nil { @@ -187,7 +181,7 @@ func ValidateConfigPostImageDownload(rc *ResolvedConfig) error { return nil } -func validateInput(configChain []*ConfigWithBasePath, inputImageFile string, inputImage string, +func validateInput(configChain []*ConfigWithPath, inputImageFile string, inputImage string, ) (imagecustomizerapi.InputImage, error) { if inputImageFile != "" { if yes, err := file.IsFile(inputImageFile); err != nil { @@ -216,7 +210,7 @@ func validateInput(configChain []*ConfigWithBasePath, inputImageFile string, inp for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.Input.Image.Path != "" { inputImageAbsPath := file.GetAbsPathWithBase( - configWithBase.BaseConfigPath, + configWithBase.ConfigDir(), configWithBase.Config.Input.Image.Path, ) @@ -397,7 +391,7 @@ func validatePackageLists(baseConfigPath string, config *imagecustomizerapi.OS, return nil } -func validateOutputImageFormat(configChain []*ConfigWithBasePath, cliOutputImageFormat imagecustomizerapi.ImageFormatType, +func validateOutputImageFormat(configChain []*ConfigWithPath, cliOutputImageFormat imagecustomizerapi.ImageFormatType, ) (imagecustomizerapi.ImageFormatType, error) { if cliOutputImageFormat != "" { return cliOutputImageFormat, nil @@ -413,7 +407,7 @@ func validateOutputImageFormat(configChain []*ConfigWithBasePath, cliOutputImage return "", ErrOutputImageFormatRequired } -func validateOutputImageFile(configChain []*ConfigWithBasePath, cliOutputImageFile string, +func validateOutputImageFile(configChain []*ConfigWithPath, cliOutputImageFile string, outputImageFormat imagecustomizerapi.ImageFormatType, ) (string, error) { if cliOutputImageFile != "" { @@ -431,7 +425,7 @@ func validateOutputImageFile(configChain []*ConfigWithBasePath, cliOutputImageFi for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.Output.Image.Path != "" { outputImageFile := file.GetAbsPathWithBase( - configWithBase.BaseConfigPath, + configWithBase.ConfigDir(), configWithBase.Config.Output.Image.Path, ) @@ -479,7 +473,7 @@ func validateUser(baseConfigPath string, user imagecustomizerapi.User) error { return nil } -func validateOutputSelinuxPolicyPath(configChain []*ConfigWithBasePath, cliOutputSelinuxPolicyPath string) (string, error) { +func validateOutputSelinuxPolicyPath(configChain []*ConfigWithPath, cliOutputSelinuxPolicyPath string) (string, error) { // CLI parameter takes precedence. if cliOutputSelinuxPolicyPath != "" { if isDir, err := file.DirExists(cliOutputSelinuxPolicyPath); err != nil { @@ -496,7 +490,7 @@ func validateOutputSelinuxPolicyPath(configChain []*ConfigWithBasePath, cliOutpu for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.Output.SelinuxPolicyPath != "" { outputSelinuxPolicyPath := file.GetAbsPathWithBase( - configWithBase.BaseConfigPath, + configWithBase.ConfigDir(), configWithBase.Config.Output.SelinuxPolicyPath, ) @@ -538,7 +532,7 @@ func validateIsoPxeCustomization(rc *ResolvedConfig) error { return nil } -func validateStorage(configChain []*ConfigWithBasePath) error { +func validateStorage(configChain []*ConfigWithPath) error { var baseHasDisks bool for _, configWithBase := range configChain { @@ -549,7 +543,7 @@ func validateStorage(configChain []*ConfigWithBasePath) error { hasReinitVerity := storage.ReinitializeVerity != imagecustomizerapi.ReinitializeVerityTypeDefault if baseHasDisks { - configFullPath := filepath.Join(configWithBase.BaseConfigPath, configWithBase.ConfigFileName) + configFullPath := configWithBase.ConfigFilePath if hasResetUUID { return fmt.Errorf( "cannot specify 'resetPartitionsUuidsType' in %s when a base config specifies '.storage.disks'", configFullPath) @@ -569,7 +563,7 @@ func validateStorage(configChain []*ConfigWithBasePath) error { return nil } -func resolveStorage(configChain []*ConfigWithBasePath) (imagecustomizerapi.Storage, error) { +func resolveStorage(configChain []*ConfigWithPath) (imagecustomizerapi.Storage, error) { var resolvedStorage imagecustomizerapi.Storage for _, configWithBase := range slices.Backward(configChain) { @@ -598,7 +592,7 @@ func resolveStorage(configChain []*ConfigWithBasePath) (imagecustomizerapi.Stora return resolvedStorage, nil } -func resolveOutputArtifacts(configChain []*ConfigWithBasePath) *imagecustomizerapi.Artifacts { +func resolveOutputArtifacts(configChain []*ConfigWithPath) *imagecustomizerapi.Artifacts { var artifacts *imagecustomizerapi.Artifacts for _, configWithBase := range configChain { @@ -610,7 +604,7 @@ func resolveOutputArtifacts(configChain []*ConfigWithBasePath) *imagecustomizera // Artifacts path from current config overrides previous one if configWithBase.Config.Output.Artifacts.Path != "" { artifacts.Path = file.GetAbsPathWithBase( - configWithBase.BaseConfigPath, + configWithBase.ConfigDir(), configWithBase.Config.Output.Artifacts.Path, ) } @@ -650,7 +644,7 @@ func mergeOutputArtifactTypes(base, current []imagecustomizerapi.OutputArtifacts return merged } -func resolveHostname(configChain []*ConfigWithBasePath) string { +func resolveHostname(configChain []*ConfigWithPath) string { for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.OS != nil && configWithBase.Config.OS.Hostname != "" { return configWithBase.Config.OS.Hostname @@ -660,7 +654,7 @@ func resolveHostname(configChain []*ConfigWithBasePath) string { return "" } -func resolveSeLinux(configChain []*ConfigWithBasePath) imagecustomizerapi.SELinux { +func resolveSeLinux(configChain []*ConfigWithPath) imagecustomizerapi.SELinux { for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.OS != nil && configWithBase.Config.OS.SELinux.Mode != "" { return configWithBase.Config.OS.SELinux @@ -669,7 +663,7 @@ func resolveSeLinux(configChain []*ConfigWithBasePath) imagecustomizerapi.SELinu return imagecustomizerapi.SELinux{} } -func resolveUki(configChain []*ConfigWithBasePath) *imagecustomizerapi.Uki { +func resolveUki(configChain []*ConfigWithPath) *imagecustomizerapi.Uki { for _, configWithBase := range slices.Backward(configChain) { if configWithBase.Config.OS != nil && configWithBase.Config.OS.Uki != nil { return configWithBase.Config.OS.Uki @@ -678,7 +672,7 @@ func resolveUki(configChain []*ConfigWithBasePath) *imagecustomizerapi.Uki { return nil } -func resolveBootLoaderResetType(configChain []*ConfigWithBasePath) imagecustomizerapi.ResetBootLoaderType { +func resolveBootLoaderResetType(configChain []*ConfigWithPath) imagecustomizerapi.ResetBootLoaderType { for _, cfg := range slices.Backward(configChain) { if cfg.Config.OS == nil { continue @@ -697,7 +691,7 @@ func resolveBootLoaderResetType(configChain []*ConfigWithBasePath) imagecustomiz return "" } -func resolveKernelCommandLine(configChain []*ConfigWithBasePath) imagecustomizerapi.KernelCommandLine { +func resolveKernelCommandLine(configChain []*ConfigWithPath) imagecustomizerapi.KernelCommandLine { var mergedArgs []string // Concatenate all kernel command line args diff --git a/toolkit/tools/pkg/imagecustomizerlib/customizeos.go b/toolkit/tools/pkg/imagecustomizerlib/customizeos.go index 1356321504..04ca4b1aed 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/customizeos.go +++ b/toolkit/tools/pkg/imagecustomizerlib/customizeos.go @@ -35,7 +35,7 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection snapshotTime = rc.Options.PackageSnapshotTime } - err = addRemoveAndUpdatePackages(ctx, rc.BuildDirAbs, rc.BaseConfigPath, configWithBase.Config.OS, + err = addRemoveAndUpdatePackages(ctx, rc.BuildDirAbs, rc.ConfigDir(), configWithBase.Config.OS, imageChroot, nil, rc.Options.RpmsSources, rc.Options.UseBaseImageRpmRepos, distroHandler, snapshotTime) if err != nil { @@ -54,21 +54,21 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection return err } - err = AddOrUpdateUsers(ctx, configWithBase.Config.OS.Users, configWithBase.BaseConfigPath, imageChroot) + err = AddOrUpdateUsers(ctx, configWithBase.Config.OS.Users, configWithBase.ConfigDir(), imageChroot) if err != nil { return err } } for _, configWithBase := range rc.ConfigChain { - err = copyAdditionalDirs(ctx, configWithBase.BaseConfigPath, configWithBase.Config.OS.AdditionalDirs, imageChroot) + err = copyAdditionalDirs(ctx, configWithBase.ConfigDir(), configWithBase.Config.OS.AdditionalDirs, imageChroot) if err != nil { return err } } for _, configWithBase := range rc.ConfigChain { - err = copyAdditionalFiles(ctx, configWithBase.BaseConfigPath, configWithBase.Config.OS.AdditionalFiles, + err = copyAdditionalFiles(ctx, configWithBase.ConfigDir(), configWithBase.Config.OS.AdditionalFiles, imageChroot) if err != nil { return err @@ -95,7 +95,7 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection } if rc.Config.OS.ImageHistory != imagecustomizerapi.ImageHistoryNone { - err = addImageHistory(ctx, imageChroot, rc.ImageUuidStr, rc.BaseConfigPath, ToolVersion, buildTime, rc.Config) + err = addImageHistory(ctx, imageChroot, rc.ImageUuidStr, rc.ConfigDir(), ToolVersion, buildTime, rc.Config) if err != nil { return err } @@ -132,7 +132,7 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection } } - err = runUserScripts(ctx, rc.BaseConfigPath, rc.Config.Scripts.PostCustomization, "postCustomization", imageChroot) + err = runUserScripts(ctx, rc.ConfigDir(), rc.Config.Scripts.PostCustomization, "postCustomization", imageChroot) if err != nil { return err } @@ -152,7 +152,7 @@ func doOsCustomizations(ctx context.Context, rc *ResolvedConfig, imageConnection return err } - err = runUserScripts(ctx, rc.BaseConfigPath, rc.Config.Scripts.FinalizeCustomization, "finalizeCustomization", + err = runUserScripts(ctx, rc.ConfigDir(), rc.Config.Scripts.FinalizeCustomization, "finalizeCustomization", imageChroot) if err != nil { return err diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecreator.go b/toolkit/tools/pkg/imagecustomizerlib/imagecreator.go index c052840b4c..19fbd14abc 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecreator.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecreator.go @@ -18,7 +18,7 @@ import ( func CustomizeImageHelperImageCreator(ctx context.Context, rc *ResolvedConfig, tarFile string, distroHandler distroHandler, ) ([]fstabEntryPartNum, string, error) { - logger.Log.Debugf("Customizing OS image with config file %s", rc.BaseConfigPath) + logger.Log.Debugf("Customizing OS image with config file %s", rc.ConfigFilePath) toolsChrootDir := filepath.Join(rc.BuildDirAbs, toolsRoot) toolsChroot := safechroot.NewChroot(toolsChrootDir, false) @@ -87,7 +87,7 @@ func doOsCustomizationsImageCreator( snapshotTime = rc.Options.PackageSnapshotTime } - err = addRemoveAndUpdatePackages(ctx, rc.BuildDirAbs, rc.BaseConfigPath, configWithBase.Config.OS, + err = addRemoveAndUpdatePackages(ctx, rc.BuildDirAbs, rc.ConfigDir(), configWithBase.Config.OS, imageChroot, toolsChroot, rc.Options.RpmsSources, rc.Options.UseBaseImageRpmRepos, distroHandler, snapshotTime) if err != nil { @@ -115,7 +115,7 @@ func doOsCustomizationsImageCreator( return fmt.Errorf("failed to clear systemd state:\n%w", err) } - err = runUserScripts(ctx, rc.BaseConfigPath, rc.Config.Scripts.PostCustomization, "postCustomization", imageChroot) + err = runUserScripts(ctx, rc.ConfigDir(), rc.Config.Scripts.PostCustomization, "postCustomization", imageChroot) if err != nil { return err } @@ -128,7 +128,7 @@ func doOsCustomizationsImageCreator( return err } - err = runUserScripts(ctx, rc.BaseConfigPath, rc.Config.Scripts.FinalizeCustomization, "finalizeCustomization", imageChroot) + err = runUserScripts(ctx, rc.ConfigDir(), rc.Config.Scripts.FinalizeCustomization, "finalizeCustomization", imageChroot) if err != nil { return err } diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go index 11c95dd748..724e8b2937 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer.go @@ -285,7 +285,7 @@ func CustomizeImageOptions(ctx context.Context, baseConfigPath string, configFil } if rc.OutputArtifacts != nil { - outputDir := file.GetAbsPathWithBase(baseConfigPath, rc.OutputArtifacts.Path) + outputDir := file.GetAbsPathWithBase(rc.ConfigDir(), rc.OutputArtifacts.Path) err = outputArtifacts(ctx, rc.OutputArtifacts.Items, outputDir, rc.BuildDirAbs, rc.RawImageFile, im.verityMetadata) @@ -622,13 +622,13 @@ func convertWriteableFormatToOutputImage(ctx context.Context, rc *ResolvedConfig // Either re-build the full OS image, or just re-package the existing one if rebuildFullOsImage { requestedSELinuxMode := rc.SELinux.Mode - err := createLiveOSFromRaw(ctx, rc.BuildDirAbs, rc.BaseConfigPath, inputIsoArtifacts, requestedSELinuxMode, + err := createLiveOSFromRaw(ctx, rc.BuildDirAbs, rc.ConfigDir(), inputIsoArtifacts, requestedSELinuxMode, rc.Config.Iso, rc.Config.Pxe, rc.RawImageFile, rc.OutputImageFormat, rc.OutputImageFile) if err != nil { return fmt.Errorf("%w:\n%w", ErrCreateLiveOSArtifacts, err) } } else { - err := repackageLiveOS(rc.BuildDirAbs, rc.BaseConfigPath, rc.Config.Iso, rc.Config.Pxe, + err := repackageLiveOS(rc.BuildDirAbs, rc.ConfigDir(), rc.Config.Iso, rc.Config.Pxe, inputIsoArtifacts, rc.OutputImageFormat, rc.OutputImageFile) if err != nil { return fmt.Errorf("%w:\n%w", ErrCreateLiveOSArtifacts, err) diff --git a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go index ae9378061a..f1e8877343 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go +++ b/toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go @@ -167,6 +167,9 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { baseConfigPath := testDir config := &imagecustomizerapi.Config{} + // Use a config file path in testDir so relative paths resolve correctly + configFilePath := filepath.Join(baseConfigPath, "dummy-config.yaml") + inputImageFileFake := filepath.Join(testDir, "testimages", "doesnotexist.xxx") inputImageFileReal := filepath.Join(testDir, "testimages", "empty.vhdx") inputImageFileRealRelativeCwd, err := filepath.Rel(cwd, inputImageFileReal) @@ -204,19 +207,19 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { config.Input.Image.Path = inputImageFileReal // The input image file can be specified in the config without being specified as an argument. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileRealRelativeConfig // The input image file specified in the config can be relative to the bash config path. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileFake // The input image file, specified in the config, must be a file. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "doesnotexist.xxx: no such file or directory") @@ -224,18 +227,19 @@ func TestValidateInput_AcceptsValidPaths(t *testing.T) { config.Input.Image.Path = inputImageFileReal // The input image file can be specified both as an argument and in the config. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Input.Image.Path = inputImageFileFake // The input image file can even be invalid in the config if it is specified as an argument. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) } func TestValidateConfigValidAdditionalFiles(t *testing.T) { - _, err := ValidateConfig(t.Context(), "", + configFilePath := filepath.Join(testDir, "dummy-config.yaml") + _, err := ValidateConfig(t.Context(), configFilePath, &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -260,7 +264,8 @@ func TestValidateConfigValidAdditionalFiles(t *testing.T) { } func TestValidateConfigMissingAdditionalFiles(t *testing.T) { - _, err := ValidateConfig(t.Context(), "", + configFilePath := filepath.Join(testDir, "dummy-config.yaml") + _, err := ValidateConfig(t.Context(), configFilePath, &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -284,7 +289,8 @@ func TestValidateConfigMissingAdditionalFiles(t *testing.T) { } func TestValidateConfigdditionalFilesIsDir(t *testing.T) { - _, err := ValidateConfig(t.Context(), "", + configFilePath := filepath.Join(testDir, "dummy-config.yaml") + _, err := ValidateConfig(t.Context(), configFilePath, &imagecustomizerapi.Config{ OS: &imagecustomizerapi.OS{ AdditionalFiles: imagecustomizerapi.AdditionalFileList{ @@ -346,8 +352,11 @@ func TestValidateConfig_CallsValidateOutput(t *testing.T) { OutputImageFormat: imagecustomizerapi.ImageFormatTypeVhd, } + // Use a dummy config file path so relative input image path resolves correctly + configFilePath := filepath.Join(testDir, "dummy-config.yaml") + // Test that the output is being validated in validateConfig by triggering an error in validateOutput. - _, err := ValidateConfig(t.Context(), "", config, false, options) + _, err := ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "output image file must be specified") } @@ -372,6 +381,9 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { }, } + // Use a dummy config file path so relative paths are resolved correctly + configFilePath := filepath.Join(baseConfigPath, "dummy-config.yaml") + options := ImageCustomizerOptions{} outputImageDir := filepath.Join(testTempDir, "out") @@ -400,91 +412,91 @@ func TestValidateOutput_AcceptsValidPaths(t *testing.T) { options.OutputImageFormat = imagecustomizerapi.ImageFormatType(filepath.Ext(options.OutputImageFile)[1:]) // The output image file can be sepcified as an argument without being in specified the config. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileNewRelativeCwd // The output image file can be specified as an argument relative to the current working directory. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageDir // The output image file, specified as an argument, must not be a directory. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") options.OutputImageFile = outputImageDirRelativeCwd // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") options.OutputImageFile = outputImageFileExists // The output image file, specified as an argument, may be a file that already exists. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileExistsRelativeCwd // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) options.OutputImageFile = "" config.Output.Image.Path = outputImageFileNew // The output image file cab be specified in the config without being specified as an argument. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageFileNewRelativeConfig // The output image file can be specified in the config relative to the base config path. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file, specified in the config, must not be a directory. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") config.Output.Image.Path = outputImageDirRelativeConfig // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.Error(t, err) assert.ErrorContains(t, err, "is a directory") config.Output.Image.Path = outputImageFileExists // The output image file, specified in the config, may be a file that already exists. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageFileExistsRelativeConfig // The above is also true for relative paths. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) options.OutputImageFile = outputImageFileNew config.Output.Image.Path = outputImageFileNew // The output image file can be specified both as an argument and in the config. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) config.Output.Image.Path = outputImageDir // The output image file can even be invalid in the config if it is specified as an argument. - _, err = ValidateConfig(t.Context(), "", config, false, options) + _, err = ValidateConfig(t.Context(), configFilePath, config, false, options) assert.NoError(t, err) } diff --git a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go index 28281acd94..e119a1b8c6 100644 --- a/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go +++ b/toolkit/tools/pkg/imagecustomizerlib/resolvedconfig.go @@ -14,8 +14,7 @@ import ( // ResolvedConfig contains the final image configuration, including the merged CLI and config values. type ResolvedConfig struct { // Configurations - BaseConfigPath string - ConfigFileName string + ConfigFilePath string Config *imagecustomizerapi.Config Options ImageCustomizerOptions CustomizeOSPartitions bool @@ -62,7 +61,12 @@ type ResolvedConfig struct { Uki *imagecustomizerapi.Uki // Hierarchical config chain - ConfigChain []*ConfigWithBasePath + ConfigChain []*ConfigWithPath +} + +// ConfigDir returns the directory containing the config file +func (c *ResolvedConfig) ConfigDir() string { + return filepath.Dir(c.ConfigFilePath) } func (c *ResolvedConfig) InputFileExt() string {