disk: separate PathPolicies implementation to its own package

The `PathPolicies` implements a generic concept that can be fit on more
use cases than just mountpoints. Another one would be a policy for
creating custom files and directories in the image. Having the
implementation in the `disk` package and using data structures from the
`disk` and `blueprint` packages makes it impossible to use it for any
additional BP customization due to a circular dependencies that always
occurs.

Split out the implementation into a separate package `pathpolicy` as the
first step.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
This commit is contained in:
Tomáš Hozza 2023-02-03 11:45:41 +01:00 committed by Sanne Raymaekers
parent b3f695db72
commit 3075d3a2ad
5 changed files with 18 additions and 18 deletions

View file

@ -24,6 +24,7 @@ import (
"github.com/google/uuid"
"github.com/osbuild/osbuild-composer/internal/blueprint"
"github.com/osbuild/osbuild-composer/internal/pathpolicy"
)
const (
@ -52,7 +53,7 @@ const (
XBootLDRPartitionGUID = "BC13C2FF-59E6-4262-A352-B275FD6F7172"
)
var MountpointPolicies = NewPathPolicies(map[string]PathPolicy{
var MountpointPolicies = pathpolicy.NewPathPolicies(map[string]pathpolicy.PathPolicy{
"/": {Exact: true},
"/boot": {Exact: true},
"/var": {},
@ -184,7 +185,7 @@ func NewVolIDFromRand(r *rand.Rand) string {
return hex.EncodeToString(volid)
}
func CheckMountpoints(mountpoints []blueprint.FilesystemCustomization, mountpointAllowList *PathPolicies) error {
func CheckMountpoints(mountpoints []blueprint.FilesystemCustomization, mountpointAllowList *pathpolicy.PathPolicies) error {
invalidMountpoints := []string{}
for _, m := range mountpoints {
err := mountpointAllowList.Check(m.Mountpoint)

View file

@ -1,54 +0,0 @@
package disk
import (
"fmt"
"path"
)
type PathPolicy struct {
Deny bool // explicitly do not allow this entry
Exact bool // require and exact match, no subdirs
}
type PathPolicies = PathTrie
// Create a new PathPolicies trie from a map of path to PathPolicy
func NewPathPolicies(entries map[string]PathPolicy) *PathPolicies {
noType := make(map[string]interface{}, len(entries))
for k, v := range entries {
noType[k] = v
}
return NewPathTrieFromMap(noType)
}
// Check a given path at dir against the PathPolicies
func (pol *PathPolicies) Check(dir string) error {
// Quickly check we have a mountpoint and it is absolute
if dir == "" || dir[0] != '/' {
return fmt.Errorf("mountpoint must be absolute path")
}
// ensure that only clean mountpoints are valid
if dir != path.Clean(dir) {
return fmt.Errorf("mountpoint must be a canonical path")
}
node, left := pol.Lookup(dir)
policy, ok := node.Payload.(PathPolicy)
if !ok {
panic("programming error: invalid path trie payload")
}
// 1) path is explicitly not allowed or
// 2) a subpath was match but an explicit match is required
if policy.Deny || (policy.Exact && len(left) > 0) {
return fmt.Errorf("path '%s ' is not allowed", dir)
}
// exact match or recursive mountpoints allowed
return nil
}

View file

@ -1,50 +0,0 @@
package disk
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestPathPolicyCheck(t *testing.T) {
assert := assert.New(t)
entires := map[string]PathPolicy{
"/": {Exact: true},
"/boot": {Exact: true},
"/boot/efi": {Exact: true},
"/var": {},
"/var/empty": {Deny: true},
"/srv": {},
"/home": {},
}
policies := NewPathPolicies(entires)
assert.NotNil(policies)
tests := map[string]bool{
"/": true,
"/custom": false,
"/boot": true,
"/boot/grub2": false,
"/boot/efi": true,
"/boot/efi/redora": false,
"/srv": true,
"/srv/www": true,
"/srv/www/data": true,
"/var": true,
"/var/log": true,
"/var/empty": false,
"/var/empty/dir": false,
}
for k, v := range tests {
err := policies.Check(k)
if v {
assert.NoError(err)
} else {
assert.Errorf(err, "unexpected error for path '%s'", k)
}
}
}

View file

@ -1,135 +0,0 @@
package disk
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/osbuild/osbuild-composer/internal/common"
)
func TestNewPathTrieFromMap(t *testing.T) {
assert := assert.New(t)
type testCase struct {
entries map[string]interface{}
trie *PathTrie
}
tests := []testCase{
{
entries: map[string]interface{}{},
trie: &PathTrie{
Name: []string{},
},
},
{
entries: map[string]interface{}{
"/": common.ToPtr(1),
},
trie: &PathTrie{
Name: []string{},
Payload: common.ToPtr(1),
},
},
{
entries: map[string]interface{}{
"/": common.ToPtr(1),
"/var": common.ToPtr(2),
"/var/lib/chrony": common.ToPtr(3),
"/var/lib/chrony/logs": common.ToPtr(4),
"/var/lib/osbuild": common.ToPtr(5),
"/var/lib/osbuild/store/cache": common.ToPtr(6),
"/boot": common.ToPtr(7),
"/boot/efi": common.ToPtr(8),
},
trie: &PathTrie{
Name: []string{},
Payload: common.ToPtr(1),
Paths: []*PathTrie{
{
Name: []string{"boot"},
Payload: common.ToPtr(7),
Paths: []*PathTrie{
{
Name: []string{"efi"},
Payload: common.ToPtr(8),
},
},
},
{
Name: []string{"var"},
Payload: common.ToPtr(2),
Paths: []*PathTrie{
{
Name: []string{"lib", "chrony"},
Payload: common.ToPtr(3),
Paths: []*PathTrie{
{
Name: []string{"logs"},
Payload: common.ToPtr(4),
},
},
},
{
Name: []string{"lib", "osbuild"},
Payload: common.ToPtr(5),
Paths: []*PathTrie{
{
Name: []string{"store", "cache"},
Payload: common.ToPtr(6),
},
},
},
},
},
},
},
},
}
for _, tc := range tests {
have := NewPathTrieFromMap(tc.entries)
assert.NotNil(have)
assert.Equal(tc.trie, have)
}
}
func TestPathTrieLookup(t *testing.T) {
assert := assert.New(t)
entries := map[string]interface{}{
"/": "/",
"/boot": "/boot",
"/boot/efi": "/boot/efi",
"/var": "/var",
"/var/lib/osbuild": "/var/lib/osbuild",
"/var/lib/osbuild/store/cache": "/var/lib/osbuild/store/cache",
"/var/lib/chrony": "/var/lib/chrony",
"/var/lib/chrony/logs": "/var/lib/chrony/logs",
}
trie := NewPathTrieFromMap(entries)
testCases := map[string]string{
"/": "/",
"/srv": "/",
"/srv/data": "/",
"/boot": "/boot",
"/boot/efi": "/boot/efi",
"/boot/grub2": "/boot",
"/boot/efi/fedora": "/boot/efi",
"/var/lib/osbuild": "/var/lib/osbuild",
"/var/lib/osbuild/test": "/var/lib/osbuild",
"/var/lib/chrony": "/var/lib/chrony",
"/var/lib/chrony/test": "/var/lib/chrony",
"/var/lib/chrony/logs": "/var/lib/chrony/logs",
"/var/lib/chrony/logs/data": "/var/lib/chrony/logs",
}
for k, v := range testCases {
node, _ := trie.Lookup(k)
assert.NotNil(node)
assert.Equal(v, node.Payload, "Lookup path: '%s' (%+v)", k, node.Name)
}
}

View file

@ -1,123 +0,0 @@
package disk
import (
"sort"
"strings"
)
// splits the path into its individual components. Retruns the
// empty list if the path is just the absolute root, i.e. "/".
func pathTrieSplitPath(path string) []string {
path = strings.Trim(path, "/")
if path == "" {
return []string{}
}
return strings.Split(path, "/")
}
type PathTrie struct {
Name []string
Paths []*PathTrie
Payload interface{}
}
// match checks if the given trie is a prefix of path
func (trie *PathTrie) match(path []string) bool {
if len(trie.Name) > len(path) {
return false
}
for i := range trie.Name {
if path[i] != trie.Name[i] {
return false
}
}
return true
}
func (trie *PathTrie) get(path []string) (*PathTrie, []string) {
if len(path) < 1 {
panic("programming error: expected root node")
}
var node *PathTrie
for i := range trie.Paths {
if trie.Paths[i].match(path) {
node = trie.Paths[i]
break
}
}
// no subpath match, we are the best match
if node == nil {
return trie, path
}
// node, or one of its sub-nodes, is a match
prefix := len(node.Name)
// the node is a perfect match, return it
if len(path) == prefix {
return node, nil
}
// check if any sub-path's of node match
return node.get(path[prefix:])
}
func (trie *PathTrie) add(path []string) *PathTrie {
node := &PathTrie{Name: path}
if trie.Paths == nil {
trie.Paths = make([]*PathTrie, 0, 1)
}
trie.Paths = append(trie.Paths, node)
return node
}
// Construct a new trie from a map of paths to their payloads.
// Returns the root node of the trie.
func NewPathTrieFromMap(entries map[string]interface{}) *PathTrie {
root := &PathTrie{Name: []string{}}
keys := make([]string, 0, len(entries))
for k := range entries {
keys = append(keys, k)
}
sort.Strings(keys)
for _, k := range keys {
node, left := root.Lookup(k)
if len(left) > 0 {
node = node.add(left)
}
node.Payload = entries[k]
}
return root
}
// Lookup returns the node that is the prefix of path and
// the unmatched path segment. Must be called on the root
// trie node.
func (root *PathTrie) Lookup(path string) (*PathTrie, []string) {
if len(root.Name) != 0 {
panic("programming error: lookup on non-root trie node")
}
elements := pathTrieSplitPath(path)
if len(elements) == 0 {
return root, elements
}
return root.get(elements)
}