Skip to content

Commit

Permalink
bib: propagate error messages from policy.Check()
Browse files Browse the repository at this point in the history
The errors returned by policy.Check() can contain a reason for why a
path was rejected, especially when the rejection is not based on the
path policy itself.  For example, a non-absolute or non-canonical path
will be rejected.  This information is useful for the user, so let's not
discard that extra information and return all error messages we
encountered.

Co-authored-by: Michael Vogt <[email protected]>
  • Loading branch information
achilleas-k and mvo5 committed Aug 12, 2024
1 parent 48e9207 commit a9a80b8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
13 changes: 8 additions & 5 deletions bib/cmd/bootc-image-builder/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
cryptorand "crypto/rand"
"errors"
"fmt"
"math"
"math/big"
Expand Down Expand Up @@ -114,17 +115,19 @@ var (
)

func checkMountpoints(filesystems []blueprint.FilesystemCustomization, policy *pathpolicy.PathTrie) error {
invalid := make([]string, 0)
errs := []error{}
for _, fs := range filesystems {
if err := policy.Check(fs.Mountpoint); err != nil {
invalid = append(invalid, fs.Mountpoint)
errs = append(errs, err)
}
if fs.Mountpoint == "/var" {
invalid = append(invalid, "/var")
// this error message is consistent with the errors returned by policy.Check()
// TODO: remove trailing space inside the quoted path when the function is fixed in osbuild/images.
errs = append(errs, fmt.Errorf("path '/var ' is not allowed"))
}
}
if len(invalid) > 0 {
return fmt.Errorf("The following custom mountpoints are not supported %+q", invalid)
if len(errs) > 0 {
return fmt.Errorf("The following errors occurred while validating custom mountpoints:\n%w", errors.Join(errs...))
}
return nil
}
Expand Down
23 changes: 20 additions & 3 deletions bib/cmd/bootc-image-builder/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,40 @@ func TestCheckFilesystemCustomizationsValidates(t *testing.T) {
{Mountpoint: "/ostree"},
},
ptmode: disk.RawPartitioningMode,
expectedErr: `The following custom mountpoints are not supported ["/ostree"]`,
expectedErr: "The following errors occurred while validating custom mountpoints:\npath '/ostree ' is not allowed",
},
{
fsCust: []blueprint.FilesystemCustomization{
{Mountpoint: "/"},
{Mountpoint: "/var"},
},
ptmode: disk.RawPartitioningMode,
expectedErr: `The following custom mountpoints are not supported ["/var"]`,
expectedErr: "The following errors occurred while validating custom mountpoints:\npath '/var ' is not allowed",
},
{
fsCust: []blueprint.FilesystemCustomization{
{Mountpoint: "/"},
{Mountpoint: "/var/data"},
},
ptmode: disk.BtrfsPartitioningMode,
expectedErr: `The following custom mountpoints are not supported ["/var/data"]`,
expectedErr: "The following errors occurred while validating custom mountpoints:\npath '/var/data ' is not allowed",
},
{
fsCust: []blueprint.FilesystemCustomization{
{Mountpoint: "/"},
{Mountpoint: "/boot/"},
},
ptmode: disk.BtrfsPartitioningMode,
expectedErr: "The following errors occurred while validating custom mountpoints:\npath must be canonical",
},
{
fsCust: []blueprint.FilesystemCustomization{
{Mountpoint: "/"},
{Mountpoint: "/boot/"},
{Mountpoint: "/opt"},
},
ptmode: disk.BtrfsPartitioningMode,
expectedErr: "The following errors occurred while validating custom mountpoints:\npath must be canonical\npath '/opt ' is not allowed",
},
} {
cust := &blueprint.Customizations{
Expand Down

0 comments on commit a9a80b8

Please sign in to comment.