Skip to content

Commit

Permalink
Merge pull request #1412 from prestist/path-validation
Browse files Browse the repository at this point in the history 
filesystem.go: add warning when path matches device
  • Loading branch information
@prestist
prestist authored Oct 25, 2022
2 parents 83f9b3c + b099378 commit bc00008
Show file tree
Hide file tree
Showing 12 changed files with 653 additions and 10 deletions.
2 changes: 2 additions & 0 deletions config/shared/errors/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ var (
ErrNoPath = errors.New("path not specified")
ErrPathRelative = errors.New("path not absolute")
ErrDirtyPath = errors.New("path is not fully simplified")
ErrPartitionsOverwritten = errors.New("filesystem overwrites partitioned device")
ErrFilesystemImplicitWipe = errors.New("device matches disk with wipeTable enabled; filesystem will be wiped")
ErrRaidLevelRequired = errors.New("raid level is required")
ErrSparesUnsupportedForLevel = errors.New("spares unsupported for linear and raid0 arrays")
ErrUnrecognizedRaidLevel = errors.New("unrecognized raid level")
Expand Down
33 changes: 32 additions & 1 deletion config/v3_0/types/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,34 @@ func (s Storage) MergedKeys() map[string]string {
}

func (s Storage) Validate(c vpath.ContextPath) (r report.Report) {
s.validateDirectories(c, &r)
s.validateFiles(c, &r)
s.validateLinks(c, &r)
s.validateFilesystems(c, &r)
return
}

func (s Storage) validateDirectories(c vpath.ContextPath, r *report.Report) {
for i, d := range s.Directories {
for _, l := range s.Links {
if strings.HasPrefix(d.Path, l.Path+"/") {
r.AddOnError(c.Append("directories", i), errors.ErrDirectoryUsedSymlink)
}
}
}
}

func (s Storage) validateFiles(c vpath.ContextPath, r *report.Report) {
for i, f := range s.Files {
for _, l := range s.Links {
if strings.HasPrefix(f.Path, l.Path+"/") {
r.AddOnError(c.Append("files", i), errors.ErrFileUsedSymlink)
}
}
}
}

func (s Storage) validateLinks(c vpath.ContextPath, r *report.Report) {
for i, l1 := range s.Links {
for _, l2 := range s.Links {
if strings.HasPrefix(l1.Path, l2.Path+"/") {
Expand Down Expand Up @@ -76,5 +90,22 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) {
ownerCheck(l1.Group.ID == nil, c.Append("links", i, "group", "id"))
ownerCheck(l1.Group.Name == nil, c.Append("links", i, "group", "name"))
}
return
}

func (s Storage) validateFilesystems(c vpath.ContextPath, r *report.Report) {
disks := make(map[string]Disk)
for _, d := range s.Disks {
disks[d.Device] = d
}

for i, f := range s.Filesystems {
disk, exist := disks[f.Device]
if exist {
if len(disk.Partitions) > 0 {
r.AddOnWarn(c.Append("filesystems", i, "device"), errors.ErrPartitionsOverwritten)
} else if !util.IsTrue(f.WipeFilesystem) && util.IsTrue(disk.WipeTable) {
r.AddOnWarn(c.Append("filesystems", i, "device"), errors.ErrFilesystemImplicitWipe)
}
}
}
}
98 changes: 97 additions & 1 deletion config/v3_0/types/storage_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ import (
"github.com/coreos/vcontext/report"
)

func TestStorageValidate(t *testing.T) {
func TestStorageValidateErrors(t *testing.T) {
tests := []struct {
in Storage
at path.ContextPath
err error
warn error
}{
// test empty storage config returns nil
{
in: Storage{},
},
// test a storage config with no conflicting paths returns nil
{
in: Storage{
Links: []Link{
Expand All @@ -57,6 +59,7 @@ func TestStorageValidate(t *testing.T) {
},
},
},
// test when a file uses a configured symlink path returns ErrFileUsedSymlink
{
in: Storage{
Links: []Link{
Expand All @@ -73,6 +76,7 @@ func TestStorageValidate(t *testing.T) {
err: errors.ErrFileUsedSymlink,
at: path.New("", "files", 0),
},
// test when a directory uses a configured symlink path returns ErrDirectoryUsedSymlink
{
in: Storage{
Links: []Link{
Expand All @@ -89,6 +93,7 @@ func TestStorageValidate(t *testing.T) {
err: errors.ErrDirectoryUsedSymlink,
at: path.New("", "directories", 0),
},
// test the same path listed for two separate symlinks returns ErrLinkUsedSymlink
{
in: Storage{
Links: []Link{
Expand All @@ -103,6 +108,7 @@ func TestStorageValidate(t *testing.T) {
err: errors.ErrLinkUsedSymlink,
at: path.New("", "links", 1),
},
// test that two symlinks can be configured at a time
{
in: Storage{
Links: []Link{
Expand All @@ -115,6 +121,7 @@ func TestStorageValidate(t *testing.T) {
},
},
},
// test when a directory uses a configured symlink with the 'Hard:= true' returns ErrHardLinkToDirectory
{
in: Storage{
Links: []Link{
Expand Down Expand Up @@ -227,3 +234,92 @@ func TestStorageValidate(t *testing.T) {
}
}
}

func TestStorageValidateWarnings(t *testing.T) {
tests := []struct {
in Storage
at path.ContextPath
out error
}{
// test a disk with partitions with the same 'device' as a filesystem returns ErrPartitionsOverwritten
{
in: Storage{
Disks: []Disk{
{
Device: "/dev/sda",
Partitions: []Partition{
{}, {},
},
},
},
Filesystems: []Filesystem{
{
Device: "/dev/sda",
},
},
},
out: errors.ErrPartitionsOverwritten,
at: path.New("", "filesystems", 0, "device"),
},
// test a disk with the same 'device' and 'WipeTable:=true' as a configured filesystem returns ErrFilesystemImplicitWipe
{
in: Storage{
Disks: []Disk{
{
Device: "/dev/sda",
WipeTable: util.BoolToPtr(true),
},
},
Filesystems: []Filesystem{
{
Device: "/dev/sda",
},
},
},
out: errors.ErrFilesystemImplicitWipe,
at: path.New("", "filesystems", 0, "device"),
},
// test a disk with the same 'device' and 'WipeTable:=false' as a configured filesystem returns nil
{
in: Storage{
Disks: []Disk{
{
Device: "/dev/sda",
WipeTable: util.BoolToPtr(false),
},
},
Filesystems: []Filesystem{
{
Device: "/dev/sda",
},
},
},
out: nil,
},
// test a disk with no partitions with the same 'device' as a filesystem returns nil
{
in: Storage{
Disks: []Disk{
{
Device: "/dev/sdb",
},
},
Filesystems: []Filesystem{
{
Device: "/dev/sdb",
},
},
},
out: nil,
},
}

for i, test := range tests {
r := test.in.Validate(path.ContextPath{})
expected := report.Report{}
expected.AddOnWarn(test.at, test.out)
if !reflect.DeepEqual(expected, r) {
t.Errorf("#%d: bad report: want %v, got %v", i, expected, r)
}
}
}
33 changes: 32 additions & 1 deletion config/v3_1/types/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,34 @@ func (s Storage) MergedKeys() map[string]string {
}

func (s Storage) Validate(c vpath.ContextPath) (r report.Report) {
s.validateDirectories(c, &r)
s.validateFiles(c, &r)
s.validateLinks(c, &r)
s.validateFilesystems(c, &r)
return
}

func (s Storage) validateDirectories(c vpath.ContextPath, r *report.Report) {
for i, d := range s.Directories {
for _, l := range s.Links {
if strings.HasPrefix(d.Path, l.Path+"/") {
r.AddOnError(c.Append("directories", i), errors.ErrDirectoryUsedSymlink)
}
}
}
}

func (s Storage) validateFiles(c vpath.ContextPath, r *report.Report) {
for i, f := range s.Files {
for _, l := range s.Links {
if strings.HasPrefix(f.Path, l.Path+"/") {
r.AddOnError(c.Append("files", i), errors.ErrFileUsedSymlink)
}
}
}
}

func (s Storage) validateLinks(c vpath.ContextPath, r *report.Report) {
for i, l1 := range s.Links {
for _, l2 := range s.Links {
if strings.HasPrefix(l1.Path, l2.Path+"/") {
Expand Down Expand Up @@ -76,5 +90,22 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) {
ownerCheck(l1.Group.ID == nil, c.Append("links", i, "group", "id"))
ownerCheck(l1.Group.Name == nil, c.Append("links", i, "group", "name"))
}
return
}

func (s Storage) validateFilesystems(c vpath.ContextPath, r *report.Report) {
disks := make(map[string]Disk)
for _, d := range s.Disks {
disks[d.Device] = d
}

for i, f := range s.Filesystems {
disk, exist := disks[f.Device]
if exist {
if len(disk.Partitions) > 0 {
r.AddOnWarn(c.Append("filesystems", i, "device"), errors.ErrPartitionsOverwritten)
} else if !util.IsTrue(f.WipeFilesystem) && util.IsTrue(disk.WipeTable) {
r.AddOnWarn(c.Append("filesystems", i, "device"), errors.ErrFilesystemImplicitWipe)
}
}
}
}
Loading

0 comments on commit bc00008

Please sign in to comment.