From b007b0ea12017401412051fbca2da599edbc7b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Thu, 12 Jan 2023 17:39:54 +0100 Subject: [PATCH] internal/fsnode: add API for custom directories and files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an internal API for working with custom FS nodes such as Files and Directories. This implementation is agnostic to external API, such as Weldr API, Cloud API or osbuild stages. The purpose of it is to be the common translation layer between all of these "external" APIs and osbuild. In this stage, the representation for Files and Directories is added. The functionality is not yet used by any existing code. Note about user/group type being `interface{}`: I considered using the internal `users` representation for users and groups, but it contains additional information, which are not relevant for FS node user / group ownership representation. Therefore I didn't use it. I also considered using separate variables for user / group name (string) and uid / gid (int64). However, the implementation would need to ensure that only one of these typed values is set for user / group or ensure that it refers to the same group / user. My estimate was that the code ensuring that only one of these typed values is set would be probably as complex as the current implementation that checks the types stored in `interface{}` typed variable. And ensuring that the set user / group name and uid / gid is referring to the same user / group is nearly impossible to get right without actually building the image. Signed-off-by: Tomáš Hozza --- internal/fsnode/dir.go | 34 +++++++ internal/fsnode/dir_test.go | 81 ++++++++++++++++ internal/fsnode/file.go | 36 ++++++++ internal/fsnode/file_test.go | 81 ++++++++++++++++ internal/fsnode/fsnode.go | 133 +++++++++++++++++++++++++++ internal/fsnode/fsnode_test.go | 163 +++++++++++++++++++++++++++++++++ 6 files changed, 528 insertions(+) create mode 100644 internal/fsnode/dir.go create mode 100644 internal/fsnode/dir_test.go create mode 100644 internal/fsnode/file.go create mode 100644 internal/fsnode/file_test.go create mode 100644 internal/fsnode/fsnode.go create mode 100644 internal/fsnode/fsnode_test.go diff --git a/internal/fsnode/dir.go b/internal/fsnode/dir.go new file mode 100644 index 000000000..702165ab0 --- /dev/null +++ b/internal/fsnode/dir.go @@ -0,0 +1,34 @@ +package fsnode + +import "os" + +type Directory struct { + baseFsNode + ensureParentDirs bool +} + +func (d *Directory) IsDir() bool { + return true +} + +func (d *Directory) EnsureParentDirs() bool { + if d == nil { + return false + } + return d.ensureParentDirs +} + +// NewDirectory creates a new directory with the given path, mode, user and group. +// user and group can be either a string (user name/group name), an int64 (UID/GID) or nil. +func NewDirectory(path string, mode *os.FileMode, user interface{}, group interface{}, ensureParentDirs bool) (*Directory, error) { + baseNode, err := newBaseFsNode(path, mode, user, group) + + if err != nil { + return nil, err + } + + return &Directory{ + baseFsNode: *baseNode, + ensureParentDirs: ensureParentDirs, + }, nil +} diff --git a/internal/fsnode/dir_test.go b/internal/fsnode/dir_test.go new file mode 100644 index 000000000..303fc447d --- /dev/null +++ b/internal/fsnode/dir_test.go @@ -0,0 +1,81 @@ +package fsnode + +import ( + "os" + "testing" + + "github.com/osbuild/osbuild-composer/internal/common" + "github.com/stretchr/testify/assert" +) + +func TestDirectoryIsDir(t *testing.T) { + dir, err := NewDirectory("/etc/dir", nil, nil, nil, false) + assert.NoError(t, err) + assert.True(t, dir.IsDir()) +} + +func TestNewDirectory(t *testing.T) { + testCases := []struct { + name string + path string + mode *os.FileMode + user interface{} + group interface{} + ensureParentDirs bool + expected *Directory + }{ + { + name: "directory-simple", + path: "/etc/dir", + mode: nil, + user: nil, + group: nil, + ensureParentDirs: false, + expected: &Directory{baseFsNode: baseFsNode{path: "/etc/dir", mode: nil, user: nil, group: nil}, ensureParentDirs: false}, + }, + { + name: "directory-with-mode", + path: "/etc/dir", + mode: common.ToPtr(os.FileMode(0644)), + user: nil, + group: nil, + ensureParentDirs: false, + expected: &Directory{baseFsNode: baseFsNode{path: "/etc/dir", mode: common.ToPtr(os.FileMode(0644)), user: nil, group: nil}, ensureParentDirs: false}, + }, + { + name: "directory-with-user-and-group-string", + path: "/etc/dir", + mode: nil, + user: "user", + group: "group", + ensureParentDirs: false, + expected: &Directory{baseFsNode: baseFsNode{path: "/etc/dir", mode: nil, user: "user", group: "group"}, ensureParentDirs: false}, + }, + { + name: "directory-with-user-and-group-int64", + path: "/etc/dir", + mode: nil, + user: int64(1000), + group: int64(1000), + ensureParentDirs: false, + expected: &Directory{baseFsNode: baseFsNode{path: "/etc/dir", mode: nil, user: int64(1000), group: int64(1000)}, ensureParentDirs: false}, + }, + { + name: "directory-with-ensure-parent-dirs", + path: "/etc/dir", + mode: nil, + user: nil, + group: nil, + ensureParentDirs: true, + expected: &Directory{baseFsNode: baseFsNode{path: "/etc/dir", mode: nil, user: nil, group: nil}, ensureParentDirs: true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dir, err := NewDirectory(tc.path, tc.mode, tc.user, tc.group, tc.ensureParentDirs) + assert.NoError(t, err) + assert.Equal(t, tc.expected, dir) + }) + } +} diff --git a/internal/fsnode/file.go b/internal/fsnode/file.go new file mode 100644 index 000000000..3c6813ece --- /dev/null +++ b/internal/fsnode/file.go @@ -0,0 +1,36 @@ +package fsnode + +import ( + "os" +) + +type File struct { + baseFsNode + data []byte +} + +func (f *File) IsDir() bool { + return false +} + +func (f *File) Data() []byte { + if f == nil { + return nil + } + return f.data +} + +// NewFile creates a new file with the given path, data, mode, user and group. +// user and group can be either a string (user name/group name), an int64 (UID/GID) or nil. +func NewFile(path string, mode *os.FileMode, user interface{}, group interface{}, data []byte) (*File, error) { + baseNode, err := newBaseFsNode(path, mode, user, group) + + if err != nil { + return nil, err + } + + return &File{ + baseFsNode: *baseNode, + data: data, + }, nil +} diff --git a/internal/fsnode/file_test.go b/internal/fsnode/file_test.go new file mode 100644 index 000000000..32161841e --- /dev/null +++ b/internal/fsnode/file_test.go @@ -0,0 +1,81 @@ +package fsnode + +import ( + "os" + "testing" + + "github.com/osbuild/osbuild-composer/internal/common" + "github.com/stretchr/testify/assert" +) + +func TestFileIsDir(t *testing.T) { + file, err := NewFile("/etc/file", nil, nil, nil, nil) + assert.NoError(t, err) + assert.False(t, file.IsDir()) +} + +func TestNewFile(t *testing.T) { + testCases := []struct { + name string + path string + data []byte + mode *os.FileMode + user interface{} + group interface{} + expected *File + }{ + { + name: "empty-file", + path: "/etc/file", + data: nil, + mode: nil, + user: nil, + group: nil, + expected: &File{baseFsNode: baseFsNode{path: "/etc/file", mode: nil, user: nil, group: nil}, data: nil}, + }, + { + name: "file-with-data", + path: "/etc/file", + data: []byte("data"), + mode: nil, + user: nil, + group: nil, + expected: &File{baseFsNode: baseFsNode{path: "/etc/file", mode: nil, user: nil, group: nil}, data: []byte("data")}, + }, + { + name: "file-with-mode", + path: "/etc/file", + data: nil, + mode: common.ToPtr(os.FileMode(0644)), + user: nil, + group: nil, + expected: &File{baseFsNode: baseFsNode{path: "/etc/file", mode: common.ToPtr(os.FileMode(0644)), user: nil, group: nil}, data: nil}, + }, + { + name: "file-with-user-and-group-string", + path: "/etc/file", + data: nil, + mode: nil, + user: "user", + group: "group", + expected: &File{baseFsNode: baseFsNode{path: "/etc/file", mode: nil, user: "user", group: "group"}, data: nil}, + }, + { + name: "file-with-user-and-group-int64", + path: "/etc/file", + data: nil, + mode: nil, + user: int64(1000), + group: int64(1000), + expected: &File{baseFsNode: baseFsNode{path: "/etc/file", mode: nil, user: int64(1000), group: int64(1000)}, data: nil}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + file, err := NewFile(tc.path, tc.mode, tc.user, tc.group, tc.data) + assert.NoError(t, err) + assert.Equal(t, tc.expected, file) + }) + } +} diff --git a/internal/fsnode/fsnode.go b/internal/fsnode/fsnode.go new file mode 100644 index 000000000..06e0e2237 --- /dev/null +++ b/internal/fsnode/fsnode.go @@ -0,0 +1,133 @@ +package fsnode + +import ( + "fmt" + "os" + "path" + "regexp" +) + +const usernameRegex = `^[A-Za-z0-9_.][A-Za-z0-9_.-]{0,31}$` +const groupnameRegex = `^[A-Za-z0-9_][A-Za-z0-9_-]{0,31}$` + +type FsNode interface { + Path() string + Mode() *os.FileMode + // User can return either a string (user name/group name), an int64 (UID/GID) or nil + User() interface{} + // Group can return either a string (user name/group name), an int64 (UID/GID) or nil + Group() interface{} + IsDir() bool +} + +type baseFsNode struct { + path string + mode *os.FileMode + user interface{} + group interface{} +} + +func (f *baseFsNode) Path() string { + if f == nil { + return "" + } + return f.path +} + +func (f *baseFsNode) Mode() *os.FileMode { + if f == nil { + return nil + } + return f.mode +} + +// User can return either a string (user name) or an int64 (UID) +func (f *baseFsNode) User() interface{} { + if f == nil { + return nil + } + return f.user +} + +// Group can return either a string (group name) or an int64 (GID) +func (f *baseFsNode) Group() interface{} { + if f == nil { + return nil + } + return f.group +} + +func newBaseFsNode(path string, mode *os.FileMode, user interface{}, group interface{}) (*baseFsNode, error) { + node := &baseFsNode{ + path: path, + mode: mode, + user: user, + group: group, + } + + err := node.validate() + if err != nil { + return nil, err + } + return node, nil +} + +func (f *baseFsNode) validate() error { + // Check that the path is valid + if f.path == "" { + return fmt.Errorf("path must not be empty") + } + if f.path[0] != '/' { + return fmt.Errorf("path must be absolute") + } + if f.path[len(f.path)-1] == '/' { + return fmt.Errorf("path must not end with a slash") + } + if f.path != path.Clean(f.path) { + return fmt.Errorf("path must be canonical") + } + + // Check that the mode is valid + if f.mode != nil && *f.mode&os.ModeType != 0 { + return fmt.Errorf("mode must not contain file type bits") + } + + // Check that the user and group are valid + switch user := f.user.(type) { + case string: + nameRegex := regexp.MustCompile(usernameRegex) + if !nameRegex.MatchString(user) { + return fmt.Errorf("user name %q doesn't conform to validating regex (%s)", user, nameRegex.String()) + } + case int64: + if user < 0 { + return fmt.Errorf("user ID must be non-negative") + } + case nil: + // user is not set + default: + return fmt.Errorf("user must be either a string or an int64, got %T", user) + } + + switch group := f.group.(type) { + case string: + nameRegex := regexp.MustCompile(groupnameRegex) + if !nameRegex.MatchString(group) { + return fmt.Errorf("group name %q doesn't conform to validating regex (%s)", group, nameRegex.String()) + } + case int64: + if group < 0 { + return fmt.Errorf("group ID must be non-negative") + } + case nil: + // group is not set + default: + return fmt.Errorf("group must be either a string or an int64, got %T", group) + } + + return nil +} + +func (f *baseFsNode) IsDir() bool { + panic("IsDir() called on baseFsNode") +} diff --git a/internal/fsnode/fsnode_test.go b/internal/fsnode/fsnode_test.go new file mode 100644 index 000000000..3862143cd --- /dev/null +++ b/internal/fsnode/fsnode_test.go @@ -0,0 +1,163 @@ +package fsnode + +import ( + "fmt" + "os" + "testing" + + "github.com/osbuild/osbuild-composer/internal/common" + "github.com/stretchr/testify/assert" +) + +func TestBaseFsNodeValidate(t *testing.T) { + testCases := []struct { + Node baseFsNode + Error bool + }{ + // PATH + // relative path is not allowed + { + Node: baseFsNode{ + path: "relative/path/file", + }, + Error: true, + }, + // path ending with slash is not allowed + { + Node: baseFsNode{ + path: "/dir/with/trailing/slash/", + }, + Error: true, + }, + // empty path is not allowed + { + Node: baseFsNode{ + path: "", + }, + Error: true, + }, + // path must be canonical + { + Node: baseFsNode{ + path: "/dir/../file", + }, + Error: true, + }, + { + Node: baseFsNode{ + path: "/dir/./file", + }, + Error: true, + }, + // valid paths + { + Node: baseFsNode{ + path: "/etc/file", + }, + }, + { + Node: baseFsNode{ + path: "/etc/dir", + }, + }, + // MODE + // invalid mode + { + Node: baseFsNode{ + path: "/etc/file", + mode: common.ToPtr(os.FileMode(os.ModeDir)), + }, + Error: true, + }, + // valid mode + { + Node: baseFsNode{ + path: "/etc/file", + mode: common.ToPtr(os.FileMode(0o644)), + }, + }, + // USER + // invalid user + { + Node: baseFsNode{ + path: "/etc/file", + user: "", + }, + Error: true, + }, + { + Node: baseFsNode{ + path: "/etc/file", + user: "invalid@@@user", + }, + Error: true, + }, + { + Node: baseFsNode{ + path: "/etc/file", + user: int64(-1), + }, + Error: true, + }, + // valid user + { + Node: baseFsNode{ + path: "/etc/file", + user: "osbuild", + }, + }, + { + Node: baseFsNode{ + path: "/etc/file", + user: int64(0), + }, + }, + // GROUP + // invalid group + { + Node: baseFsNode{ + path: "/etc/file", + group: "", + }, + Error: true, + }, + { + Node: baseFsNode{ + path: "/etc/file", + group: "invalid@@@group", + }, + Error: true, + }, + { + Node: baseFsNode{ + path: "/etc/file", + group: int64(-1), + }, + Error: true, + }, + // valid group + { + Node: baseFsNode{ + path: "/etc/file", + group: "osbuild", + }, + }, + { + Node: baseFsNode{ + path: "/etc/file", + group: int64(0), + }, + }, + } + + for idx, testCase := range testCases { + t.Run(fmt.Sprintf("case #%d", idx), func(t *testing.T) { + err := testCase.Node.validate() + if testCase.Error { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}