Skip to content

Commit 1ca3fbf

Browse files
Implement Named Error Objects System for Image Customizer (#324)
This PR implements a comprehensive named error objects system for the Image Customizer to improve error telemetry and provide organized error messages for users. ## Key Changes **Named Error Objects Pattern** - Replaced centralized error categories and codes with locally-defined named error objects - Each error is defined in the file where it's used with descriptive names like `ErrCannotSetUidOnExistingUser` - Error objects follow the pattern: `NewImageCustomizerError("Module:ErrorType", "error message")` **Error Message Transformation** Updated all error handling to use proper wrapping patterns: ```go // Before return fmt.Errorf("cannot set UID (%d) on a user (%s) that already exists", *user.UID, user.Name) // After var ErrCannotSetUidOnExistingUser = NewImageCustomizerError("Users:SetUidOnExistingUser", "cannot set UID on a user that already exists") return fmt.Errorf("%w (UID='%d', user='%s')", ErrCannotSetUidOnExistingUser, *user.UID, user.Name) ``` **Telemetry Integration** - Simplified telemetry to only record `error.name` in span attributes - The deepest ImageCustomizerError in the error chain is used for the error name in the span **Test Compatibility** - Updated error object messages to align with existing test expectations - Ensured error messages match patterns expected by `assert.ErrorContains()` calls **Comprehensive Coverage** The system now covers all leaf errors across the entire imagecustomizerlib package, including: - Image processing and format conversion errors - Input/output validation errors - OS customization errors (packages, services, users, groups, etc.) - Verity and security feature errors - Environment and configuration validation errors The system maintains full functionality while providing better error categorization for telemetry and more descriptive error messages for end users. <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. --------- Co-authored-by: Adit Jha <[email protected]> Co-authored-by: Adit Jha <[email protected]>
1 parent 36b1c03 commit 1ca3fbf

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+861
-314
lines changed

toolkit/scripts/telemetry_hopper/telemetry_hopper.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4+
import json
45
import os
56
import logging
67
import grpc
@@ -159,12 +160,22 @@ def _create_spans(self, request) -> List[SpanData]:
159160
def extract_attribute_value(value_proto: Any) -> Optional[Any]:
160161
"""Extract value from protobuf AnyValue."""
161162
value_case = value_proto.WhichOneof("value")
163+
162164
value_mapping = {
163165
"string_value": value_proto.string_value,
164166
"int_value": value_proto.int_value,
165167
"double_value": value_proto.double_value,
166168
"bool_value": value_proto.bool_value,
167169
}
170+
171+
if value_case == "array_value":
172+
array_values = []
173+
for item in value_proto.array_value.values:
174+
item_value = extract_attribute_value(item)
175+
if item_value is not None:
176+
array_values.append(item_value)
177+
return json.dumps(array_values)
178+
168179
return value_mapping.get(value_case)
169180

170181

toolkit/tools/internal/userutils/userutils.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,7 @@ func userExistsHelper(name string, installChroot safechroot.ChrootInterface, gro
7171
_, stderr, err := shell.Execute("id", typeFlag, name)
7272
if err != nil {
7373
if !strings.Contains(stderr, "no such user") {
74-
typeStr := "user"
75-
if group {
76-
typeStr = "group"
77-
}
78-
79-
return fmt.Errorf("failed to check if %s exists (%s):\n%w", typeStr, name, err)
74+
return err
8075
}
8176

8277
userExists = false

toolkit/tools/pkg/imagecustomizerlib/artifactsinputoutput.go

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,29 @@ import (
2727
"golang.org/x/sys/unix"
2828
)
2929

30+
var (
31+
// Artifact handling errors
32+
ErrArtifactImageConnection = NewImageCustomizerError("Artifacts:ImageConnection", "failed to connect to image file to output artifacts")
33+
ErrArtifactESPPartitionMount = NewImageCustomizerError("Artifacts:ESPPartitionMount", "failed to mount ESP partition")
34+
ErrArtifactUKIDirectoryRead = NewImageCustomizerError("Artifacts:UKIDirectoryRead", "failed to read UKI directory")
35+
ErrArtifactBinaryCopy = NewImageCustomizerError("Artifacts:BinaryCopy", "failed to copy binary")
36+
ErrArtifactRootHashDump = NewImageCustomizerError("Artifacts:RootHashDump", "failed to dump root hash")
37+
ErrArtifactInjectFilesYamlWrite = NewImageCustomizerError("Artifacts:InjectFilesYamlWrite", "failed to write inject-files.yaml")
38+
ErrArtifactInjectFilesYamlMarshal = NewImageCustomizerError("Artifacts:InjectFilesYamlMarshal", "failed to marshal inject files metadata")
39+
ErrArtifactInvalidInjectFilesConfig = NewImageCustomizerError("Artifacts:InvalidInjectFilesConfig", "invalid inject files config")
40+
ErrArtifactInjectFilesPathResolution = NewImageCustomizerError("Artifacts:InjectFilesPathResolution", "failed to get absolute path of inject-files.yaml")
41+
ErrArtifactInjectFilesImageConnection = NewImageCustomizerError("Artifacts:InjectFilesImageConnection", "failed to connect to image file to inject files")
42+
ErrArtifactInjectFilesPartitionMount = NewImageCustomizerError("Artifacts:InjectFilesPartitionMount", "failed to mount partition for file injection")
43+
ErrArtifactPartitionUnmount = NewImageCustomizerError("Artifacts:PartitionUnmount", "failed to cleanly unmount partition")
44+
ErrArtifactCosiImageConversion = NewImageCustomizerError("Artifacts:CosiImageConversion", "failed to convert customized raw image to cosi output format")
45+
ErrArtifactOutputImageConversion = NewImageCustomizerError("Artifacts:OutputImageConversion", "failed to convert customized raw image to output format")
46+
ErrArtifactImageConnectionForExtraction = NewImageCustomizerError("Artifacts:ImageConnectionForExtraction", "failed to connect to image")
47+
ErrArtifactImageConnectionClose = NewImageCustomizerError("Artifacts:ImageConnectionClose", "failed to cleanly close image connection")
48+
ErrArtifactReleaseFileRead = NewImageCustomizerError("Artifacts:ReleaseFileRead", "failed to read release file")
49+
ErrArtifactImageUuidNotFound = NewImageCustomizerError("Artifacts:ImageUuidNotFound", "IMAGE_UUID not found in release file")
50+
ErrArtifactImageUuidParse = NewImageCustomizerError("Artifacts:ImageUuidParse", "failed to parse IMAGE_UUID")
51+
)
52+
3053
const (
3154
ShimDir = "EFI/BOOT"
3255
SystemdBootDir = "EFI/systemd"
@@ -46,7 +69,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
4669

4770
loopback, err := safeloopback.NewLoopback(buildImage)
4871
if err != nil {
49-
return fmt.Errorf("failed to connect to image file to output artifacts:\n%w", err)
72+
return fmt.Errorf("%w:\n%w", ErrArtifactImageConnection, err)
5073
}
5174
defer loopback.Close()
5275

@@ -69,7 +92,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
6992
systemBootPartitionMount, err := safemount.NewMount(systemBootPartition.Path,
7093
systemBootPartitionTmpDir, systemBootPartition.FileSystemType, unix.MS_RDONLY, "", true)
7194
if err != nil {
72-
return fmt.Errorf("failed to mount esp partition (%s):\n%w", systemBootPartition.Path, err)
95+
return fmt.Errorf("%w (partition='%s'):\n%w", ErrArtifactESPPartitionMount, systemBootPartition.Path, err)
7396
}
7497
defer systemBootPartitionMount.Close()
7598

@@ -94,7 +117,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
94117
ukiDir := filepath.Join(systemBootPartitionTmpDir, UkiOutputDir)
95118
dirEntries, err := os.ReadDir(ukiDir)
96119
if err != nil {
97-
return fmt.Errorf("failed to read UKI directory (%s):\n%w", ukiDir, err)
120+
return fmt.Errorf("%w (directory='%s'):\n%w", ErrArtifactUKIDirectoryRead, ukiDir, err)
98121
}
99122

100123
for _, entry := range dirEntries {
@@ -103,7 +126,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
103126
destPath := filepath.Join(outputDir, entry.Name())
104127
err := file.Copy(srcPath, destPath)
105128
if err != nil {
106-
return fmt.Errorf("failed to copy binary from (%s) to (%s):\n%w", srcPath, destPath, err)
129+
return fmt.Errorf("%w (source='%s', destination='%s'):\n%w", ErrArtifactBinaryCopy, srcPath, destPath, err)
107130
}
108131

109132
signedName := replaceSuffix(entry.Name(), ".efi", ".signed.efi")
@@ -126,7 +149,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
126149
destPath := filepath.Join(outputDir, bootConfig.bootBinary)
127150
err := file.Copy(srcPath, destPath)
128151
if err != nil {
129-
return fmt.Errorf("failed to copy binary from (%s) to (%s):\n%w", srcPath, destPath, err)
152+
return fmt.Errorf("%w (source='%s', destination='%s'):\n%w", ErrArtifactBinaryCopy, srcPath, destPath, err)
130153
}
131154

132155
signedPath := "./" + replaceSuffix(bootConfig.bootBinary, ".efi", ".signed.efi")
@@ -145,7 +168,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
145168
destPath := filepath.Join(outputDir, bootConfig.systemdBootBinary)
146169
err := file.Copy(srcPath, destPath)
147170
if err != nil {
148-
return fmt.Errorf("failed to copy binary from (%s) to (%s):\n%w", srcPath, destPath, err)
171+
return fmt.Errorf("%w (source='%s', destination='%s'):\n%w", ErrArtifactBinaryCopy, srcPath, destPath, err)
149172
}
150173

151174
signedPath := "./" + replaceSuffix(bootConfig.systemdBootBinary, ".efi", ".signed.efi")
@@ -169,7 +192,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
169192
destPath := filepath.Join(outputDir, unsignedHashFile)
170193
err = file.Write(verity.rootHash, destPath)
171194
if err != nil {
172-
return fmt.Errorf("failed to dump root hash for (%s) to (%s):\n%w", verity.name, destPath, err)
195+
return fmt.Errorf("%w (name='%s', path='%s'):\n%w", ErrArtifactRootHashDump, verity.name, destPath, err)
173196
}
174197

175198
signedHashFile := replaceSuffix(unsignedHashFile, ".hash", ".hash.sig")
@@ -185,7 +208,7 @@ func outputArtifacts(ctx context.Context, items []imagecustomizerapi.OutputArtif
185208

186209
err = writeInjectFilesYaml(outputArtifactsMetadata, outputDir)
187210
if err != nil {
188-
return fmt.Errorf("failed to write inject-files.yaml:\n%w", err)
211+
return fmt.Errorf("%w (outputDir='%s'):\n%w", ErrArtifactInjectFilesYamlWrite, outputDir, err)
189212
}
190213

191214
err = systemBootPartitionMount.CleanClose()
@@ -216,12 +239,12 @@ func writeInjectFilesYaml(metadata []imagecustomizerapi.InjectArtifactMetadata,
216239

217240
yamlBytes, err := yaml.Marshal(&yamlStruct)
218241
if err != nil {
219-
return fmt.Errorf("failed to marshal inject files metadata: %w", err)
242+
return fmt.Errorf("%w:\n%w", ErrArtifactInjectFilesYamlMarshal, err)
220243
}
221244

222245
outputFilePath := filepath.Join(outputDir, "inject-files.yaml")
223246
if err := os.WriteFile(outputFilePath, yamlBytes, 0o644); err != nil {
224-
return fmt.Errorf("failed to write inject-files.yaml: %w", err)
247+
return fmt.Errorf("%w (file='%s'):\n%w", ErrArtifactInjectFilesYamlWrite, outputFilePath, err)
225248
}
226249

227250
return nil
@@ -237,14 +260,14 @@ func InjectFilesWithConfigFile(ctx context.Context, buildDir string, configFile
237260
}
238261

239262
if err := injectConfig.IsValid(); err != nil {
240-
return fmt.Errorf("invalid inject files config: %w", err)
263+
return fmt.Errorf("%w:\n%w", ErrArtifactInvalidInjectFilesConfig, err)
241264
}
242265

243266
baseConfigPath, _ := filepath.Split(configFile)
244267

245268
absBaseConfigPath, err := filepath.Abs(baseConfigPath)
246269
if err != nil {
247-
return fmt.Errorf("failed to get absolute path of inject-files.yaml:\n%w", err)
270+
return fmt.Errorf("%w (path='%s'):\n%w", ErrArtifactInjectFilesPathResolution, baseConfigPath, err)
248271
}
249272

250273
err = InjectFiles(ctx, buildDir, absBaseConfigPath, inputImageFile, injectConfig.InjectFiles,
@@ -286,7 +309,7 @@ func InjectFiles(ctx context.Context, buildDir string, baseConfigPath string, in
286309

287310
loopback, err := safeloopback.NewLoopback(rawImageFile)
288311
if err != nil {
289-
return fmt.Errorf("failed to connect to image file to inject files:\n%w", err)
312+
return fmt.Errorf("%w:\n%w", ErrArtifactInjectFilesImageConnection, err)
290313
}
291314
defer loopback.Close()
292315

@@ -310,7 +333,7 @@ func InjectFiles(ctx context.Context, buildDir string, baseConfigPath string, in
310333

311334
mount, err := safemount.NewMount(partition.Path, partitionsToMountpoints[partitionKey], partition.FileSystemType, 0, "", true)
312335
if err != nil {
313-
return fmt.Errorf("failed to mount partition (%s):\n%w", partition.Path, err)
336+
return fmt.Errorf("%w (partition='%s'):\n%w", ErrArtifactInjectFilesPartitionMount, partition.Path, err)
314337
}
315338
defer mount.Close()
316339

@@ -321,13 +344,13 @@ func InjectFiles(ctx context.Context, buildDir string, baseConfigPath string, in
321344
destPath := filepath.Join(partitionsToMountpoints[partitionKey], item.Destination)
322345
err := file.Copy(srcPath, destPath)
323346
if err != nil {
324-
return fmt.Errorf("failed to copy binary from (%s) to (%s):\n%w", srcPath, destPath, err)
347+
return fmt.Errorf("%w (source='%s', destination='%s'):\n%w", ErrArtifactBinaryCopy, srcPath, destPath, err)
325348
}
326349
}
327350

328351
for _, m := range mountedPartitions {
329352
if err := m.CleanClose(); err != nil {
330-
return fmt.Errorf("failed to cleanly unmount (%s):\n%w", m.Target(), err)
353+
return fmt.Errorf("%w (target='%s'):\n%w", ErrArtifactPartitionUnmount, m.Target(), err)
331354
}
332355
}
333356

@@ -345,12 +368,12 @@ func InjectFiles(ctx context.Context, buildDir string, baseConfigPath string, in
345368
err = convertToCosi(buildDirAbs, rawImageFile, outputImageFile, partUuidToFstabEntry,
346369
baseImageVerityMetadata, osRelease, osPackages, imageUuid, imageUuidStr, cosiBootMetadata)
347370
if err != nil {
348-
return fmt.Errorf("failed to convert customized raw image to cosi output format:\n%w", err)
371+
return fmt.Errorf("%w (output='%s'):\n%w", ErrArtifactCosiImageConversion, outputImageFile, err)
349372
}
350373
} else {
351374
err = ConvertImageFile(rawImageFile, outputImageFile, detectedImageFormat)
352375
if err != nil {
353-
return fmt.Errorf("failed to convert customized raw image to output format:\n%w", err)
376+
return fmt.Errorf("%w (output='%s', format='%s'):\n%w", ErrArtifactOutputImageConversion, outputImageFile, detectedImageFormat, err)
354377
}
355378
}
356379

@@ -367,7 +390,7 @@ func prepareImageConversionData(ctx context.Context, rawImageFile string, buildD
367390
imageConnection, partUuidToFstabEntry, baseImageVerityMetadata, _, err := connectToExistingImage(ctx,
368391
rawImageFile, buildDir, chrootDir, true, true, false)
369392
if err != nil {
370-
return nil, nil, "", nil, [randomization.UuidSize]byte{}, "", nil, fmt.Errorf("failed to connect to image:\n%w", err)
393+
return nil, nil, "", nil, [randomization.UuidSize]byte{}, "", nil, fmt.Errorf("%w:\n%w", ErrArtifactImageConnectionForExtraction, err)
371394
}
372395
defer imageConnection.Close()
373396

@@ -387,7 +410,7 @@ func prepareImageConversionData(ctx context.Context, rawImageFile string, buildD
387410
}
388411

389412
if err := imageConnection.CleanClose(); err != nil {
390-
return nil, nil, "", nil, [randomization.UuidSize]byte{}, "", nil, fmt.Errorf("failed to cleanly close image connection:\n%w", err)
413+
return nil, nil, "", nil, [randomization.UuidSize]byte{}, "", nil, fmt.Errorf("%w:\n%w", ErrArtifactImageConnectionClose, err)
391414
}
392415

393416
return partUuidToFstabEntry, baseImageVerityMetadata, osRelease, osPackages, imageUuid, imageUuidStr, cosiBootMetadata, nil
@@ -399,22 +422,22 @@ func extractImageUUID(imageConnection *imageconnection.ImageConnection) ([random
399422
releasePath := filepath.Join(imageConnection.Chroot().RootDir(), ImageCustomizerReleasePath)
400423
data, err := file.Read(releasePath)
401424
if err != nil {
402-
return emptyUuid, "", fmt.Errorf("failed to read %s:\n%w", releasePath, err)
425+
return emptyUuid, "", fmt.Errorf("%w (path='%s'):\n%w", ErrArtifactReleaseFileRead, releasePath, err)
403426
}
404427

405428
lines := strings.Split(string(data), "\n")
406429
line, found := sliceutils.FindValueFunc(lines, func(line string) bool {
407430
return strings.HasPrefix(line, "IMAGE_UUID=")
408431
})
409432
if !found {
410-
return emptyUuid, "", fmt.Errorf("IMAGE_UUID not found in %s", releasePath)
433+
return emptyUuid, "", fmt.Errorf("%w (path='%s')", ErrArtifactImageUuidNotFound, releasePath)
411434
}
412435

413436
uuidStr := strings.Trim(strings.TrimPrefix(line, "IMAGE_UUID="), `"`)
414437

415438
parsed, err := randomization.ParseUuidString(uuidStr)
416439
if err != nil {
417-
return emptyUuid, "", fmt.Errorf("failed to parse IMAGE_UUID (%s):\n%w", uuidStr, err)
440+
return emptyUuid, "", fmt.Errorf("%w (IMAGE_UUID='%s'):\n%w", ErrArtifactImageUuidParse, uuidStr, err)
418441
}
419442

420443
return parsed, uuidStr, nil

toolkit/tools/pkg/imagecustomizerlib/bootcustomizer.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ import (
1111
"github.com/microsoft/azurelinux/toolkit/tools/internal/safechroot"
1212
)
1313

14+
var (
15+
// Boot customization errors
16+
ErrBootGrubMkconfigGeneration = NewImageCustomizerError("Boot:GrubMkconfigGeneration", "failed to generate grub.cfg via grub2-mkconfig")
17+
)
18+
1419
type BootCustomizer struct {
1520
// The contents of the /boot/grub2/grub.cfg file.
1621
grubCfgContent string
@@ -208,7 +213,7 @@ func (b *BootCustomizer) WriteToFile(imageChroot safechroot.ChrootInterface) err
208213
// Update /boot/grub2/grub.cfg file.
209214
err = installutils.CallGrubMkconfig(imageChroot)
210215
if err != nil {
211-
return fmt.Errorf("failed to generate grub.cfg via grub2-mkconfig:\n%w", err)
216+
return fmt.Errorf("%w:\n%w", ErrBootGrubMkconfigGeneration, err)
212217
}
213218
} else {
214219
// Update grub.cfg file.

toolkit/tools/pkg/imagecustomizerlib/checkfilesystem.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ import (
1515
"go.opentelemetry.io/otel"
1616
)
1717

18+
var (
19+
// Filesystem check errors
20+
ErrFilesystemE2fsckCheck = NewImageCustomizerError("FilesystemCheck:E2fsck", "failed to check filesystem with e2fsck")
21+
ErrFilesystemXfsRepairCheck = NewImageCustomizerError("FilesystemCheck:XfsRepair", "failed to check filesystem with xfs_repair")
22+
ErrFilesystemFsckCheck = NewImageCustomizerError("FilesystemCheck:Fsck", "failed to check filesystem with fsck")
23+
)
24+
1825
func checkFileSystems(ctx context.Context, rawImageFile string) error {
1926
logger.Log.Infof("Checking for file system errors")
2027

@@ -108,21 +115,20 @@ func checkFileSystem(fileSystemType string, path string) error {
108115
// Add -f flag to force check to run even if the journal is marked as clean.
109116
err := shell.ExecuteLive(true /*squashErrors*/, "e2fsck", "-fn", path)
110117
if err != nil {
111-
return fmt.Errorf("failed to check (%s) with e2fsck:\n%w", path, err)
112-
118+
return fmt.Errorf("%w (path='%s'):\n%w", ErrFilesystemE2fsckCheck, path, err)
113119
}
114120

115121
case "xfs":
116122
// The fsck.xfs tool doesn't do anything. So, call xfs_repair instead.
117123
err := shell.ExecuteLive(true /*squashErrors*/, "xfs_repair", "-n", path)
118124
if err != nil {
119-
return fmt.Errorf("failed to check (%s) with xfs_repair:\n%w", path, err)
125+
return fmt.Errorf("%w (path='%s'):\n%w", ErrFilesystemXfsRepairCheck, path, err)
120126
}
121127

122128
default:
123129
err := shell.ExecuteLive(true /*squashErrors*/, "fsck", "-n", path)
124130
if err != nil {
125-
return fmt.Errorf("failed to check (%s) with fsck:\n%w", path, err)
131+
return fmt.Errorf("%w (path='%s'):\n%w", ErrFilesystemFsckCheck, path, err)
126132
}
127133
}
128134

0 commit comments

Comments
 (0)