diff --git a/config/shared/errors/errors.go b/config/shared/errors/errors.go index 3ee52801b..9fa1645a7 100644 --- a/config/shared/errors/errors.go +++ b/config/shared/errors/errors.go @@ -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") diff --git a/config/v3_0/types/storage.go b/config/v3_0/types/storage.go index 04c9b5442..24668954b 100644 --- a/config/v3_0/types/storage.go +++ b/config/v3_0/types/storage.go @@ -34,6 +34,14 @@ 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+"/") { @@ -41,6 +49,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -48,6 +59,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -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) + } + } + } } diff --git a/config/v3_0/types/storage_test.go b/config/v3_0/types/storage_test.go index 0bffdf3ab..f106deb80 100644 --- a/config/v3_0/types/storage_test.go +++ b/config/v3_0/types/storage_test.go @@ -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{ @@ -57,6 +59,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a file uses a configured symlink path returns ErrFileUsedSymlink { in: Storage{ Links: []Link{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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) + } + } +} diff --git a/config/v3_1/types/storage.go b/config/v3_1/types/storage.go index 04c9b5442..24668954b 100644 --- a/config/v3_1/types/storage.go +++ b/config/v3_1/types/storage.go @@ -34,6 +34,14 @@ 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+"/") { @@ -41,6 +49,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -48,6 +59,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -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) + } + } + } } diff --git a/config/v3_1/types/storage_test.go b/config/v3_1/types/storage_test.go index 0bffdf3ab..f106deb80 100644 --- a/config/v3_1/types/storage_test.go +++ b/config/v3_1/types/storage_test.go @@ -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{ @@ -57,6 +59,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a file uses a configured symlink path returns ErrFileUsedSymlink { in: Storage{ Links: []Link{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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) + } + } +} diff --git a/config/v3_2/types/storage.go b/config/v3_2/types/storage.go index 4c09b556b..5cec008de 100644 --- a/config/v3_2/types/storage.go +++ b/config/v3_2/types/storage.go @@ -34,6 +34,14 @@ 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+"/") { @@ -41,6 +49,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -48,6 +59,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -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) + } + } + } } diff --git a/config/v3_2/types/storage_test.go b/config/v3_2/types/storage_test.go index 70d07ef2d..4667b8d20 100644 --- a/config/v3_2/types/storage_test.go +++ b/config/v3_2/types/storage_test.go @@ -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{ @@ -57,6 +59,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a file uses a configured symlink path returns ErrFileUsedSymlink { in: Storage{ Links: []Link{ @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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) + } + } +} diff --git a/config/v3_3/types/storage.go b/config/v3_3/types/storage.go index 3cb82ab79..20cb73048 100644 --- a/config/v3_3/types/storage.go +++ b/config/v3_3/types/storage.go @@ -34,6 +34,14 @@ 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+"/") { @@ -41,6 +49,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -48,6 +59,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -80,5 +94,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) + } + } + } } diff --git a/config/v3_3/types/storage_test.go b/config/v3_3/types/storage_test.go index 108ba1661..551688a27 100644 --- a/config/v3_3/types/storage_test.go +++ b/config/v3_3/types/storage_test.go @@ -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{ @@ -59,6 +61,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a file uses a configured symlink path returns ErrFileUsedSymlink { in: Storage{ Links: []Link{ @@ -76,6 +79,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{ @@ -93,6 +97,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{ @@ -109,6 +114,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkUsedSymlink, at: path.New("", "links", 1), }, + // test a configured symlink with no target returns ErrLinkTargetRequired { in: Storage{ Links: []Link{ @@ -121,6 +127,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkTargetRequired, at: path.New("", "links", 0, "target"), }, + // test a configured symlink with a nil target returns ErrLinkTargetRequired { in: Storage{ Links: []Link{ @@ -133,6 +140,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkTargetRequired, at: path.New("", "links", 0, "target"), }, + // test that two symlinks can be configured at a time { in: Storage{ Links: []Link{ @@ -147,6 +155,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a directory uses a configured symlink with the 'Hard:= true' returns ErrHardLinkToDirectory { in: Storage{ Links: []Link{ @@ -259,3 +268,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) + } + } +} diff --git a/config/v3_4_experimental/types/storage.go b/config/v3_4_experimental/types/storage.go index 3cb82ab79..20cb73048 100644 --- a/config/v3_4_experimental/types/storage.go +++ b/config/v3_4_experimental/types/storage.go @@ -34,6 +34,14 @@ 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+"/") { @@ -41,6 +49,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -48,6 +59,9 @@ func (s Storage) Validate(c vpath.ContextPath) (r report.Report) { } } } +} + +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+"/") { @@ -80,5 +94,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) + } + } + } } diff --git a/config/v3_4_experimental/types/storage_test.go b/config/v3_4_experimental/types/storage_test.go index 108ba1661..551688a27 100644 --- a/config/v3_4_experimental/types/storage_test.go +++ b/config/v3_4_experimental/types/storage_test.go @@ -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{ @@ -59,6 +61,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a file uses a configured symlink path returns ErrFileUsedSymlink { in: Storage{ Links: []Link{ @@ -76,6 +79,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{ @@ -93,6 +97,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{ @@ -109,6 +114,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkUsedSymlink, at: path.New("", "links", 1), }, + // test a configured symlink with no target returns ErrLinkTargetRequired { in: Storage{ Links: []Link{ @@ -121,6 +127,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkTargetRequired, at: path.New("", "links", 0, "target"), }, + // test a configured symlink with a nil target returns ErrLinkTargetRequired { in: Storage{ Links: []Link{ @@ -133,6 +140,7 @@ func TestStorageValidate(t *testing.T) { err: errors.ErrLinkTargetRequired, at: path.New("", "links", 0, "target"), }, + // test that two symlinks can be configured at a time { in: Storage{ Links: []Link{ @@ -147,6 +155,7 @@ func TestStorageValidate(t *testing.T) { }, }, }, + // test when a directory uses a configured symlink with the 'Hard:= true' returns ErrHardLinkToDirectory { in: Storage{ Links: []Link{ @@ -259,3 +268,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) + } + } +} diff --git a/docs/release-notes.md b/docs/release-notes.md index aa633fca3..1e15fac93 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -16,6 +16,8 @@ nav_order: 9 ### Changes - Warn if template for enabled systemd instance unit has no `Install` section +- Warn if filesystem overwrites partitioned disk +- Warn if `wipeTable` overwrites a filesystem that would otherwise be reused - Warn if `user`/`group` specified for hard link - Install ignition-apply in `/usr/libexec` - Convert `NEWS` to Markdown and move to docs site